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

Nesting Svelte Web Components is broken with slots #8377

Closed
fallaciousreasoning opened this issue Mar 12, 2023 · 13 comments
Closed

Nesting Svelte Web Components is broken with slots #8377

fallaciousreasoning opened this issue Mar 12, 2023 · 13 comments

Comments

@fallaciousreasoning
Copy link
Contributor

fallaciousreasoning commented Mar 12, 2023

Describe the bug

  1. Create a Svelte Web Component with <svelte:options tag="x-parent"/>
  2. Create a child component with a named & unamed slot and add it to the parent component <svelte:options tag="x-child"/>
  3. Set customElement: true in the compileOptions in Svelte

When rendering an x-parent on the page inspecting it's shadow dom reveals that the markup for x-child has been inlined into x-parent, which means that x-child's slots are exposed via x-parent and styles for x-child are not applied.

(note: I've set the severity as blocking all usage but it only blocks it a specific case: Nesting elements, when customElement: true)

Reproduction

Working REPL (without customElement: true)
https://svelte.dev/repl/fde37dfe1ca64e56960a0cf7b64bc242?version=3.56.0

If you download the REPL and set customElement: true in rollup.config.js and run npm run dev nothing will render.

Inspecting the output ShadowDOM you can see that the source for x-child has been inlined into the template for x-parent.

The expected result here would be that <x-parent> contains an <x-child>

Logs

No response

System Info

System:
    OS: Linux 5.19 Ubuntu 22.10 22.10 (Kinetic Kudu)
    CPU: (20) x64 12th Gen Intel(R) Core(TM) i9-12900HK
    Memory: 31.62 GB / 62.48 GB
    Container: Yes
    Shell: 5.2.2 - /bin/bash
  Binaries:
    Node: 16.18.0 - ~/.nvm/versions/node/v16.18.0/bin/node
    npm: 8.19.2 - ~/.nvm/versions/node/v16.18.0/bin/npm
  Browsers:
    Brave Browser: 110.1.48.171
    Chrome: 112.0.5615.20
    Chromium: 110.0.5481.177
    Firefox: 110.0.1
  npmPackages:
    rollup: ^3.15.0 => 3.19.1 
    svelte: ^3.55.0 => 3.56.0

Severity

blocking all usage of svelte (when customElement: true)

Potential Workaround

Instead of

<script>
    import Child from './child.svelte'
</script>

<Child/>

it might be possible to directly use the web component

 <x-child/>

However, this is not a solution for our (admittedly very specific) use case, as we want to be able to optionally compile to Web Components, rather than always. We're building a component Library, so it's useful to be able to consume the elements from Svelte or expose them via Web Components so we can consume them from existing React & Polymer projects.

@fallaciousreasoning
Copy link
Contributor Author

fallaciousreasoning commented Mar 13, 2023

Possibly a duplicate of
#1689 or #7428

Related to
#4274, #4402, #6481, #5870

@fallaciousreasoning
Copy link
Contributor Author

Okay, so to get around this, we've had to stop using Svelte's built in customElement support due to a number of limitations which are covered more thuroughly in #6481. However, we still really want to use custom elements, so we can expose the components to other frameworks.

Our current workaround has been to create our own web component wrapper for Svelte components which works pretty well.
https://github.com/brave/leo/blob/main/src/components/svelte-web.ts

I think it's probably worth removing the built in customElement support, as it's kind of half baked and doesn't support a number of common use cases. In addition, having it in the documentation is misleading, as it implies that it's the best path for using Svelte with Web Components. A better alternative might be to point to some "Official Svelte Web Component Wrapper Example". That way people don't get pushed down the wrong path at the beginning.

Svelte can play nice with web components, it's just that the blessed path doesn't seem to.

@dummdidumm
Copy link
Member

Closing as duplicate of #1689 and #7428

@dummdidumm dummdidumm closed this as not planned Won't fix, can't repro, duplicate, stale Apr 3, 2023
@patricknelson
Copy link

patricknelson commented Jun 15, 2023

@fallaciousreasoning depending on if you're using the light DOM or not, svelte-retag might work for you. I know that it works very well with nesting Svelte custom elements in the regular DOM. Welcome your feedback!

p.s. It might also work pretty well regarding nesting in the shadow DOM as well, it's just fairly new and I haven't tested yet (thus open to feedback)!

@patricknelson
Copy link

Also, just to clarify (after rereading):

  1. This allows you to import both <ExampleComponent/> as usual in .svelte files
  2. Separately define your specific <example-component> custom element tags as needed when using it externally outside of Svelte
  3. Nesting works fine.

@patricknelson
Copy link

patricknelson commented Jun 15, 2023

p.s.s. @fallaciousreasoning it's insane how similar the code you wrote and svelte-retag are 😅. Convergent evolution (that and the haphazard conglomeration of the efforts of many, rebuilt multiple times over due to a repeated common need . . .).

Was your solution capable of handling nesting elegantly? Also, is it capable of handling early rendering (like via iife and umd)? I solve that, but found it to be particularly tricky since slot contents are not only unknown during initial parsing (even though the component is constructed and connectedCallback() is still fired while parsing its contents) but also ensuring that we can re-render again consistently with your original slots as the contents of parents re-render with their slots.

Basically my solutions these various issues were:

  1. During rendering, ensure the Svelte component is placed in a special area (in this case a noop custom element called <svelte-retag>) so we can avoid it when we need to pull out default slot content
  2. Ensure that if the component gets disconnected, we clean up and re-insert our previously parsed slots. This was huge in resolving nested slot issues as content/context was lost (at least in the light DOM) as components render sometimes out of order or re-render (e.g. from the inside out)

There are some other nuances, but those are the broad strokes.

@fallaciousreasoning
Copy link
Contributor Author

fallaciousreasoning commented Jun 15, 2023

Wow, that's quite funny, I wish I'd found svelte-retag 3 months ago 😆. For now our solution is working pretty well for us.

Yeah, not knowing slots during initial parsing is a bit of a pain. I was thinking of adding a step to our build process to extract them, but for now I just destroy and recreate the Svelte component. It's not particularly efficient but most of our components are leaf nodes, and for now it seems to work (but it means I have to keep track of all the events which have been added to the component).

It was a bit of a shock to me that you can't actually change the slots that render inside a Svelte component, like, it seems quite surprising that this doesn't work (in native Svelte):

<MyComponent>
{#if condition}
   <span slot="foo">Bar</span>
{/if}
</MyComponent>

I've definitely run into quite a few sharp edges trying to use Svelte with Web Components, it's been a bit of a painful experience 😆

@patricknelson
Copy link

for now I just destroy and recreate the Svelte component. It's not particularly efficient but most of our components are leaf nodes, and for now it seems to work

Similar situation here. Got me thinking, though... there are certainly some optimizations that could be performed by first queueing the Svelte component [re]render in a requestAnimationFrame(), especially since that always runs prior to paint (when anything actually gets displayed). In my testing, I noticed that when running as a deferred module, it saved from pointless renders that would been eventually disconnected/reconnected anyway. However, the biggest improvements (at least on avoiding wasted rerenders) were in using iife or umd (running early prior to parsing): There were lots of disconnects but also it was able to defer "just in time" during DOM mutations so that it only renders the 1 time prior to repaint, which helped a bunch.

PoC here: patricknelson/svelte-retag@main...experimental-defer-request-animation-frame. Not ready to do a PR yet since this really breaks unit tests which depend on synchronous rendering right now (plus I'd wanna unit test this particular functionality too, of course). 😅 Nice thing about this svelte-retag is that at least all changes are ideally well tested against regressions.

@patricknelson
Copy link

patricknelson commented Jun 16, 2023

It was a bit of a shock to me that you can't actually change the slots that render inside a Svelte component, like, it seems quite surprising that this doesn't work (in native Svelte)

Oh interesting. Well, thankfully it's at least a work in progress, however it seems (per Simon's post here #8304 (comment)) that's probably going to wait until the architectural dust settles for v5 (so if it lands, it'll likely be 4.x or later).

I'm glad when I tested that it wasn't a bug in my code. I'm still new to Svelte (I've studied it for a while but only coded for the past month or two) so I was worried I did something wrong in svelte-retag! Hah. Thankfully I was able to confirm that was an issue in a regular SvelteKit site and then found that issue/PR (#5604 & #8304)

@patricknelson
Copy link

@fallaciousreasoning: Not sure how easy it is for you to give it a shot, but if you're willing: I'm curious to know if it actually saves you some render time. I'll at least setup a draft PR here (fixed the broken unit tests which relied on synchronous rendering).
patricknelson/svelte-retag#12.

Keep an eye on DOMContentLoaded and see if your numbers are any better there once it defers rendering to rAF. Best way to test that is to try svelte-retag@^1.0.2 and then [email protected] (which I published just for you if you're interested in tinkering with it).

@patricknelson
Copy link

p.s. @fallaciousreasoning just setup a demo for you to check out if you want: https://svelte-retag.vercel.app/

@fallaciousreasoning
Copy link
Contributor Author

fallaciousreasoning commented Jun 18, 2023

I'm still new to Svelte (I've studied it for a while but only coded for the past month or two)

Funnily enough me too 😆 I keep thinking I've done something wrong and being surprised when I can repro it in the Svelte repl.

Not sure how easy it is for you to give it a shot, but if you're willing

I'd definitely be interested in experimenting with it (I'm sure svelte-retag is much more performant than my bundle of hacks) but it might be a while till I can get to it (I'm a bit hectic on some other stuff at the moment, and what we have works well enough for now).

Looking at the demo, it seems pretty cool! Do you think it would be possible to avoid the svelte-retag elements which are sprinkled throughout the DOM? It just makes things a little harder to follow in the inspector.

@patricknelson
Copy link

Sorry I missed this!

Do you think it would be possible to avoid the svelte-retag elements which are sprinkled throughout the DOM? It just makes things a little harder to follow in the inspector.

Readability and comprehensibility is key, I do agree! I’m not entirely sure, but totally open to a PR if someone can figure it out.

Right now it’s primary use case is to assist in segmenting default slot content from the rendering of the component itself, as well as serving as a partition/boundary layer against MutationObserver (and any other selectors) where we need to keep an eye on changes in the element while also ignoring changes in the Svelte rendered part of the component. You can see how right now this has been useful both in IIFE/UMD rendering and hydration using a related wrapper (which I’ve also added experimental support for, by the way, potentially making way for SSR of Svelte based web components in the light DOM 😅).

That said, now that I’ve added context support, I switched to using requestAnimationFrame() to defer Svelte component rendering inside the custom elements such that it happens in document order (top down), so this means that some of the utility of MutationObserver and <svelte-retag> may not be necessary; at least at start up. Please submit a PR if you happen to find a solution! Warning: Might require tweaking the unit tests which expect the tag to be there, but that shouldn’t be too difficult.

While I do concede it’s a bit ugly aesthetically, it has proven to be highly functional for now (see demo at https://svelte-retag.vercel.app/). I agree though that the ideal solution wouldn’t require this extra nesting though! It’s at least in my TODO’s in the code.

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

No branches or pull requests

3 participants