Skip to content

fix: implement ref counting for mount with same target#17695

Merged
Rich-Harris merged 5 commits intosveltejs:mainfrom
abdel-17:fix-mount
Feb 13, 2026
Merged

fix: implement ref counting for mount with same target#17695
Rich-Harris merged 5 commits intosveltejs:mainfrom
abdel-17:fix-mount

Conversation

@abdel-17
Copy link
Contributor

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

Fixes #17694

Not really sure how to add a failing test here.

The code was generated by Codex 5.3, but I cleaned it up and manually reviewed it myself. Not sure if this is the best approach to solving the issue, though. It does add some overhead with the extra maps.

@changeset-bot
Copy link

changeset-bot bot commented Feb 13, 2026

🦋 Changeset detected

Latest commit: 23cedac

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

@abdel-17 abdel-17 requested a deployment to Publish pkg.pr.new (external contributors) February 13, 2026 04:55 — with GitHub Actions Waiting
@Rich-Harris Rich-Harris requested a deployment to Publish pkg.pr.new (external contributors) February 13, 2026 14:40 — with GitHub Actions Waiting
Copy link
Member

@Rich-Harris Rich-Harris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you — the fix is correct, though we can reuse the map between target and document so I tweaked that.

Finding it curiously different to create a suitable test for this — the one in #17694 is too complicated to whittle down because it uses bits-ui, and the one in #17492 uses Svelte 4 stuff that will eventually go away (and stops failing if converted to Svelte 5), but a regression seems very unlikely so in this case, 🤷

@Rich-Harris Rich-Harris merged commit 0565dca into sveltejs:main Feb 13, 2026
16 checks passed
@github-actions github-actions bot mentioned this pull request Feb 13, 2026
Rich-Harris pushed a commit that referenced this pull request Feb 13, 2026
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## svelte@5.51.0

### Minor Changes

- feat: Use `TrustedTypes` for HTML handling where supported
([#16271](#16271))

### Patch Changes

- fix: sanitize template-literal-special-characters in SSR attribute
values ([#17692](#17692))

- fix: follow-up formatting in `print()` — flush block-level elements
into separate sequences
([#17699](#17699))

- fix: preserve delegated event handlers as long as one or more root
components are using them
([#17695](#17695))

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@abdel-17
Copy link
Contributor Author

Would it be reasonable to maybe add an internal __property that contains the count map instead as a performance optimization?

@Rich-Harris
Copy link
Member

This isn't hot code — in most apps there'll only be a single mount/hydrate. Different if you're using custom elements but even then I don't think it's expensive enough to make it worthwhile to pollute the element

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.

Unmounting can break event handling

2 participants