test(query-devtools/utils): add tests for 'setupStyleSheet'#10649
Merged
test(query-devtools/utils): add tests for 'setupStyleSheet'#10649
Conversation
Contributor
📝 WalkthroughWalkthroughAdds an exported utility ChangesStylesheet Setup Utility
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
View your CI Pipeline Execution ↗ for commit 6f595f0
☁️ Nx Cloud last updated this comment at |
Contributor
🚀 Changeset Version PreviewNo changeset entries found. Merging this PR will not cause a version bump for any packages. |
Contributor
size-limit report 📦
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/query-devtools/src/__tests__/utils.test.ts`:
- Around line 928-948: Add a regression test that ensures a pre-existing head
style tag does not block insertion into a provided ShadowRoot: create and append
a style element with id "_goober" and a known nonce to document.head, then
create a host with attachShadow and call setupStyleSheet('test-nonce', shadow);
assert that shadow.querySelector('#_goober') is not null (and has the expected
nonce if relevant) and that document.head still contains the original style (so
insertion happened into the shadow target despite the head entry). Also add a
variant that calls setupStyleSheet twice (first seeding head, then calling
setupStyleSheet on the same shadow) to ensure no duplicate style is added inside
the ShadowRoot while preserving the head tag; reference setupStyleSheet and the
internal style id "_goober" (and styleExists in utils.tsx) when locating the
behavior to test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1a3250a0-c6ba-45e6-a060-39cef711560a
📒 Files selected for processing (1)
packages/query-devtools/src/__tests__/utils.test.ts
Comment on lines
+928
to
+948
| it('should append the style tag to the provided "ShadowRoot" target', () => { | ||
| const host = document.createElement('div') | ||
| const shadow = host.attachShadow({ mode: 'open' }) | ||
|
|
||
| setupStyleSheet('test-nonce', shadow) | ||
|
|
||
| expect(shadow.querySelector('#_goober')).not.toBeNull() | ||
| expect(document.head.querySelector('#_goober')).toBeNull() | ||
| }) | ||
|
|
||
| it('should not insert a duplicate style tag when the target already has one', () => { | ||
| const host = document.createElement('div') | ||
| const shadow = host.attachShadow({ mode: 'open' }) | ||
|
|
||
| setupStyleSheet('first-nonce', shadow) | ||
| setupStyleSheet('second-nonce', shadow) | ||
|
|
||
| const styleTags = shadow.querySelectorAll('#_goober') | ||
| expect(styleTags).toHaveLength(1) | ||
| expect(styleTags[0]?.getAttribute('nonce')).toBe('first-nonce') | ||
| }) |
Contributor
There was a problem hiding this comment.
Missing regression test for head-vs-shadow precedence bug.
On Line 932/Line 942, ShadowRoot behavior is only tested when document.head is clean. In packages/query-devtools/src/utils.tsx (Lines 305-323), styleExists checks document.querySelector('#_goober') first, so an existing head tag can incorrectly block insertion into the provided ShadowRoot target. Please add a test that seeds document.head first, then asserts target insertion still occurs.
Suggested test addition
it('should not insert a duplicate style tag when the target already has one', () => {
const host = document.createElement('div')
const shadow = host.attachShadow({ mode: 'open' })
setupStyleSheet('first-nonce', shadow)
setupStyleSheet('second-nonce', shadow)
const styleTags = shadow.querySelectorAll('#_goober')
expect(styleTags).toHaveLength(1)
expect(styleTags[0]?.getAttribute('nonce')).toBe('first-nonce')
})
+
+ it('should still append to ShadowRoot when document.head already has "_goober"', () => {
+ setupStyleSheet('head-nonce')
+ const host = document.createElement('div')
+ const shadow = host.attachShadow({ mode: 'open' })
+
+ setupStyleSheet('shadow-nonce', shadow)
+
+ expect(document.head.querySelectorAll('#_goober')).toHaveLength(1)
+ expect(shadow.querySelectorAll('#_goober')).toHaveLength(1)
+ expect(shadow.querySelector('#_goober')?.getAttribute('nonce')).toBe(
+ 'shadow-nonce',
+ )
+ })
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it('should append the style tag to the provided "ShadowRoot" target', () => { | |
| const host = document.createElement('div') | |
| const shadow = host.attachShadow({ mode: 'open' }) | |
| setupStyleSheet('test-nonce', shadow) | |
| expect(shadow.querySelector('#_goober')).not.toBeNull() | |
| expect(document.head.querySelector('#_goober')).toBeNull() | |
| }) | |
| it('should not insert a duplicate style tag when the target already has one', () => { | |
| const host = document.createElement('div') | |
| const shadow = host.attachShadow({ mode: 'open' }) | |
| setupStyleSheet('first-nonce', shadow) | |
| setupStyleSheet('second-nonce', shadow) | |
| const styleTags = shadow.querySelectorAll('#_goober') | |
| expect(styleTags).toHaveLength(1) | |
| expect(styleTags[0]?.getAttribute('nonce')).toBe('first-nonce') | |
| }) | |
| it('should append the style tag to the provided "ShadowRoot" target', () => { | |
| const host = document.createElement('div') | |
| const shadow = host.attachShadow({ mode: 'open' }) | |
| setupStyleSheet('test-nonce', shadow) | |
| expect(shadow.querySelector('#_goober')).not.toBeNull() | |
| expect(document.head.querySelector('#_goober')).toBeNull() | |
| }) | |
| it('should not insert a duplicate style tag when the target already has one', () => { | |
| const host = document.createElement('div') | |
| const shadow = host.attachShadow({ mode: 'open' }) | |
| setupStyleSheet('first-nonce', shadow) | |
| setupStyleSheet('second-nonce', shadow) | |
| const styleTags = shadow.querySelectorAll('#_goober') | |
| expect(styleTags).toHaveLength(1) | |
| expect(styleTags[0]?.getAttribute('nonce')).toBe('first-nonce') | |
| }) | |
| it('should still append to ShadowRoot when document.head already has "_goober"', () => { | |
| setupStyleSheet('head-nonce') | |
| const host = document.createElement('div') | |
| const shadow = host.attachShadow({ mode: 'open' }) | |
| setupStyleSheet('shadow-nonce', shadow) | |
| expect(document.head.querySelectorAll('#_goober')).toHaveLength(1) | |
| expect(shadow.querySelectorAll('#_goober')).toHaveLength(1) | |
| expect(shadow.querySelector('#_goober')?.getAttribute('nonce')).toBe( | |
| 'shadow-nonce', | |
| ) | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/query-devtools/src/__tests__/utils.test.ts` around lines 928 - 948,
Add a regression test that ensures a pre-existing head style tag does not block
insertion into a provided ShadowRoot: create and append a style element with id
"_goober" and a known nonce to document.head, then create a host with
attachShadow and call setupStyleSheet('test-nonce', shadow); assert that
shadow.querySelector('#_goober') is not null (and has the expected nonce if
relevant) and that document.head still contains the original style (so insertion
happened into the shadow target despite the head entry). Also add a variant that
calls setupStyleSheet twice (first seeding head, then calling setupStyleSheet on
the same shadow) to ensure no duplicate style is added inside the ShadowRoot
while preserving the head tag; reference setupStyleSheet and the internal style
id "_goober" (and styleExists in utils.tsx) when locating the behavior to test.
sukvvon
force-pushed
the
test/query-devtools-setup-stylesheet
branch
from
1fe4036 to
3bfa156
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/query-devtools/src/__tests__/utils.test.ts (1)
956-964:⚠️ Potential issue | 🟠 Major | ⚡ Quick winShadowRoot precedence test is asserting the wrong contract.
On Line 956, this test currently expects a provided
ShadowRoottarget to be skipped ifdocument.headalready has#_goober. That locks in incorrect behavior: target-scoped insertion should still happen for the provided shadow root, while preserving the head tag.Suggested test fix
- it('should not insert into the target when "document.head" already has a style tag', () => { + it('should still insert into the target when "document.head" already has a style tag', () => { const host = document.createElement('div') const shadow = host.attachShadow({ mode: 'open' }) setupStyleSheet('first-nonce') setupStyleSheet('second-nonce', shadow) - expect(shadow.querySelector('#_goober')).toBeNull() + expect(document.head.querySelectorAll('#_goober')).toHaveLength(1) + expect(document.head.querySelector('#_goober')?.getAttribute('nonce')).toBe('first-nonce') + expect(shadow.querySelectorAll('#_goober')).toHaveLength(1) + expect(shadow.querySelector('#_goober')?.getAttribute('nonce')).toBe('second-nonce') })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/query-devtools/src/__tests__/utils.test.ts` around lines 956 - 964, The test using setupStyleSheet('first-nonce') and setupStyleSheet('second-nonce', shadow) incorrectly asserts that the shadow target is skipped when document.head already has `#_goober`; instead, change the assertion to verify that a head-scoped stylesheet remains and that a target-scoped stylesheet was also inserted into the provided ShadowRoot: assert that document.head.querySelector('#_goober') is not null and that shadow.querySelector('#_goober') is not null (i.e., ensure both the head and the shadow root contain their own `#_goober` elements after calling setupStyleSheet with and without the ShadowRoot).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@packages/query-devtools/src/__tests__/utils.test.ts`:
- Around line 956-964: The test using setupStyleSheet('first-nonce') and
setupStyleSheet('second-nonce', shadow) incorrectly asserts that the shadow
target is skipped when document.head already has `#_goober`; instead, change the
assertion to verify that a head-scoped stylesheet remains and that a
target-scoped stylesheet was also inserted into the provided ShadowRoot: assert
that document.head.querySelector('#_goober') is not null and that
shadow.querySelector('#_goober') is not null (i.e., ensure both the head and the
shadow root contain their own `#_goober` elements after calling setupStyleSheet
with and without the ShadowRoot).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c28b3e49-93ec-44da-ba28-adae74fdf0f7
📒 Files selected for processing (1)
packages/query-devtools/src/__tests__/utils.test.ts
sukvvon
force-pushed
the
test/query-devtools-setup-stylesheet
branch
from
3bfa156 to
6f595f0
Compare
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
1 participant
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.
🎯 Changes
Add unit tests for the
setupStyleSheethelper inquery-devtools/utils.tsx.The helper inserts a
<style id="_goober">tag with the givennonceattribute, either intodocument.heador into a providedShadowRoottarget. It is a no-op when nononceis provided, and it does not insert a duplicate tag if one already exists in the destination.Cases:
nonceis missingnonceis an empty string (guards against strict=== undefinedregressions)<style id="_goober">todocument.headnonceattribute on the inserted style tagdocument.headalready has oneShadowRoottarget instead ofdocument.headThe shared
<style>tag indocument.headis removed inafterEachto keep tests isolated.The cross-target dedup behavior (whether a
document.headtag should block insertion into a provided ShadowRoot target) is intentionally left unasserted while its intent is being clarified separately.✅ Checklist
pnpm run test:pr.🚀 Release Impact