fix: fix light dom slot forwarding bug #4452
Conversation
| // tree causes the slotter to unmount, then the slottable will also unmount. So using the | ||
| // current VM works either way. | ||
| if (isVCustomElement(vnode)) { | ||
| addVNodeToChildLWC(clonedVNode as VCustomElement); |
There was a problem hiding this comment.
This is the main change.
There was a problem hiding this comment.
Note that the order of disconnectedCallbacks firing has changed, when light DOM slot forwarding is enabled. I could not manage to get it to fire in exactly the same order as it did before.
I think this not a super-serious observable change – most of the time, disconnectedCallback ordering doesn't matter as much as connectedCallback ordering. This is because dC is mostly used for simple teardowns, whereas cC may be used to fire events that should be caught by the container. (dC cannot fire events because the element is disconnected from the DOM.)
There was a problem hiding this comment.
If I remember correctly, the original issue with cloning vnodes was disconnectedCallback not firing, so I think it should be fine as long as it's being called.
There was a problem hiding this comment.
Yep, this is why I added a test – should invoke connectedCallback/disconnectedCallback at all. It's more important that disconnectedCallback actually fires than that it fires in a particular order.
| '12:connectedCallback', | ||
| '4:disconnectedCallback', | ||
| ] | ||
| ); |
There was a problem hiding this comment.
This is actually a bugfix – we are now correctly firing the expected disconnectedCallbacks in synthetic lifecycle mode.
| '9:disconnectedCallback', | ||
| '4:disconnectedCallback', | ||
| ]); | ||
| }); |
There was a problem hiding this comment.
I didn't strictly need to add any remove entire element tests here, but I wanted to cover all my bases.
|
I think this should also resolve #3731 |
Details
Fixes #4446, partially reverts #4258.
The main thing this PR does is revert us from the 252 behavior to the 250 behavior, while also adding additional code to ensure that
disconnectedCallbackfires correctly. The bulk of what you need to know is in the code comments.Does this pull request introduce a breaking change?
Does this pull request introduce an observable change?
The ordering of
disconnectedCallbacks firing in light DOM may change when using slot forwarding.