Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address nested slot issues (light DOM) #5

Merged
merged 11 commits into from
Jun 8, 2023

Conversation

patricknelson
Copy link
Owner

@patricknelson patricknelson commented Jun 7, 2023

Fix for crisward/svelte-tag#7. Primary changes

  • Keep track of slots on connectedCallback() and then unwind them symmetrically on disconnectedCallback() to handle state/initialization bugs in case nested elements render from inside out
  • Ensure we restrict selection of slots only to the current element and not nested elements (i.e. from inverse perspective: traverse DOM up from slot and ensure nearest custom element is the current one being initialized).
  • Unit test to validate that only the slots that apply to the current element are selected and used during initialization
  • Unit test to validate that a different nested custom element is properly re-initialized (even though it renders first) when setup inside the default slot of another custom element

@patricknelson patricknelson marked this pull request as draft June 7, 2023 04:26
@patricknelson patricknelson force-pushed the issue-7-fix-nested-slot-issues branch 2 times, most recently from b6ee160 to f083c20 Compare June 7, 2023 05:05
@patricknelson patricknelson changed the title Address nested slot issues Address nested slot issues (light DOM) Jun 7, 2023
@patricknelson patricknelson marked this pull request as ready for review June 8, 2023 00:46
1. Keep track of slots on connectedCallback() and then unwind them symmetrically on disconnectedCallback() to handle state/initialization bugs in case nested elements render from inside out
2. Ensure we restrict selection of slots only to the current element and not nested elements (i.e. from inverse perspective: traverse DOM up from slot and ensure nearest custom element is the current one being initialized).
… and unnamed slots (i.e. elements without "slot" attrib) to coexist, which is what Svelte already does. Throw error in edge case if named slots (including a named slot="default") exists with still some unnamed content/elements remaining.
…re importantly helped with debugging an issue (edge case triggered when editing in DevTools)
…n't result in selecting the wrong slots. Using direct node equality to verify that the provided slot's parent belongs to the parent currently being processed. Also documenting interesting edge case (which hopefully doesn't come up in normal use, but if it does, will have some idea of what might be happening)
… be reused later (e.g. MutationObserver's for IIFEs executed prior to parsing of the light DOM elements)
…es for easier management/reference (also quicker tests). Using "test()" function instead of "it()" due to now they're already named anyway.
…itly named default conflicts with remaining content left over. Also: Renaming top level describe()'s
…ir own slots and not conflicting with slots lower down, or, that slots lower down aren't applied to custom element tags higher up, etc.
…t/outer elements will retain their state (slots) if outer element happens to render AFTER.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant