search

enhance-dev / enhance-ssr

Server side render for custom elements.
143 stars 9 forks source link

Make stripping slots from markup optional #45

Closed lovelle-cardoso closed 1 year ago

lovelle-cardoso commented 1 year ago

Allows the user to choose whether enhance should strip slots out of their markup or leave them as is.

kristoferjoseph commented 1 year ago

This is not a preferred pattern since it changes the DOM structure of what should be a template. The way I got to the conclusion of removing unused slots is by making a Shadow DOM enabled component and then mimicking the ideal template DOM structure required to achieve the correct slotted content.

This PR would create excess DOM nodes that can break the intended behavior.

lovelle-cardoso commented 1 year ago

@kristoferjoseph I'd like to clarify that stripping slots from markup does not mimic shadow dom component structure, because shadow dom structure always includes slots in the dom tree. See screenshots below:

SHADOW DOM STRUCTURE: (notice that slot elements are present in markup tree) SHADOW_DOM_STRUCTURE

CURRENT ENHANCE DOM STRUCTURE: (notice that slots are not present in markup tree) OLD_LIGHT_DOM_STRUCTURE

OPTIONAL ENHANCE DOM STRUCTURE: (with stripSlotsFromMarkup=false from this PR) (notice that dom structure exactly matches shadow dom structure) NEW_LIGHT_DOM_STRUCTURE

Ultimately, rejecting the PR is your decision, but I see no harm in allowing users this option which they can turn on or off. Especially because we've seen that stripping slots actually breaks functionality in many cases.

For example, the bug this user is experiencing here is due to the automatic stripping of slots from markup: https://github.com/enhance-dev/enhance-ssr/issues/41

Do you have any specific examples where functionality is broken because slots are present?

(Also, in case changing the expected behaviour is your concern, I made sure that the stripSlotsFromMarkup defaults to the current behavior (stripSlotsFromMarkup=true), so it shouldn't impact any users that don't choose to switch it to false.)