feat: add support for rem unit transformation across style processing and decoding pipelines#2403
Conversation
🦋 Changeset detectedLatest commit: 6cbe043 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 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a boolean Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/web-platform/web-core/ts/server/deploy.ts (1)
10-19:⚠️ Potential issue | 🟠 MajorExternal callers of
executeTemplatemust be updated to include the newtransformREMparameter.The function signature requires seven boolean/object parameters plus an optional viewAttributes parameter. The following callers are passing incomplete arguments and will cause TypeScript compilation errors:
packages/web-platform/web-core-e2e/shell-project/devMiddleware.ts(lines 61-69): passes only 7 arguments withviewAttributesin the wrong position (should be 8th, optional parameter); missingtransformREMas the 7th argumentpackages/web-platform/web-core-e2e/server-tests/server-e2e.test.ts(lines 14-19): passes only 4 arguments; missingtransformVW,transformVH, andtransformREMpackages/web-platform/web-core-e2e/bench/server.bench.vitest.spec.ts(lines 45-50): passes only 4 arguments; missingtransformVW,transformVH, andtransformREM🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web-platform/web-core/ts/server/deploy.ts` around lines 10 - 19, The executeTemplate signature now adds a transformREM boolean before viewAttributes, so update all external callers to pass the three transform flags in the correct order: transformVW, transformVH, transformREM (booleans) as the 6th–8th arguments respectively, and ensure viewAttributes remains the optional final argument; specifically, revise invocations of executeTemplate that currently supply fewer arguments or put viewAttributes in the 7th position to instead supply the missing transformVW/transformVH/transformREM values (or appropriate defaults) and move viewAttributes to the final parameter position so TypeScript compiles.packages/web-platform/web-core/ts/client/mainthread/TemplateManager.ts (1)
47-56:⚠️ Potential issue | 🟠 MajorCache lookup is not config-aware for transform flags.
At Line 55, cache reuse is keyed only by
url. If two views request the same URL with differenttransformVW/transformVH/transformREMsettings, the second view can receive a bundle decoded with the wrong transform configuration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web-platform/web-core/ts/client/mainthread/TemplateManager.ts` around lines 47 - 56, The cache check in fetchBundle uses only url (this.#bundles) so bundles fetched with different transform flags (transformVW, transformVH, transformREM) or overrideConfig can be reused incorrectly; update fetchBundle to make the cache key config-aware (e.g., derive a stable key from url plus transformVW/transformVH/transformREM and a deterministic representation of overrideConfig) or change `#bundles` to map url -> Map<configKey, bundle>; use that key for lookup, storage, and reuse so each unique combination of url and transform/overrideConfig gets the correct decoded bundle.packages/web-platform/web-core/src/main_thread/client/element_apis/style_apis.rs (1)
107-149:⚠️ Potential issue | 🟠 MajorAdd required JS-side tests for this main-thread API change.
This file is under
src/main_thread, but the PR does not include corresponding updates inpackages/web-platform/web-core/tests/element-apis.spec.tsfor the newtransform_rembehavior (both string and key-value inline style paths).As per coding guidelines "
packages/web-platform/web-core/src/main_thread/**/*.rs: When modifyingsrc/main_thread, ALWAYS add corresponding tests intests/element-apis.spec.tsto verify the JS-side behavior."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web-platform/web-core/src/main_thread/client/element_apis/style_apis.rs` around lines 107 - 149, Add JS tests in packages/web-platform/web-core/tests/element-apis.spec.ts that exercise the new transform_rem behavior for both main-thread APIs: call set_inline_styles_in_str and set_inline_styles_in_key_value_vec (via the wasm bindings or exported client API used in other tests), apply a style containing rem units, set transform_rem true and false, and assert the DOM element's style attribute is transformed (or unchanged) as expected; ensure you cover both the string input path (set_inline_styles_in_str) and the key-value vector path (set_inline_styles_in_key_value_vec) and mirror the structure and assertions used by existing tests in that file for vw/vh transforms.packages/web-platform/web-core/src/template/template_sections/style_info/style_info_decoder.rs (1)
38-45:⚠️ Potential issue | 🔴 CriticalUpdate
StyleInfoDecoder::newcall sites to include the missingtransform_remargument.Constructor now requires 6 arguments, but calls at lines 919 and 970 still pass only 5. This breaks builds with
feature = "encode"enabled.Fix
- let decoder = StyleInfoDecoder::new(decoded_raw, None, true, false, false) + let decoder = StyleInfoDecoder::new(decoded_raw, None, true, false, false, false) .expect("StyleInfoDecoder should succeed");Apply to both lines 919 and 970.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web-platform/web-core/src/template/template_sections/style_info/style_info_decoder.rs` around lines 38 - 45, StyleInfoDecoder::new signature now requires a sixth boolean parameter transform_rem, so update all call sites that pass only five arguments to include the transform_rem value; locate the calls to StyleInfoDecoder::new (the two places that currently pass raw_style_info, entry_name, config_enable_css_selector, transform_vw, transform_vh) and add the missing transform_rem boolean (either the existing local transform_rem variable or the appropriate literal) as the final argument so the constructor is invoked with all six parameters.
🧹 Nitpick comments (3)
packages/web-platform/web-core/src/style_transformer/inline_style.rs (1)
30-34: REM transformation flag correctly threaded through inline style transformers.Both
transform_inline_style_stringandtransform_inline_style_key_value_vecnow forwardconfig.transform_remto theTransformerConfig, enabling REM unit transformation when the flag is set. This integrates with the existing token transformation logic intoken_transformer.rsthat convertsremvalues tocalc(<value> * var(--rem-unit)).Consider adding a test case for REM transformation in inline styles, similar to the existing
rpx_unit_in_key_value_vectest at line 297:#[test] fn rem_unit_in_inline_style_string() { let result = transform_inline_style_string( "height:2rem;", &TransformerConfig { transform_rem: true, ..Default::default() }, ); assert_eq!(result, "height:calc(2 * var(--rem-unit));"); }Based on learnings: "CSS Processing in web-core should use high-performance tokenization and transformation of Lynx-specific CSS (e.g.,
rpxunits,linearlayout) into standard Web CSS"Also applies to: 50-54
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web-platform/web-core/src/style_transformer/inline_style.rs` around lines 30 - 34, Add tests that verify REM transformation is applied when TransformerConfig.transform_rem is true: create a unit test rem_unit_in_inline_style_string that calls transform_inline_style_string("height:2rem;", &TransformerConfig { transform_rem: true, ..Default::default() }) and asserts it returns "height:calc(2 * var(--rem-unit));", and likewise add a rem_unit_in_key_value_vec test mirroring the existing rpx_unit_in_key_value_vec that uses transform_inline_style_key_value_vec with transform_rem enabled and checks the converted value; use the existing rpx tests as a template and reference transform_inline_style_string, transform_inline_style_key_value_vec, and TransformerConfig when adding these tests.packages/web-platform/web-core/src/main_thread/server/main_thread_server_context.rs (1)
527-574: Consider adding a test case that validates REM transformation in server context.The existing tests correctly pass the new parameter but don't verify the REM transformation behavior. Per coding guidelines, modifications to
src/main_threadshould have corresponding tests to verify JS-side behavior.🧪 Example test case for REM transformation
#[test] fn test_transform_rem() { let mut ctx = MainThreadServerContext::new("".to_string(), true, false, false, true); let div_id = ctx.create_element("div".to_string(), None, None, None); ctx.set_attribute(div_id, "style".to_string(), "font-size: 1.5rem;".to_string()); let html = ctx.generate_html(div_id); assert!(html.contains("calc(1.5 * var(--rem-unit))")); }As per coding guidelines: "When modifying
src/main_thread, ALWAYS add corresponding tests intests/element-apis.spec.tsto verify the JS-side behavior."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web-platform/web-core/src/main_thread/server/main_thread_server_context.rs` around lines 527 - 574, Add a unit test that covers REM transformation by constructing a MainThreadServerContext with the REM flag enabled via MainThreadServerContext::new(..., true/..., true) (the fifth boolean in the constructor), creating an element (create_element), setting a style containing a rem value via set_attribute or add_inline_style_raw_string_key, calling generate_html on the element id, and asserting the produced HTML contains the expected transformed expression (e.g., "calc(... var(--rem-unit))"); place this test alongside the existing tests in the same tests module so rem behavior is validated on the server side.packages/web-platform/web-core/tests/template-manager.spec.ts (1)
85-101: Consider adding test coverage for REM transformation.The existing tests correctly add the new
transformREMparameter but always passfalse. Consider adding a test case that verifies REM transformation actually works when enabled (e.g., a style withremunits gets transformed tocalc(... * var(--rem-unit))).Would you like me to generate a test case that validates the REM transformation behavior?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web-platform/web-core/tests/template-manager.spec.ts` around lines 85 - 101, Add a test that passes transformREM=true to templateManager.fetchBundle (replace the appropriate false argument in the call to fetchBundle) and assert that style rules with rem units are transformed into the calc(... * var(--rem-unit)) form; use the same pattern as the existing test (call fetchBundle, retrieve bundle via templateManager.getBundle(templateUrl)?.customSections, decode with TextDecoder) and compare the transformed CSS against an expected value derived from sampleTasm (or add a small sampleTasm entry containing a rem-based style and its expected transformed output) to validate REM transformation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@packages/web-platform/web-core/src/main_thread/client/element_apis/style_apis.rs`:
- Around line 107-149: Add JS tests in
packages/web-platform/web-core/tests/element-apis.spec.ts that exercise the new
transform_rem behavior for both main-thread APIs: call set_inline_styles_in_str
and set_inline_styles_in_key_value_vec (via the wasm bindings or exported client
API used in other tests), apply a style containing rem units, set transform_rem
true and false, and assert the DOM element's style attribute is transformed (or
unchanged) as expected; ensure you cover both the string input path
(set_inline_styles_in_str) and the key-value vector path
(set_inline_styles_in_key_value_vec) and mirror the structure and assertions
used by existing tests in that file for vw/vh transforms.
In
`@packages/web-platform/web-core/src/template/template_sections/style_info/style_info_decoder.rs`:
- Around line 38-45: StyleInfoDecoder::new signature now requires a sixth
boolean parameter transform_rem, so update all call sites that pass only five
arguments to include the transform_rem value; locate the calls to
StyleInfoDecoder::new (the two places that currently pass raw_style_info,
entry_name, config_enable_css_selector, transform_vw, transform_vh) and add the
missing transform_rem boolean (either the existing local transform_rem variable
or the appropriate literal) as the final argument so the constructor is invoked
with all six parameters.
In `@packages/web-platform/web-core/ts/client/mainthread/TemplateManager.ts`:
- Around line 47-56: The cache check in fetchBundle uses only url
(this.#bundles) so bundles fetched with different transform flags (transformVW,
transformVH, transformREM) or overrideConfig can be reused incorrectly; update
fetchBundle to make the cache key config-aware (e.g., derive a stable key from
url plus transformVW/transformVH/transformREM and a deterministic representation
of overrideConfig) or change `#bundles` to map url -> Map<configKey, bundle>; use
that key for lookup, storage, and reuse so each unique combination of url and
transform/overrideConfig gets the correct decoded bundle.
In `@packages/web-platform/web-core/ts/server/deploy.ts`:
- Around line 10-19: The executeTemplate signature now adds a transformREM
boolean before viewAttributes, so update all external callers to pass the three
transform flags in the correct order: transformVW, transformVH, transformREM
(booleans) as the 6th–8th arguments respectively, and ensure viewAttributes
remains the optional final argument; specifically, revise invocations of
executeTemplate that currently supply fewer arguments or put viewAttributes in
the 7th position to instead supply the missing
transformVW/transformVH/transformREM values (or appropriate defaults) and move
viewAttributes to the final parameter position so TypeScript compiles.
---
Nitpick comments:
In
`@packages/web-platform/web-core/src/main_thread/server/main_thread_server_context.rs`:
- Around line 527-574: Add a unit test that covers REM transformation by
constructing a MainThreadServerContext with the REM flag enabled via
MainThreadServerContext::new(..., true/..., true) (the fifth boolean in the
constructor), creating an element (create_element), setting a style containing a
rem value via set_attribute or add_inline_style_raw_string_key, calling
generate_html on the element id, and asserting the produced HTML contains the
expected transformed expression (e.g., "calc(... var(--rem-unit))"); place this
test alongside the existing tests in the same tests module so rem behavior is
validated on the server side.
In `@packages/web-platform/web-core/src/style_transformer/inline_style.rs`:
- Around line 30-34: Add tests that verify REM transformation is applied when
TransformerConfig.transform_rem is true: create a unit test
rem_unit_in_inline_style_string that calls
transform_inline_style_string("height:2rem;", &TransformerConfig {
transform_rem: true, ..Default::default() }) and asserts it returns
"height:calc(2 * var(--rem-unit));", and likewise add a
rem_unit_in_key_value_vec test mirroring the existing rpx_unit_in_key_value_vec
that uses transform_inline_style_key_value_vec with transform_rem enabled and
checks the converted value; use the existing rpx tests as a template and
reference transform_inline_style_string, transform_inline_style_key_value_vec,
and TransformerConfig when adding these tests.
In `@packages/web-platform/web-core/tests/template-manager.spec.ts`:
- Around line 85-101: Add a test that passes transformREM=true to
templateManager.fetchBundle (replace the appropriate false argument in the call
to fetchBundle) and assert that style rules with rem units are transformed into
the calc(... * var(--rem-unit)) form; use the same pattern as the existing test
(call fetchBundle, retrieve bundle via
templateManager.getBundle(templateUrl)?.customSections, decode with TextDecoder)
and compare the transformed CSS against an expected value derived from
sampleTasm (or add a small sampleTasm entry containing a rem-based style and its
expected transformed output) to validate REM transformation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6820edc2-38b9-4180-ac0b-ff8202196a15
📒 Files selected for processing (27)
.changeset/rem-unit-transform.mdpackages/web-platform/web-core/binary/client/client.d.tspackages/web-platform/web-core/binary/client/client_bg.wasm.d.tspackages/web-platform/web-core/binary/client_legacy/client.d.tspackages/web-platform/web-core/binary/client_legacy/client_bg.wasm.d.tspackages/web-platform/web-core/binary/encode/encode.d.tspackages/web-platform/web-core/binary/encode/encode_bg.wasm.d.tspackages/web-platform/web-core/binary/server/server.d.tspackages/web-platform/web-core/binary/server/server_bg.wasm.d.tspackages/web-platform/web-core/src/main_thread/client/element_apis/style_apis.rspackages/web-platform/web-core/src/main_thread/server/main_thread_server_context.rspackages/web-platform/web-core/src/style_transformer/inline_style.rspackages/web-platform/web-core/src/style_transformer/token_transformer.rspackages/web-platform/web-core/src/template/template_sections/style_info/decoded_style_data.rspackages/web-platform/web-core/src/template/template_sections/style_info/raw_style_info.rspackages/web-platform/web-core/src/template/template_sections/style_info/style_info_decoder.rspackages/web-platform/web-core/tests/template-manager.spec.tspackages/web-platform/web-core/ts/client/decodeWorker/cssLoader.tspackages/web-platform/web-core/ts/client/decodeWorker/decode.worker.tspackages/web-platform/web-core/ts/client/decodeWorker/types.tspackages/web-platform/web-core/ts/client/mainthread/LynxView.tspackages/web-platform/web-core/ts/client/mainthread/LynxViewInstance.tspackages/web-platform/web-core/ts/client/mainthread/TemplateManager.tspackages/web-platform/web-core/ts/client/mainthread/elementAPIs/createElementAPI.tspackages/web-platform/web-core/ts/server/decode.tspackages/web-platform/web-core/ts/server/deploy.tspackages/web-platform/web-core/ts/server/elementAPIs/createElementAPI.ts
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Merging this PR will degrade performance by 25.41%
Performance Changes
Comparing Footnotes
|
Web Explorer#8707 Bundle Size — 730.24KiB (+0.19%).6cbe043(current) vs 5f2cea3 main#8686(baseline) Bundle metrics
Bundle size by type
Bundle analysis report Branch PupilTong:p/hw/support-rem-trans... Project dashboard Generated by RelativeCI Documentation Report issue |
React Example#7132 Bundle Size — 236.85KiB (0%).6cbe043(current) vs 5f2cea3 main#7111(baseline) Bundle metrics
|
| Current #7132 |
Baseline #7111 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
4 |
4 |
|
179 |
179 |
|
70 |
70 |
|
46.11% |
46.11% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #7132 |
Baseline #7111 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
91.09KiB |
91.09KiB |
Bundle analysis report Branch PupilTong:p/hw/support-rem-trans... Project dashboard
Generated by RelativeCI Documentation Report issue
React MTF Example#265 Bundle Size — 206.05KiB (0%).6cbe043(current) vs 5f2cea3 main#244(baseline) Bundle metrics
|
| Current #265 |
Baseline #244 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
3 |
3 |
|
173 |
173 |
|
67 |
67 |
|
45.77% |
45.77% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #265 |
Baseline #244 |
|
|---|---|---|
111.23KiB |
111.23KiB |
|
94.81KiB |
94.81KiB |
Bundle analysis report Branch PupilTong:p/hw/support-rem-trans... Project dashboard
Generated by RelativeCI Documentation Report issue
… and decoding pipelines
b952de5 to
4a05801
Compare
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/ts/client/mainthread/TemplateManager.ts (1)
47-89:⚠️ Potential issue | 🟠 MajorCache key must include transform flags (
transformVW/transformVH/transformREM).
At Line 55 and Line 66, cache/load reuse is keyed only byurl. Different flag combinations for the same URL can reuse an incompatible decoded stylesheet/config pipeline result.[suggested fix]
Patch sketch
export class TemplateManager { + `#bundleKey`( + url: string, + transformVW: boolean, + transformVH: boolean, + transformREM: boolean, + overrideConfig?: Record<string, string> | Partial<PageConfig>, + ): string { + const cfg = overrideConfig ? JSON.stringify(overrideConfig) : ''; + return `${url}|vw:${Number(transformVW)}|vh:${Number(transformVH)}|rem:${Number(transformREM)}|cfg:${cfg}`; + } public fetchBundle( url: string, lynxViewInstancePromise: Promise<LynxViewInstance>, transformVW: boolean, transformVH: boolean, transformREM: boolean, overrideConfig?: Record<string, string>, ): Promise<void> { - if (this.#bundles.has(url) && !overrideConfig) { + const key = this.#bundleKey(url, transformVW, transformVH, transformREM, overrideConfig); + if (this.#bundles.has(key) && !overrideConfig) { ... - } else if (this.#loadingPromises.has(url)) { - return this.#loadingPromises.get(url)!.then(async () => { + } else if (this.#loadingPromises.has(key)) { + return this.#loadingPromises.get(key)!.then(async () => { ... }); } else { - this.createBundle(url); + this.createBundle(key); const promise = this.#load( url, lynxViewInstancePromise, transformVW, transformVH, transformREM, overrideConfig, ); - this.#loadingPromises.set(url, promise); + this.#loadingPromises.set(key, promise); return promise; } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web-platform/web-core/ts/client/mainthread/TemplateManager.ts` around lines 47 - 89, fetchBundle currently keys cached entries and loadingPromises only by url, causing incorrect reuse when transformVW/transformVH/transformREM or overrideConfig differ; update the cache key logic in fetchBundle (and where createBundle, `#bundles`, `#loadingPromises`, and `#load` are used) to build a composite key that includes url plus the transform flags and a stable serialization of overrideConfig (e.g., url|vw=true|vh=false|rem=true|overrideKey1=val1...), then use that composite key for has/get/set calls so each unique flag/config combination gets its own bundle/loading promise.
🤖 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/binary/encode/encode_bg.wasm.d.ts`:
- Around line 9-10: All call sites that invoke the updated wasm exports
decode_style_info and encode_legacy_json_generated_raw_style_info are missing
the 7th parameter; update each invocation (the calls in the modules that
reference decode_style_info and encode_legacy_json_generated_raw_style_info) to
pass the new 7th argument required by the wasm signature (do not leave it
undefined). Locate the calls to decode_style_info and
encode_legacy_json_generated_raw_style_info and add the proper seventh value
(the rem-transformation/flags pointer or value your wasm layer expects) by
reusing the existing rem/transform-related variable or computing the value the
same way other wasm calls do in this codebase so REM transformation propagation
is preserved. Ensure the added argument type matches the declared signature
(number) and run the affected code paths after the change.
---
Outside diff comments:
In `@packages/web-platform/web-core/ts/client/mainthread/TemplateManager.ts`:
- Around line 47-89: fetchBundle currently keys cached entries and
loadingPromises only by url, causing incorrect reuse when
transformVW/transformVH/transformREM or overrideConfig differ; update the cache
key logic in fetchBundle (and where createBundle, `#bundles`, `#loadingPromises`,
and `#load` are used) to build a composite key that includes url plus the
transform flags and a stable serialization of overrideConfig (e.g.,
url|vw=true|vh=false|rem=true|overrideKey1=val1...), then use that composite key
for has/get/set calls so each unique flag/config combination gets its own
bundle/loading promise.
🪄 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: 55fd682d-60c1-46e4-b0d4-a59235b2eb74
📒 Files selected for processing (27)
.changeset/rem-unit-transform.mdpackages/web-platform/web-core/binary/client/client.d.tspackages/web-platform/web-core/binary/client/client_bg.wasm.d.tspackages/web-platform/web-core/binary/client_legacy/client.d.tspackages/web-platform/web-core/binary/client_legacy/client_bg.wasm.d.tspackages/web-platform/web-core/binary/encode/encode.d.tspackages/web-platform/web-core/binary/encode/encode_bg.wasm.d.tspackages/web-platform/web-core/binary/server/server.d.tspackages/web-platform/web-core/binary/server/server_bg.wasm.d.tspackages/web-platform/web-core/src/main_thread/client/element_apis/style_apis.rspackages/web-platform/web-core/src/main_thread/server/main_thread_server_context.rspackages/web-platform/web-core/src/style_transformer/inline_style.rspackages/web-platform/web-core/src/style_transformer/token_transformer.rspackages/web-platform/web-core/src/template/template_sections/style_info/decoded_style_data.rspackages/web-platform/web-core/src/template/template_sections/style_info/raw_style_info.rspackages/web-platform/web-core/src/template/template_sections/style_info/style_info_decoder.rspackages/web-platform/web-core/tests/template-manager.spec.tspackages/web-platform/web-core/ts/client/decodeWorker/cssLoader.tspackages/web-platform/web-core/ts/client/decodeWorker/decode.worker.tspackages/web-platform/web-core/ts/client/decodeWorker/types.tspackages/web-platform/web-core/ts/client/mainthread/LynxView.tspackages/web-platform/web-core/ts/client/mainthread/LynxViewInstance.tspackages/web-platform/web-core/ts/client/mainthread/TemplateManager.tspackages/web-platform/web-core/ts/client/mainthread/elementAPIs/createElementAPI.tspackages/web-platform/web-core/ts/server/decode.tspackages/web-platform/web-core/ts/server/deploy.tspackages/web-platform/web-core/ts/server/elementAPIs/createElementAPI.ts
✅ Files skipped from review due to trivial changes (5)
- packages/web-platform/web-core/ts/client/decodeWorker/types.ts
- packages/web-platform/web-core/src/style_transformer/inline_style.rs
- packages/web-platform/web-core/ts/client/decodeWorker/decode.worker.ts
- .changeset/rem-unit-transform.md
- packages/web-platform/web-core/binary/client/client_bg.wasm.d.ts
🚧 Files skipped from review as they are similar to previous changes (15)
- packages/web-platform/web-core/ts/server/deploy.ts
- packages/web-platform/web-core/ts/server/decode.ts
- packages/web-platform/web-core/tests/template-manager.spec.ts
- packages/web-platform/web-core/ts/client/decodeWorker/cssLoader.ts
- packages/web-platform/web-core/src/style_transformer/token_transformer.rs
- packages/web-platform/web-core/ts/client/mainthread/elementAPIs/createElementAPI.ts
- packages/web-platform/web-core/src/template/template_sections/style_info/decoded_style_data.rs
- packages/web-platform/web-core/ts/client/mainthread/LynxViewInstance.ts
- packages/web-platform/web-core/src/template/template_sections/style_info/style_info_decoder.rs
- packages/web-platform/web-core/src/main_thread/client/element_apis/style_apis.rs
- packages/web-platform/web-core/binary/encode/encode.d.ts
- packages/web-platform/web-core/ts/server/elementAPIs/createElementAPI.ts
- packages/web-platform/web-core/src/main_thread/server/main_thread_server_context.rs
- packages/web-platform/web-core/binary/client/client.d.ts
- packages/web-platform/web-core/binary/client_legacy/client.d.ts
React External#251 Bundle Size — 590.59KiB (0%).6cbe043(current) vs 5f2cea3 main#230(baseline) Bundle metrics
|
| Current #251 |
Baseline #230 |
|
|---|---|---|
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/support-rem-trans... Project dashboard
Generated by RelativeCI Documentation Report issue
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/web-platform/web-core/src/template/template_sections/style_info/style_info_decoder.rs (1)
390-390: Add one decoder-level test fortransform_rem = true.All updated call sites currently pass
false, so this file still doesn’t exercise the new enabled path. Please add at least one assertion for thetruebranch to guard regressions in REM transformation plumbing.Also applies to: 919-920, 970-971
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web-platform/web-core/src/template/template_sections/style_info/style_info_decoder.rs` at line 390, Add a decoder-level unit test that exercises the transform_rem = true path by constructing the decoder with transform_rem enabled (use the StyleInfoDecoder type and the decoder entrypoint used in this file, e.g., decode_style_info or StyleInfoDecoder::decode), feed it a small encoded style-info blob that contains rem-based values, and assert the output uses the expected transformed units; this test should mirror existing tests in the same file but set transform_rem = true (instead of the current false) so the true branch for REM transformation at the decoder level is covered.
🤖 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/src/template/template_sections/style_info/style_info_decoder.rs`:
- Line 390: Add a decoder-level unit test that exercises the transform_rem =
true path by constructing the decoder with transform_rem enabled (use the
StyleInfoDecoder type and the decoder entrypoint used in this file, e.g.,
decode_style_info or StyleInfoDecoder::decode), feed it a small encoded
style-info blob that contains rem-based values, and assert the output uses the
expected transformed units; this test should mirror existing tests in the same
file but set transform_rem = true (instead of the current false) so the true
branch for REM transformation at the decoder level is covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8d29957c-3ad3-4ebd-8210-a4402e44a679
📒 Files selected for processing (2)
packages/web-platform/web-core/css/index.csspackages/web-platform/web-core/src/template/template_sections/style_info/style_info_decoder.rs
✅ Files skipped from review due to trivial changes (1)
- packages/web-platform/web-core/css/index.css
…esponding unit test
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/tests/deploy.spec.ts`:
- Around line 30-40: The test currently uses unsafe "as any" casts for
mockDecodeTemplate.mockReturnValue and mockCreateElementAPI.mockReturnValue;
replace those with proper typed fixtures by importing the concrete interfaces
(e.g., the decode result/interface used by decodeTemplate and the API interface
used by createElementAPI) from their source modules and use them to type the
returned objects (or alternatively use ReturnType<typeof decodeTemplate> and
ReturnType<typeof createElementAPI> if appropriate) so
mockDecodeTemplate.mockReturnValue({ ... } as DecodedTemplateType) and
mockCreateElementAPI.mockReturnValue({ ... } as CreateElementApiType) instead of
"as any", keeping the shapes aligned with the real module contracts.
🪄 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: b51cc97c-838b-4ecd-a172-3d27622625b3
📒 Files selected for processing (2)
packages/web-platform/web-core/tests/deploy.spec.tspackages/web-platform/web-core/ts/server/deploy.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/web-platform/web-core/ts/server/deploy.ts
Summary by CodeRabbit
New Features
Style
Tests
Documentation
Checklist