sveltejs / kit

web development, streamlined
https://kit.svelte.dev
MIT License
18.6k stars 1.92k forks source link

Permit hydrating on custom node #10411

Open samjacoby opened 1 year ago

samjacoby commented 1 year ago

Describe the problem

We've struggled for some time with the challenge of embedding sveltekit "interactives" within the React ecosystem of The New York Times. This is of particular issue on our homepage (https://www.nytimes.com/) where svelte-kit apps are embedded like so many raisins in the pudding, and mounting them to the wrong node can blow out the rest of the page DOM.

There's a fair amount of fancy-footwork going on on the homepage, so we don't always have control over where scripts are mounted on the page (they are sometimes moved around for various performance reasons).

There have been some solutions— via https://github.com/sveltejs/kit/issues/4685 and https://github.com/sveltejs/kit/issues/4685 —which have worked in the past, but unfortunately, those solutions are no longer in more recent vintages of sveltekit. (I believe that the data attribute in question has moved on?)

We're currently stuck in an older vintage of sveltekit , because of this, which is a shame, as there are a number of new features we'd like to take advantage of!

Describe the proposed solution

We'd like to figure out a durable solution for the use-case above; it appears possible that we can update this ourselves via the suggestion here but have had some trouble making that work. A pointer in the right direction, if that's the appropriate avenue, would be helpful!

Alternatives considered

No response

Importance

i cannot use SvelteKit without it

Additional Information

No response

dummdidumm commented 1 year ago

The target is now set like this: https://github.com/sveltejs/kit/blob/b251b235b321b9c75236429cc3a1ac2415475775/packages/kit/src/runtime/server/page/render.js#L322

It's probably possible to replace that string inside transformPageChunk in such a way that it points to a specific element instead, for example by giving the <div> that surrounds %sveltekit.body% in app.html a unique ID and then doing html.replace('const element = document.currentScript.parentElement;', 'const element = document.getElementById('__embedded_kit_app__')inside transformPageChunk.

Is that workaround enough of a solution for you?

samjacoby commented 1 year ago

Thanks for the pointer; this does it for us!

Not to keep pressing my luck — but is there a way to integrate that pattern into a vite plugin?

I was looking into transformIndexHtml, which appears to not be supported? We'd like to minimize modifying projects (each interactive piece is its own sveltekit application, so needs this transformPageChunk override). We can abstract the pieces away, but it'd be nice to encapsulate it entirely outside of projects.

Rich-Harris commented 1 year ago

I suppose one option would be to reinstate the long-since-removed target option in svelte.config.js, and defaulting it to document.currentScript.parentElement? i.e. you'd have this in your app.html...

<div id="<project-slug>">%sveltekit.body%</div>

...and this in your svelte.config.js...

export default {
  kit: {
    target: '#<project-slug>'
  }
};

...and it would render this, which ought to be resistant to any hoisting shenanigans (as long as the element exists in the DOM before the script runs):

<div id="<project-slug>">
  <!-- rendered html -->

  <script>
    const element = document.querySelector('#<project-slug>');
    // ...
  </script>
</div>

I can't think of a good way that doesn't involve adding config (other than the transformPageChunk hack). In a way it's still 'inside' the project since the app template and the config both need to be aware of the project slug, but presumably that's something you can do when the project is first scaffolded?

samjacoby commented 1 year ago

I suppose one option would be to reinstate the long-since-removed target option in svelte.config.js, and defaulting it to document.currentScript.parentElement? i.e. you'd have this in your app.html...

That would certainly meet our needs, and would make me feel generally safer all around. We're already doing something like the below, to populate the app.html with the project slug (originally for top-level CSS scoping), so it'd be a straightforward addition.

<div
    id="%sveltekit.env.PROJECT_SLUG%"
>
    %sveltekit.body%
</div>