chore: faster node contains paths#9536
Merged
snowystinger merged 6 commits intomainfrom Feb 9, 2026
Merged
Conversation
|
Build successful! 🎉 |
|
Build successful! 🎉 |
|
Build successful! 🎉 |
# Conflicts: # eslint.config.mjs # packages/@react-aria/utils/src/scrollIntoView.ts # packages/@react-spectrum/menu/src/ContextualHelpTrigger.tsx
|
Build successful! 🎉 |
devongovett
previously approved these changes
Feb 6, 2026
|
Build successful! 🎉 |
## API Changes
@react-aria/utils/@react-aria/utils:isFocusWithin+isFocusWithin {
+ node: Element | null | undefined
+ returnVal: undefined
+} |
reidbarber
previously approved these changes
Feb 9, 2026
# Conflicts: # packages/@react-spectrum/menu/src/ContextualHelpTrigger.tsx # packages/@react-spectrum/menu/src/SubmenuTrigger.tsx # packages/@react-spectrum/menu/src/useOverlayPosition.ts
snowystinger
dismissed stale reviews from reidbarber and devongovett
via
ad624a8
reidbarber
approved these changes
Feb 9, 2026
LFDanLu
approved these changes
Feb 9, 2026
|
Build successful! 🎉 |
## API Changes
@react-aria/utils/@react-aria/utils:isFocusWithin+isFocusWithin {
+ node: Element | null | undefined
+ returnVal: undefined
+} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
From some discussions with @devongovett about performance
There's also a side benefit
matches(:'focus-within')works with closed shadow roots, such as<video>.Noted in my previous PR, there are some alternatives to node.contains which may be faster and work with shadow dom. I've added eslint rules to help ensure that we always chose the appropriate implementation
There's a patch in here because nwsapi has a bug, I've submitted an issue here: dperini/nwsapi#158
Whenever we update jsdom (we're on 20, it's on 27), jsdom changed to a different nwsapi that's a fork, that fork still has the same problem, hopefully it'll get automatically merged upstream.
IMPORTANT
I'm worried about this PR because anyone who has similar tests to us in jsdom would run into this problem. This is more likely when using our test-utils because they traverse similar code paths to us. So downstream users would need to apply this patch as well to nwsapi.
I wrote some tests in my reproduction repo linked in that issue which validates that my patch fixes the issue so it works like browsers do.
If anyone is interested, i have perf tests for this as well
perf-test.zip
Results are promising:
Especially following https://en.wikipedia.org/wiki/Usage_share_of_web_browsers
Chrome:
Safari:
Firefox:
✅ Pull Request Checklist:
📝 Test Instructions:
🧢 Your Project: