feat(engine-core): introducing DynamicLightningElement #3204

caridy · 2022-12-02T20:27:27Z

Details

POC on how can we create a magical <dynamic-lightning-element lwc:dynamic={ctor}> that accepts a ctor property to be passed in with an arbitrary LightningElement constructor. This implements dynamic elements in templates without the issues of previous implementations that were attempting to reuse the name of the element while replacing the custom element associated to it.

How do it work?

This new element, that is NOT an abstract class, but it is not final either, can be extended if needed. This element can be used in templates (compiler will know how to resolve it). Once the element is created (via the diffing algo), it will expect a property (ctor) to be provided... if provided, it should be a qualifying LightningElement, and a definition will be procured in the process.

Then the already created VM, and the already created and linked element, can be used to reset the state of the VM, replacing the associated definition, and boot up the component again by creating a new instance of it. The old instance will be collected by the GC, and the new instance will take control over the element. This process can be repeated as many times as you want.

Few other considerations:

The compiler can remove the element via diffing algo (vnode to be removed) if the ctor is not qualifying, or is pending (promise) to be resolved.
Any expando, or work on the actual element instance (which are rare) will be preserved even after replacing the constructor associated to it since the element instance is reused, only the component (cmp) is replaced.
The VM needs to be refreshed (reset), or at least parts of it to avoid creating a new VM all together. This should be fine since this is a controlled process.
The compiler can also trigger the reset of the properties (or spread) right after setting the ctor property (if we can guarantee that the ctor prop goes first, then we can just set the others right after). This is important... even if the old props are the same. Attributes on the other hand, don't need to be reset. Finally, listeners could be tricky.
If you want to observe the replacement process in user-land, you can rely on a new callback called replacedCallback, which give us a hook into the replacing process to accommodate the new instance.

Will this work when using web components?

Yes, <dynamic-lightning-element> can be registered as web components, as well as subclasses of it. This will work fine, of course, a constructor cannot be set from HTML, meaning only the property can be set, without reflection into the ctor attribute, instead we use the magical lwc:dynamic directive, which also allows the compiler to identify that this component is actually a dynamic component that needs special handling. Now, the ctor value can only be a LightningElement, attempting to pass a CustomElement Constructor will throw a type error.

What other work is needed for this to function?

The compiler work is pending. <dynamic-lightning-element> must be recognized by the parser, and the resolution for the constructor must be the internal DynamicLightingElement, which is exported from LWC the same way LightningElement is. The work of the compiler is very similar to what we do today with lwc:dynamic directive. The difference is that promises must be controlled by the output code, or a method in api.ts (that's TBD).

How to transition from the current implementation into this?

Additionally, we should be able to transform existing lwc:dynamic syntax into the new syntax to avoid breaking existing customers. We can make it very easy by allowing other tagNames with lwc:dynamic, to inherit from dynamic-lightning-element, and that way, they can continue using the same syntax. Remember that only a very small group of people are allowed to use this. Eventually, a lot of code can be removed to eliminate the lwc:dynamic directive all together from compiler and runtime.

Does this pull request introduce a breaking change?

✅ No, it does not introduce a breaking change.

Does this pull request introduce an observable change?

✅ No, it does not introduce an observable change.

GUS work item

TBD

caridy · 2022-12-02T20:29:31Z

packages/@lwc/engine-core/src/framework/vm.ts

@@ -269,6 +269,19 @@ function getNearestShadowAncestor(vm: VM): VM | null {
    return ancestor;
 }

+export function createVMContext() {


this is just abstracting a segment of the vm creation process so we can create more in dynamic.

Note: I'm not 100% sure if the ctx is cached somewhere, I believe it is only used inside templates, in which case it doesn't matter since the template is algo going to be replaced.

Note 2: what happen if you swap ctors, but both resolves to the same template? jejejeje

caridy · 2022-12-02T20:32:03Z

packages/@lwc/engine-core/src/framework/components/dynamic.ts

+    }
+}
+
+registerComponent(DynamicLightingElement, { tmpl: defaultEmptyTemplate });


this is needed since we are not going to compile this class via our compiler.

note: do we need to mark ctor as public API? I don't think we need to since it will not be set via the regular diffing algo, but with some special mechanism.

note 2: what about IE11/compat? will this class definition work? I believe it does, we only have some prior art in this package in the reactivity class definition.

caridy · 2022-12-02T20:32:22Z

packages/@lwc/engine-core/src/framework/components/dynamic.ts

