refactor(engine): VM spring cleaning #1893
Conversation
pmdartus
changed the title
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
pmdartus
force-pushed
the
pmdartus/vm-cleanup
branch
from
a35cc63
to
e187395
Compare
|
LGTM, I would like @caridy to also do a pass. |
pmdartus
force-pushed
the
pmdartus/vm-cleanup
branch
from
6ea3880
to
10cb036
Compare
pmdartus
force-pushed
the
pmdartus/vm-cleanup
branch
from
2283edd
to
d458fdc
Compare
| '$$lwc-synthetic-mode$$': true, | ||
| } as any); | ||
|
|
||
| vm.component = this; |
There was a problem hiding this comment.
for some reason I feel that the association between vm and component should happen before attachShadow, in case that is attempting to do anything with the host, probably ok though. just saying! the fact that vm.cmpRoot is undefined at that point, it probably a good indication that this line is also ok. I think I'm talking myself out of it now! jajaja, I withdraw the concern.
| @@ -62,8 +62,8 @@ export function invokeServiceHook(vm: VM, cbs: ServiceCallback[]) { | |||
| `Optimize invokeServiceHook() to be invoked only when needed` | |||
| ); | |||
| } | |||
| const { component, data, def, context } = vm; | |||
There was a problem hiding this comment.
I'm good with this as it is... but eventually we should just remove all services now that wire reform is in. The services are NOT used by anyone else ATM AFAIK cc @jodarove
|
|
||
| import { useSyntheticShadow } from '../../dom/src/env/dom'; | ||
| export type ShadowDomMode = 'open' | 'closed'; |
There was a problem hiding this comment.
TS definition:
interface ShadowRootInit {
delegatesFocus?: boolean;
mode: ShadowRootMode;
}
Replace this with ShadowRootMode
| import { useSyntheticShadow } from '../../dom/src/env/dom'; | ||
| export type ShadowDomMode = 'open' | 'closed'; | ||
|
|
||
| export interface TemplateCache { |
There was a problem hiding this comment.
why not making this a record instead?
Details
This PR streamlines the VM creations:
UninitalizedVMtype and only useVMacross the engine.isRoot,datacreateVMmethod:aor,tro.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:
GUS work item
W-7391508