refactor(engine-core): Remove duplicate Component type definition #2157
Conversation
|
Note: This PR move certain import to use What is the rest of the team's opinion on this? @ekashida @ravijayaramappa @jodarove @caridy |
| import { Template, isUpdatingTemplate, getVMBeingRendered } from './template'; | ||
|
|
||
| export type ErrorCallback = (error: any, stack: string) => void; |
There was a problem hiding this comment.
I suspect no one was using this one, right?
There was a problem hiding this comment.
This one was used in a single place. I replaced it with the LightningElement['errorCallback'] type.
| readonly delegatesFocus?: boolean; | ||
| } | ||
|
|
||
| export interface ComponentMeta { |
There was a problem hiding this comment.
This wasn't used at all.
|
|
||
| export interface ComponentConstructor extends LightningElementConstructor { | ||
| readonly name: string; | ||
| readonly labels?: string[]; |
There was a problem hiding this comment.
I don't recall name and labels usage, any insight here?
There was a problem hiding this comment.
@pmdartus what about these ones?
There was a problem hiding this comment.
Sorry I missed your comment. I did some archeology, and those 2 fields have been introduced with SnabbDOM v2 refactor: #47. I don't recall us ever using those fields either.
I certainly will like to see that discussed outside of the context of this PR, unless that this refactor introduces another circular ref for types that makes it hard or impossible to achieve. The reasons why I'm a little concern about import type is that TS team usually recommends against it, unless of very specific circustances. |
pmdartus
force-pushed
the
pmdartus/remove-duplicate-types
branch
from
2ff1399
to
31fdc5a
Compare
|
Reverted the |
| new (): LightningElement; | ||
| readonly prototype: LightningElement; | ||
| readonly CustomElementConstructor: HTMLElementConstructor; | ||
|
|
||
| delegatesFocus?: boolean; | ||
| } | ||
|
|
||
| export declare let LightningElement: LightningElementConstructor; |
There was a problem hiding this comment.
Lifting LightningElementConstructor and LightningElement type definition to a separate module will reduce the cyclic dependencies between modules quite a bit.
Details
While investigating the
MacroElementproposal I realized that the@lwc/enginecontains competing type definition for theLightningElementclass:LightElementinterface andComponentinterface. This PR removesComponentinterface definition. It also fixes #1291.Does 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: