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

Slots content not replaced properly in custom elements when shadow root is disabled (v4) #8686

Closed
patricknelson opened this issue Jun 1, 2023 · 10 comments

Comments

@patricknelson
Copy link

patricknelson commented Jun 1, 2023

Describe the bug

Improvements were made to custom elements in #8457 for v4 to allow you to disable the shadow root. However, slot content appears to not be positioned properly when replaced. See screenshot and reproduction below.

Specifically: When slot content is defined inside of the custom element, instead of placing that content into the slot positions in the component, they are instead positioned at the very top of the host in the order in which they were defined (prior to mounting, a.k.a. connectedCallback).

Note: This affects both named and default slots.

Example component:

<svelte:options
	customElement={{
		tag: 'example-no-shadow',
		shadow: 'none',
	}}
/>

<h2>shadow: 'none'</h2>
<p>Slot 1:</p>
<slot name="slot1"><p>Fallback content 1</p></slot>
<p>Slot 2:</p>
<slot name="slot2"><p>Fallback content 1</p></slot>

Example HTML:

<example-no-shadow>
	<p class="replaced" slot="slot2">replaced slot2</p>
	<p class="replaced" slot="slot1">replaced slot1</p>
</example-no-shadow>

DOM tree:

image

Rendered output:

image

Reproduction

Code at https://github.com/patricknelson/svelte-v4-custom-element-slots

git clone https://github.com/patricknelson/svelte-v4-custom-element-slots.git
cd svelte-v4-custom-element-slots
npm i
npm run dev

Logs

No response

System Info

System:
    OS: Linux 5.15 Debian GNU/Linux 11 (bullseye) 11 (bullseye)
    CPU: (16) x64 Intel(R) Core(TM) i7-10875H CPU @ 2.30GHz
    Memory: 2.37 GB / 5.79 GB
    Container: Yes
    Shell: 5.1.4 - /bin/bash
  Binaries:
    Node: 16.14.2 - ~/.nvm/versions/node/v16.14.2/bin/node
    Yarn: 1.22.19 - ~/.nvm/versions/node/v16.14.2/bin/yarn
    npm: 8.5.0 - ~/.nvm/versions/node/v16.14.2/bin/npm
  Browsers:
    Chrome: 111.0.5563.146
  npmPackages:
    svelte: ^4.0.0-next.0 => 4.0.0-next.0

Severity

blocking an upgrade

@dominikg
Copy link
Member

dominikg commented Jun 2, 2023

related to #1689 ?

Does it work with svelte-3 or svelte-4 defaults (without shadow option)?

@dummdidumm dummdidumm added the bug label Jun 2, 2023
@dummdidumm dummdidumm added this to the 4.x milestone Jun 2, 2023
@patricknelson
Copy link
Author

@dominikg As it turns out, this issue applies only to when no shadow root is in use, which happens to be strictly a v4 feature (circa #8457). Thankfully I happen to already have an example in the demo repo noted above (here) and the first example shows this particular bug (i.e. the slots not being replaced properly at all) working just fine when the shadow root is in use.

See here:

image

@dummdidumm
Copy link
Member

This is a limitation of shadow: "none" we need to document - there's no way to use custom elements without a shadow root and still use slots, the browser does not have such a concept.

@patricknelson
Copy link
Author

Hmm... @dummdidumm I've implemented slots fairly flawlessly in my package svelte-retag (https://github.com/patricknelson/svelte-retag) for Svelte 3 (probably works in v4, haven't tried it yet since I only just created it). Not only does it work when deferred (where the full DOM is already known) but also when running early before parsing, using MutationObserver.

Check it out and see what I mean.

@patricknelson
Copy link
Author

patricknelson commented Jun 14, 2023

Note: My version, at least for light DOM, only cares about initial page load when handling slots, but since it uses the concept of repackaging/rearranging the rendering of the Svelte component into a special container (i.e. a simple <svelte-retag> custom element to cope with slots during IIFE-based initialization), it might be feasible to continue watching slots and slot mutations after page load as well (or, more specifically, after DOMContentLoaded has fired).

patricknelson added a commit to patricknelson/svelte-retag that referenced this issue Jun 14, 2023
@patricknelson
Copy link
Author

@dummdidumm I was able to setup a proof of concept for you demonstrating slots in light DOM here:

https://svelte-retag.vercel.app/

The page consists essentially of just this (and each one of these are an implementation of a Svelte component with slots as a custom element using my package svelte-retag):

<app-tag title="Module Demo">

	<intro-tag>
		<p>See <a href="iife.html">IIFE demo</a>.</p>
	</intro-tag>

	<example-tag>
		<p slot="title">Default: Count starts at 0, award at 100.</p>
		<counter-tag slot="content"></counter-tag>
	</example-tag>

	<example-tag>
		<p slot="title">Start out just 1 click away from the "award".</p>
		<counter-tag slot="content" count="99"></counter-tag>
	</example-tag>

	<example-tag>
		<p slot="title">1337 award</p>
		<counter-tag slot="content" count="1337" award="1337"></counter-tag>
	</example-tag>

	<example-tag>
		<p slot="title">Nested slot: "Total is"</p>
		<counter-tag slot="content">Total is</counter-tag>
	</example-tag>

</app-tag>

@dummdidumm
Copy link
Member

While technically possible I'm still in the camp of deliberately not supporting this use case. If you want to use custom elements, then you have to deal with their limitations. If you don't want shadow dom but still use slots, why not use Svelte directly instead at that point?

btw svelte-retag looks really good!

@patricknelson
Copy link
Author

If you don't want shadow dom but still use slots, why not use Svelte directly instead at that point?

Simply put: Legacy. It’s a very good migration path in my particular use case. An extremely large site, very limited development resources and lots of old/existing code (in global CSS, jQuery and etc). This offers a fantastic method by which we can eventually do just that: move everything over just to Svelte. But, until then, custom elements offers that standard interface by which we can maintain our existing server-side generated code (i.e. PHP in this case) while ensuring that newly built components can not finally be built using more modern techniques.

Without custom elements (and the support of slots like these), this migration path simply wouldn’t be possible and that’s why I put so much time and effort into supporting it (and will continue to do so for a while). And when you look at other folks asking for this, they’re usually in the same camp (i.e. large websites that they’re struggling to modernize). Thankfully custom elements is now so much better supported by browsers, enabling this type of migration to finally take place.

@patricknelson
Copy link
Author

IMHO, this feature was a blocker in preventing our team from picking up Svelte on our main site (which I focus on for my full time job, started in late 2014). The ability to use the combination of custom elements + slots is a major stepping stone in our work. The first major implementation of it will likely be in the redesign of our homepage, actually (the first real redesign of that particular page circa maybe 2015 or so).

@patricknelson
Copy link
Author

For those watching this issue, I set this up as a feature request here: #8963 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants