fix: distinguish between cssID and componentCSSID in element dat…#2495
fix: distinguish between cssID and componentCSSID in element dat…#2495PupilTong merged 2 commits intolynx-family:mainfrom
Conversation
…a and APIs to support correct style inheritance
🦋 Changeset detectedLatest commit: 4e61803 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 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughFixes component CSS ID handling: component-level CSS ID is now stored on elements and derived from the parent component for SSR and main-thread creation; TypeScript APIs and WASM bindings updated and tests added. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/web-platform/web-core/tests/element-apis.spec.ts (1)
1742-1775:⚠️ Potential issue | 🟡 MinorTest has no assertions.
create element wont infer css id if parent css id is 0 in SSRbuilds the tree and callswasmCtx.generate_html(viewUid)but never asserts anything on the result, so it silently passes regardless of behavior. Add an assertion mirroring the client-side counterpart (e.g., that the rendered HTML does not containl-css-id=).Proposed fix
const viewUid = api.__GetElementUniqueID(view); const html = wasmCtx.generate_html(viewUid); + expect(html).not.toContain('l-css-id='); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web-platform/web-core/tests/element-apis.spec.ts` around lines 1742 - 1775, The test "create element wont infer css id if parent css id is 0 in SSR" builds a tree and calls wasmCtx.generate_html(viewUid) but has no assertion; add an assertion that the generated HTML (result of wasmCtx.generate_html(viewUid)) does NOT contain the css id attribute (e.g., ensure the string 'l-css-id=' is absent). Update the test using the existing variables (createServerElementAPI, wasmCtx, viewUid) to capture the html and assert html.includes('l-css-id=') is false (or use your test framework's negative assertion helper).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/web-platform/web-core/ts/types/IElementPAPI.ts`:
- Around line 179-184: The __UpdateComponentInfo flow updates cssID by calling
__SetCSSId which currently only sets element_data.css_id; you need to also
update the element's component_css_id when the element is a component so future
children inherit the new value. Locate the __SetCSSId / set_css_id
implementation and add logic to detect component elements (the same condition
used elsewhere for components) and assign the new css id to component_css_id as
well as element_data.css_id; ensure the update occurs atomically with the
existing assignment to avoid divergence between element_data.css_id and
component_css_id.
---
Outside diff comments:
In `@packages/web-platform/web-core/tests/element-apis.spec.ts`:
- Around line 1742-1775: The test "create element wont infer css id if parent
css id is 0 in SSR" builds a tree and calls wasmCtx.generate_html(viewUid) but
has no assertion; add an assertion that the generated HTML (result of
wasmCtx.generate_html(viewUid)) does NOT contain the css id attribute (e.g.,
ensure the string 'l-css-id=' is absent). Update the test using the existing
variables (createServerElementAPI, wasmCtx, viewUid) to capture the html and
assert html.includes('l-css-id=') is false (or use your test framework's
negative assertion helper).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9914ee5e-8e71-46d0-9980-5f15343c21f6
📒 Files selected for processing (8)
.changeset/fix-component-cssid.mdpackages/web-platform/web-core/src/main_thread/client/main_thread_context.rspackages/web-platform/web-core/src/main_thread/element_data.rspackages/web-platform/web-core/src/main_thread/server/main_thread_server_context.rspackages/web-platform/web-core/tests/element-apis.spec.tspackages/web-platform/web-core/ts/client/mainthread/elementAPIs/createElementAPI.tspackages/web-platform/web-core/ts/server/elementAPIs/createElementAPI.tspackages/web-platform/web-core/ts/types/IElementPAPI.ts
Merging this PR will degrade performance by 16.15%
Performance Changes
Comparing Footnotes
|
React Example#7462 Bundle Size — 224.41KiB (0%).4e61803(current) vs 02be891 main#7453(baseline) Bundle metrics
|
| Current #7462 |
Baseline #7453 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
4 |
4 |
|
179 |
179 |
|
69 |
69 |
|
44.5% |
44.5% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #7462 |
Baseline #7453 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
78.65KiB |
78.65KiB |
Bundle analysis report Branch PupilTong:p/hw/patch-component-c... Project dashboard
Generated by RelativeCI Documentation Report issue
React External#580 Bundle Size — 583.27KiB (0%).4e61803(current) vs 02be891 main#571(baseline) Bundle metrics
|
| Current #580 |
Baseline #571 |
|
|---|---|---|
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/patch-component-c... Project dashboard
Generated by RelativeCI Documentation Report issue
React MTF Example#595 Bundle Size — 195.57KiB (0%).4e61803(current) vs 02be891 main#586(baseline) Bundle metrics
|
| Current #595 |
Baseline #586 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
3 |
3 |
|
173 |
173 |
|
66 |
66 |
|
43.99% |
43.99% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #595 |
Baseline #586 |
|
|---|---|---|
111.23KiB |
111.23KiB |
|
84.34KiB |
84.34KiB |
Bundle analysis report Branch PupilTong:p/hw/patch-component-c... Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#9036 Bundle Size — 898.69KiB (+0.06%).4e61803(current) vs 02be891 main#9028(baseline) Bundle metrics
Bundle size by type
Bundle analysis report Branch PupilTong:p/hw/patch-component-c... Project dashboard Generated by RelativeCI Documentation Report issue |
…a and APIs to support correct style inheritance
Summary by CodeRabbit
Bug Fixes
Tests
Checklist