Skip to content

fix: skip re-attaching shadow roots during view transitions#14737

Merged
delucis merged 3 commits intowithastro:mainfrom
Arecsu:main
Nov 10, 2025
Merged

fix: skip re-attaching shadow roots during view transitions#14737
delucis merged 3 commits intowithastro:mainfrom
Arecsu:main

Conversation

@Arecsu
Copy link
Copy Markdown
Contributor

@Arecsu Arecsu commented Nov 9, 2025

Changes

  • Fixes: Elements with declarative Shadow DOM in transition-persisted containers throw "Unable to re-attach to existing ShadowDOM" error during view transitions
  • The existing attachShadowRoots() function attempts to call attachShadow() on all elements with <template shadowrootmode>, but doesn't check if the element already has a shadow root from a previous page
  • This commonly occurs when using transition:persist on containers with web components that use declarative Shadow DOM (e.g., custom elements, third-party libraries)
  • The fix adds a simple check: if a shadow root already exists, skip re-attachment and just remove the template
  • Follow-up of a recent PR made by @delucis: Attach declarative Shadow DOM templates during view transition #14341

Testing

This happened when fixed hydration mismatches in number-flow/svelte, which is a wrapper around a web component that uses Declarative Shadow DOM. PR: barvian/number-flow#162

The component was inside a transition-persisted flyout in all page-routes.

  • Before: Navigation throws "Unable to re-attach to existing ShadowDOM" error
  • After: Navigation works smoothly, shadow roots preserved correctly

Docs

Not applicable — bug fix for existing view transitions feature.

Prevents 'Unable to re-attach to existing ShadowDOM' error when
transition-persisted elements contain declarative Shadow DOM.
@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Nov 9, 2025

🦋 Changeset detected

Latest commit: a712976

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

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

@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Nov 9, 2025
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Nov 9, 2025

CodSpeed Performance Report

Merging #14737 will not alter performance

Comparing Arecsu:main (a712976) with main (535fa36)1

Summary

✅ 6 untouched

Footnotes

  1. No successful run was found on main (ab34340) during the generation of this report, so 535fa36 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@Arecsu
Copy link
Copy Markdown
Contributor Author

Arecsu commented Nov 9, 2025

If this needs to have a changeset, please let me know. I'm not sure I fully understood if this PR needs it 😅

Copy link
Copy Markdown
Member

@delucis delucis left a comment

Choose a reason for hiding this comment

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

Looks good — thanks for the fix @Arecsu!

Yes, a changeset would be great here. This is a bug fix so it should be a patch to the astro package.

We have more guidance on how to write a changeset here: https://contribute.docs.astro.build/docs-for-code-changes/changesets/

@Arecsu
Copy link
Copy Markdown
Contributor Author

Arecsu commented Nov 10, 2025

Awesome. Done!

Copy link
Copy Markdown
Member

@delucis delucis left a comment

Choose a reason for hiding this comment

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

Perfect — nice work @Arecsu 🙌

@delucis delucis merged commit 74c8852 into withastro:main Nov 10, 2025
26 checks passed
This was referenced Nov 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pkg: astro Related to the core `astro` package (scope)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants