Skip to content

fix: properly hydrate dynamic css props components and remove element removal #16118

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

Merged
merged 1 commit into from
Jun 11, 2025

Conversation

paoloricciuti
Copy link
Member

Closes #16115

Boi this was a rollercoaster. So the original bug was actually fixed just by removing the teardown in css_props...I'm pretty sure that with #11948 it was not needed anymore but stuck around. No test failed when i removed it.

However when I wrote the test for it it was failing to hydrate. When i checked with the same component on main it was still failing hydration. Every-time i fixed that hydration some other test was failing.

The reason is that css_props was used in two different ways depending if it was a SvelteComponent it produced a code like this

{
    $.css_props();
    $.component();
    $.reset();
}

while with a dynamic component was used like this

{
    $.component(..., ()=>{
        $.css_props();
        $$component();
        $.reset();
    });
}

since $.component set the hydration_node doing next_sibling and $.css_props does it by doing first_child the fact that the two were different was problematic.

Initially I thought of moving the logic outside of build_component for both dynamic Component and SvelteComponent since build_component it's already complex but it would've been kind of a mess to change all the rest of the things to satisfy hydration.

A nice thing is that now the Component visitor is much shorter.

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • Prefix your PR title with feat:, fix:, chore:, or docs:.
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.
  • If this PR changes code within packages/svelte/src, add a changeset (npx changeset).

Tests and linting

  • Run the tests with pnpm test and lint the project with pnpm lint

Copy link

changeset-bot bot commented Jun 9, 2025

🦋 Changeset detected

Latest commit: 48bfd81

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
svelte Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

github-actions bot commented Jun 9, 2025

Playground

pnpm add https://pkg.pr.new/svelte@16118

@dummdidumm
Copy link
Member

dummdidumm commented Jun 10, 2025

The one big difference is that the <svelte-css-wrapper> element will always be visible, even if the component is falsy and therefore removed from the DOM. On the other hand, that was always the case during SSR hydration, so I tend to approve.

@paoloricciuti
Copy link
Member Author

The one big difference is that the <svelte-css-wrapper> element will always be visible, even if the component is falsy and therefore removed from the DOM. On the other hand, that was always the case during SSR hydration, so I tend to approve.

But this is also how svelte:component behave so I think unify them is better

@Rich-Harris Rich-Harris merged commit 90807ca into main Jun 11, 2025
14 checks passed
@Rich-Harris Rich-Harris deleted the dynamic-css-props-hydration branch June 11, 2025 15:17
@github-actions github-actions bot mentioned this pull request Jun 11, 2025
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.

Dynamic components with css variables passed does not render when component changes
3 participants