fix: ensure __AddClass triggers style updates when enableCSSSelector …#2515
Conversation
🦋 Changeset detectedLatest commit: 65f6f3c The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
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 |
📝 WalkthroughWalkthroughAdds a conditional behavior fix to Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/web-platform/web-core-e2e/tests/web-core.test.ts (1)
94-95: Remove debug-only console listeners andRESULT::log.
page.on('console', ...)/page.on('pageerror', ...)and theconsole.log('RESULT::', result)look like leftover debugging aids from local iteration. They don't affect assertions but pollute CI output and aren't used by any later assertion (errors aren't collected/asserted). Consider dropping them or, if you want to assert no page errors, push into an array andexpect(errors).toEqual([])like theshould not throw error...test at line 1052.🧹 Suggested cleanup
- page.on('console', msg => console.log('PAGE LOG:', msg.text())); - page.on('pageerror', err => console.log('PAGE ERROR:', err.message)); - await goto(page, 'web-core.disable-css-selector.json'); @@ - console.log('RESULT::', result); expect(result.classes).toContain('my-class');Also applies to: 133-133
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web-platform/web-core-e2e/tests/web-core.test.ts` around lines 94 - 95, Remove the debug-only console listeners and the RESULT:: log: delete the page.on('console', msg => ...) and page.on('pageerror', err => ...) handlers and remove the console.log('RESULT::', result) statement; if you want to assert no page errors instead, replace the page.on('pageerror', ...) handler with code that pushes errors into an errors array (e.g., errors.push(err.message)) and add an assertion like expect(errors).toEqual([]) similar to the existing "should not throw error..." test, keeping the rest of the test flow unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/web-platform/web-core-e2e/tests/web-core.test.ts`:
- Around line 94-95: Remove the debug-only console listeners and the RESULT::
log: delete the page.on('console', msg => ...) and page.on('pageerror', err =>
...) handlers and remove the console.log('RESULT::', result) statement; if you
want to assert no page errors instead, replace the page.on('pageerror', ...)
handler with code that pushes errors into an errors array (e.g.,
errors.push(err.message)) and add an assertion like expect(errors).toEqual([])
similar to the existing "should not throw error..." test, keeping the rest of
the test flow unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c7d53e40-cc73-4730-a430-0f32717d8a5b
📒 Files selected for processing (5)
.changeset/fix-addclass-style.mdpackages/web-platform/web-core-e2e/resources/web-core.disable-css-selector.jsonpackages/web-platform/web-core-e2e/tests/web-core.test.tspackages/web-platform/web-core/ts/client/mainthread/elementAPIs/createElementAPI.tspackages/web-platform/web-core/ts/server/elementAPIs/createElementAPI.ts
Merging this PR will improve performance by 5.07%
Performance Changes
Comparing Footnotes
|
React MTF Example#696 Bundle Size — 196.39KiB (0%).65f6f3c(current) vs 4643248 main#695(baseline) Bundle metrics
|
| Current #696 |
Baseline #695 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
3 |
3 |
|
173 |
173 |
|
66 |
66 |
|
44.07% |
44.07% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #696 |
Baseline #695 |
|
|---|---|---|
111.23KiB |
111.23KiB |
|
85.15KiB |
85.15KiB |
Bundle analysis report Branch PupilTong:p/hw/fix-css-og-add-cl... Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#7564 Bundle Size — 225.23KiB (0%).65f6f3c(current) vs 4643248 main#7563(baseline) Bundle metrics
|
| Current #7564 |
Baseline #7563 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
4 |
4 |
|
179 |
179 |
|
69 |
69 |
|
44.57% |
44.57% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #7564 |
Baseline #7563 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
79.47KiB |
79.47KiB |
Bundle analysis report Branch PupilTong:p/hw/fix-css-og-add-cl... Project dashboard
Generated by RelativeCI Documentation Report issue
React External#682 Bundle Size — 679.93KiB (0%).65f6f3c(current) vs 4643248 main#681(baseline) Bundle metrics
|
| Current #682 |
Baseline #681 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
3 |
3 |
|
17 |
17 |
|
5 |
5 |
|
8.59% |
8.59% |
|
0 |
0 |
|
0 |
0 |
Bundle analysis report Branch PupilTong:p/hw/fix-css-og-add-cl... Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#9136 Bundle Size — 900.2KiB (+0.01%).65f6f3c(current) vs 4643248 main#9135(baseline) Bundle metrics
Bundle size by type
Bundle analysis report Branch PupilTong:p/hw/fix-css-og-add-cl... Project dashboard Generated by RelativeCI Documentation Report issue |
…is disabled
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests
Checklist