+        // and it is not configurable to avoid something messing with its public API. The
+        // fact that it is installed on instance guarantees that changing the proto-chain
+        // will not affect its behavior.
+        defineProperty(getVMFromComponent(this).elm, 'ctor', {


this is the most important part, read the comment.

Does this need to be a public property? Could we use a WeakMap instead?

it can't be a weakmap if we want to allow DynamicLightningElement.CustomElementConstructor to be useful. Basically, if it is a weakmap, then a public API will have to be exposed from LWC so people can set the replace in motion by calling that API with an element and the new ctor, and that api will do what we do here... but then exporting a LWC as web components makes it a lot less useful. Another option is to make it a method rather than a setter, which I'm fine with as well.

caridy · 2022-12-02T20:32:58Z

packages/@lwc/engine-core/src/framework/components/dynamic.ts

+
+    // validation of the mode... it must match the mode of the original implementation
+    // users can extend DynamicLightingElement to adjust the mode at will.
+    // do we need the same thing for cm.renderMode? I believe we do!


I'm not sure about renderMode.

abdulsattar · 2022-12-05T10:42:16Z

Can you explain when users might want to extend DynamicLightningElement?

nolanlawson · 2022-12-05T17:53:54Z

packages/@lwc/engine-core/src/framework/components/dynamic.ts

+    replacedCallback(): void {
+        // this is useful for subclasses to get a notification every time the
+        // component is swapped after setting `ctor` property.
+    }


If this is an empty function, does it need to be defined? Unless we're worried that subclasses will get an error if they do super.replacedCallback() inside of their own replacedCallback.

I think you're right... it could be undefined... and put an if condition around the calls to it. I believe connectedCallback and co. are always defined in LightningElement though, IIRC.

They are not. Calling super.connectedCallback() throws an error.

In a sense, replacedCallback can be seen as a special case of "disconnectedCallback followed by connectedCallback." Is it necessary to even have replacedCallback, given that developers can use those events?

Arguably, developers should be handling "disconnectedCallback followed by connectedCallback" anyway (although I wouldn't be surprised if they're not – web component authors often don't expect their component to be removed and then re-added).

Ok, I'm fine making this undefined.

As for the second part of your comment, the replacedCallback is not for the author of the component being replaced, it is for the consumer to know that something on its template is being replaced... and this is only there because of the promises.

nolanlawson · 2022-12-06T16:56:12Z

packages/@lwc/engine-core/src/framework/components/dynamic.ts

+
+function replaceDefinitionForVM(vm: VM, newDef: ComponentDef, replacedCallback: () => void) {
+    if (vm.state === VMState.connected) {
+        runDisconnectedCallback(vm);


Assuming we enable native connected/disconnected callback events (#3198), this would be the one place where we are simulating our own disconnected callback behavior. This introduces unknown unknowns – e.g. subtle bugs where we incorrectly detect connectedness in templates (like #2609 (comment)).

Are we sure we want to add another source of synthetic lifecycle callbacks? Is there some other way we can achieve this? (E.g. explicitly removing and re-adding the DOM node.)

Hmm, I don't understand this part. How is this different from rehydration from SSR?

Rehydration is another case where we manually invoke connected callback, you're right. Although I'm not sure we will always do it that way. /cc @divmain

In Lit for example I believe they use the native connectedCallback, but they queue up the callbacks in SSR so that they fire in a predictable order. (Otherwise they will fire based on when customElements.define() is called, which is kind of random and unpredictable.

A compelling reason hasn't surfaced yet to move us away from manually invoking connectedCallback during rehydration. Lit uses the pattern you mention, as well as adding/removing ephemeral attrs (with corresponding event listeners) to enforce deterministic order of execution through a component hierarchy. The reason we were originally considered moving in this direction has gone away, now that we've decided to lock down DOM APIs on the host element.

The reason we were originally considered moving in this direction has gone away, now that we've decided to lock down DOM APIs on the host element.

Can you elaborate on this? You mean moving away in terms of hydration? (Because I'm still hoping to get #3198 in, at least for the non-hydration scenario.)

nolanlawson

I'm a bit concerned about this approach. A few downsides I can think of:

We will need a "magical" <dynamic-lightning-element> element that is treated specially by the parser. We don't have a precedent for this kind of thing in LWC yet.
This adds a lot of dynamism to the existing linking between the element the VM. We can no longer assume the VM is read-only – instead, it might be mutated at any time.
We are getting into the business of simulating connectedCallback and disconnectedCallback even though the element is not truly mounting/unmounting. (Yes we already do this in hydration, but why do it in more places?)
We're adding new API surface in replacedCallback.

To me, the alternative approach of using a <template> with a directive that merely removes the old element from the DOM and inserts the new element (using standard VDOM operations) is much simpler conceptually and technically. No need for a magical tag name, no need to make VM mutable, no need to simulate connectedCallback/disconnectedCallback, and no need to implement replacedCallback (unless we really like it as a convenience thing). And it also resolves the issue of sharing tag names between implementations, because the tag name simply won't be shared (i.e. no need for any kind of "pivot").

caridy · 2022-12-09T21:29:25Z

@nolanlawson

We will need a "magical" <dynamic-lightning-element> element that is treated specially by the parser. We don't have a precedent for this kind of thing in LWC yet.

No, it is not the name what matters, because you can extend that and call it whatever you want... it is the lwc:dynamic directive the one that ticks the parser to do the additional work to set the ctor property which might be a promise.

This adds a lot of dynamism to the existing linking between the element the VM. We can no longer assume the VM is read-only – instead, it might be mutated at any time.

Well... yes... although the vm is still 1-1 to the element. It is still 1-1-1 vm-component-element with the difference that the component vector is replacable. Not sure what's wrong with that since we don't cache many things by component.

We are getting into the business of simulating connectedCallback and disconnectedCallback even though the element is not truly mounting/unmounting. (Yes we already do this in hydration, but why do it in more places?)

Why do you think this is important? It is not really about rehydration, it is the fact that you need an elm to be able to create a component instance, whatever the connectedness of that elm is, that process should work the same way. This is analogous to definition of a name after elements are part of the DOM, which means they get upgraded.

We're adding new API surface in replacedCallback.

We can remove it... if we feel that's not the right thing to do... prior art is errorCallback.

To me, the alternative approach of using a <template> with a directive that merely removes the old element from the DOM and inserts the new element (using standard VDOM operations) is much simpler conceptually and technically. No need for a magical tag name, no need to make VM mutable, no need to simulate connectedCallback/disconnectedCallback, and no need to implement replacedCallback (unless we really like it as a convenience thing). And it also resolves the issue of sharing tag names between implementations, because the tag name simply won't be shared (i.e. no need for any kind of "pivot").

Let's do an actual comparison, because I will probably disagree with many of the things that you're listing there.

nolanlawson · 2022-12-09T23:24:31Z

@caridy

No, it is not the name what matters, because you can extend that and call it whatever you want...

OK that's fair, I thought <dynamic-lightning-element> was special somehow.

Not sure what's wrong with that since we don't cache many things by component.

It just feels error-prone for us to share the VM state across two completely different components... e.g. are we sure we're doing the garbage collection correctly and cleaning up styles when we unmount+mount? Whereas we already have a well-worn path for doing an element unmount+mount.

Why do you think this is important?

The current synthetic connectedCallback/disconnectedCallback system is a bug factory (#3198) with a lot of tricky edge cases that we don't handle correctly. (E.g. this fun one with <template>s.) I just get nervous when we start talking about simulating native browser behavior, especially when (again) we already have a well-worn path for doing unmount+mount.

Let's do an actual comparison

Agreed, let's do a bake-off. I suspect the other implementation is cleaner, but who knows. 🙂

jmsjtu · 2022-12-12T07:15:39Z

@caridy @nolanlawson - I did a POC (#3218) for how we can use lwc:dynamic on a <template>, let me know what you think.

nolanlawson · 2023-03-31T17:39:11Z

This design has been dropped in favor of the new lwc:dynamic proposal, so I think we can close this PR.

feat(engine-core): introducing DynamicLightningElement

db0d8e1

caridy requested review from nolanlawson, pmdartus and jmsjtu December 2, 2022 20:27

caridy commented Dec 2, 2022

View reviewed changes

nolanlawson reviewed Dec 5, 2022

View reviewed changes

nolanlawson reviewed Dec 6, 2022

View reviewed changes

feat(engine-core): making replacedCallback optional

9d6acba

nolanlawson reviewed Dec 9, 2022

View reviewed changes

jmsjtu mentioned this pull request Dec 12, 2022

POC: dynamic component template directive #3218

Closed

nolanlawson closed this Mar 31, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(engine-core): introducing DynamicLightningElement #3204

feat(engine-core): introducing DynamicLightningElement #3204

caridy commented Dec 2, 2022 •

edited

Loading

caridy Dec 2, 2022

caridy Dec 2, 2022

caridy Dec 2, 2022 •

edited

Loading

nolanlawson Dec 5, 2022

caridy Dec 6, 2022

caridy Dec 2, 2022

abdulsattar commented Dec 5, 2022

nolanlawson Dec 5, 2022

caridy Dec 6, 2022

nolanlawson Dec 6, 2022

nolanlawson Dec 6, 2022

caridy Dec 8, 2022

nolanlawson Dec 6, 2022

caridy Dec 8, 2022

nolanlawson Dec 9, 2022

divmain Dec 12, 2022

nolanlawson Dec 12, 2022

nolanlawson left a comment

caridy commented Dec 9, 2022

nolanlawson commented Dec 9, 2022 •

edited

Loading

jmsjtu commented Dec 12, 2022

nolanlawson commented Mar 31, 2023

feat(engine-core): introducing DynamicLightningElement #3204

feat(engine-core): introducing DynamicLightningElement #3204

Conversation

caridy commented Dec 2, 2022 • edited Loading

Details

How do it work?

Will this work when using web components?

What other work is needed for this to function?

How to transition from the current implementation into this?

Does this pull request introduce a breaking change?

Does this pull request introduce an observable change?

GUS work item

Choose a reason for hiding this comment

Choose a reason for hiding this comment

caridy Dec 2, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abdulsattar commented Dec 5, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nolanlawson left a comment

Choose a reason for hiding this comment

caridy commented Dec 9, 2022

nolanlawson commented Dec 9, 2022 • edited Loading

jmsjtu commented Dec 12, 2022

nolanlawson commented Mar 31, 2023

caridy commented Dec 2, 2022 •

edited

Loading

caridy Dec 2, 2022 •

edited

Loading

nolanlawson commented Dec 9, 2022 •

edited

Loading