Skip to content

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

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

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

Conversation

@Arecsu

@Arecsu Arecsu commented Nov 9, 2025

Copy link
Copy Markdown
Contributor

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

changeset-bot Bot commented Nov 9, 2025

Copy link
Copy Markdown

🦋 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

codspeed-hq Bot commented Nov 9, 2025

Copy link
Copy Markdown

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

Arecsu commented Nov 9, 2025

Copy link
Copy Markdown
Contributor Author

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

@delucis delucis left a comment

Copy link
Copy Markdown
Member

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

Arecsu commented Nov 10, 2025

Copy link
Copy Markdown
Contributor Author

Awesome. Done!

@delucis delucis left a comment

Copy link
Copy Markdown
Member

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