lwc icon indicating copy to clipboard operation
lwc copied to clipboard

refactor(engine-core): slotting mechanism to remove vnode.owner

Open jodarove opened this issue 3 years ago • 5 comments

Details

The main goal of this PR is to remove the owner property of the vnode. The proposed solution changes how the slotting mechanism works to provide the correct owner while rendering.

vnode.owner is mainly used to associate an element with the proper shadow (and to set scoped styles) and is always the same as the VM being rendered, except for the slotted content, this PR mainly focuses on storing the owner only for the slot (and slotted content) and properly switching the owner when rendering.

This PR creates a new type of VSlot vnode, to be returned from the s() (template API) function, along with the corresponding mount/patch/unmount logic that takes care of rendering the slot and slotted content correctly while switching from "VM being rendered" while patching.

Missing:

  • [x] Hydration logic: the hydration logic needs to accommodate this change (requires #2729 ).
  • [x] (compiled code) investigate if we can remove the flatten call on nodes with slots (if we flatten them for the light dom case).
  • [ ] run perf tests
  • [x] LightDom: there is no test for it, but before this PR, the slotted content to a LightDom component is passed as part of the children, with this change, we need to patch those children dynamically (updateDynamicChildren), but in order to do it, we need an element at the beginning of both old and new children, that does not change between patches so it serves as an anchor (and to get the parentNode from it) for the diffing algo. An empty text (or comment) should be sufficient.

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.

jodarove avatar Feb 25 '22 15:02 jodarove

@pmdartus @nolanlawson @caridy this is ready for review. if we decide to move forward with this PR, ill update the hydration logic.

jodarove avatar Feb 25 '22 20:02 jodarove

I haven't get my head around this change yet, but at first glance, it seems a lot more complicated now than the actual mechanism. I will like to understand this solution better, and explore maybe something a little simpler.

caridy avatar Mar 16 '22 04:03 caridy

@jodarove this is to be closed isn't it?

caridy avatar Apr 19 '22 03:04 caridy

@jodarove ping

caridy avatar Apr 28 '22 03:04 caridy

@caridy Today we are able to assign the owner to each VNode because all the VNodes are created eagerly. This will certainly be needed for implementing scoped slots. To implement this feature, a parent component will have to pass a "template fragment" to a child component so it can be stamped with data. Removing this owner property would certainly make things easier for the scoped slot implementation.

FYI @abdulsattar

pmdartus avatar May 30 '22 15:05 pmdartus

This seems ready to be closed. Please reopen as necessary.

nolanlawson avatar Mar 31 '23 17:03 nolanlawson