fix(typescript): add createElement ts definition #1991
Conversation
packages/lwc/types.d.ts
Outdated
| @@ -4,6 +4,9 @@ | |||
| * SPDX-License-Identifier: MIT | |||
| * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT | |||
| */ | |||
|
|
|||
| import { HTMLElement } from '@lwc/template-compiler/dist/types/shared/types'; | |||
There was a problem hiding this comment.
Rather than import the HTMLElement type definition from template-compiler, we should stick to the same definition source for all the DOM-related type definitions.
I believe that should be typescript's lib.dom.t.s. I'm not sure what the best way to pull that in here...it seems to be sufficient to create an empty tsconfig.json file. Could we remove this line for now and we can figure out the right way to do it in a separate PR? /cc @pmdartus
There was a problem hiding this comment.
Looked into this a little more and I believe that #1993 is what we need.
There was a problem hiding this comment.
ok, thanks, I can remove line 8.
| @@ -167,4 +169,9 @@ declare module 'lwc' { | |||
| * @param config configuration object for the accessor | |||
| */ | |||
| export function wire(getType: (config?: any) => any, config?: any): PropertyDecorator; | |||
|
|
|||
| export function createElement( | |||
There was a problem hiding this comment.
We definitely don't want to add this type, it is a high privilege function that is only accessible to some and it is pending to be removed or change in the future as we move more and move into Custom Elements.
There was a problem hiding this comment.
@caridy I'm surprised to hear that you be removing createElement, isn't that the entry-point and key to bootstrapping the LWC app? At this point everyone who is running LWC is using this method and if they are using TS then I would imagine they will have the same issue I do.
Shouldn't the types be a holistic representation of the current API? Then when you change the API the d.ts can match the new version.
I can always add //@ts-ignore but if there is TS support fro LWC then I'm confused why we wouldn't want the types for all the methods used in the API instead of it feeling broken as soon as you open the app after npx create-lwc-app -o typescript
Thanks for your input!
There was a problem hiding this comment.
No, that is just an escape hatch, and has always be that. We want folks to use the standard mechanism to create instances of custom elements rather than a proprietary API, but since web components APIs were not ready yet... we have to do something, and restrictive as much as possible. For example, in platform, folks cannot import such method... only high privilege methods can.
In OSS, you will do the same you do with any other web components abstractions, e.g.:
import MyComponent from './some/path';
customElements.define('x-the-tagname-for-my-component', MyComponent.CustomElementConstructor);
const elm = document.createElement('x-the-tagname-for-my-component');The question is: can you make it work with that? or do you have other requirements that prevent the usage of that? e.g.: do you need IE11?
There was a problem hiding this comment.
ok, thanks for the explanation, that makes more sense. As far as I know we do not need to support IE 11 or any IE 🤞.
So even though the current docs and examples use createElement from 'lwc', would you recommend we use the standard mechanism you show above for a new oss project or at this time, is there no advantage to switching?
There was a problem hiding this comment.
yes, I will recommend that! and we should probably update the docs to reflect that. cc @jodarove @jbleyleSF
There was a problem hiding this comment.
ok, we will go with the standard mechanism, thanks for your input! I'll close this PR.
Details
This PR will add a typescript definition for the
createElementmethod of the lwc module. The definition is based on this signatureDoes this PR introduce breaking changes?
No, it does not introduce breaking changes.If yes, please describe the impact and migration path for existing applications.
The PR fulfills these requirements:
GUS work item
There is no GUS WI for this change