Skip to content

Comments

fix: patch additional methods so React doesnt break with template elements#9385

Open
LFDanLu wants to merge 4 commits intomainfrom
9376
Open

fix: patch additional methods so React doesnt break with template elements#9385
LFDanLu wants to merge 4 commits intomainfrom
9376

Conversation

@LFDanLu
Copy link
Member

@LFDanLu LFDanLu commented Dec 19, 2025

Closes #9376

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

Go to S2 Picker docs. Type any description in the first example's description control, it should apply without crashing

🧢 Your Project:

RSP

@LFDanLu LFDanLu marked this pull request as ready for review December 19, 2025 18:50
@rspbot
Copy link

rspbot commented Dec 19, 2025

snowystinger
snowystinger previously approved these changes Dec 21, 2025
@snowystinger
Copy link
Member

Is a test possible for this one?

@LFDanLu
Copy link
Member Author

LFDanLu commented Dec 22, 2025

I tried to make one but couldn't get it to fail in the SSR tests without the change unfortunately

Copy link
Member

@reidbarber reidbarber left a comment

Choose a reason for hiding this comment

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

I still see the issue if I navigate to another page first, then the Picker page. Doesn't happen when just loading the Picker page first though.

@LFDanLu
Copy link
Member Author

LFDanLu commented Dec 22, 2025

oh thats bizzare, didn't expect that flow to break, I'll dig

@rspbot
Copy link

rspbot commented Dec 22, 2025

enumerable: true,
value: function (node, child) {
if (this.dataset.reactAriaHidden) {
// child might not exist in this.content for some reason (stale?), add to end instead
Copy link
Member

Choose a reason for hiding this comment

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

???

In this case, what is the child and it's parent?

Copy link
Member Author

Choose a reason for hiding this comment

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

image

the child is the bottom most div, at the time of the error this.content is a document fragment with no children hence why it breaks trying to call insertBefore

Copy link
Member

Choose a reason for hiding this comment

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

oh, this seems wrong. <template> doesn't support direct children, they should be within the document fragment. That's what these overridden methods should be doing (since react doesn't handle that). So maybe we're missing another one and that's how those dives are getting there?

Copy link
Member Author

Choose a reason for hiding this comment

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

ok I did some poking around and it seems like the monkey patches about didn't trigger their reactAriaHidden route when switching pages from some arbitrary page to the Picker docs page, hence causing the original error to appear when attempting to add a description via the example controls since we had stray direct children as you noticed above. I'm not 100% sure how that happens since the aria hidden data attribute should always be there...

On the plus side, this actually fixed a couple of tests that actually inadvertently noticed that those direct children were still appearing outside the document fragment.

@rspbot
Copy link

rspbot commented Feb 20, 2026

1 similar comment
@rspbot
Copy link

rspbot commented Feb 20, 2026

@rspbot
Copy link

rspbot commented Feb 20, 2026

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error when entering a description for the Spectrum Picker

5 participants