feat: Add web-core-wasm package#2039
Conversation
pick rust files This is a part of lynx-family#1937
|
📝 WalkthroughWalkthroughAdds a new workspace crate packages/web-platform/web-core-wasm implementing a WASM-focused runtime: CSS tokenizer, style transformer, template/style encoding & decoding, LEO ASM, main-thread context and element APIs, wasm-bindgen bindings, plus CI/editor config changes and workspace manifest updates. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (4)📓 Common learnings📚 Learning: 2025-09-18T08:12:56.802ZApplied to files:
📚 Learning: 2025-08-21T07:21:51.621ZApplied to files:
📚 Learning: 2025-07-15T10:00:56.154ZApplied to files:
🧬 Code graph analysis (1)packages/web-platform/web-core-wasm/src/template/template_sections/style_info/decoded_style_info.rs (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🔇 Additional comments (5)
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 |
…d", and "compond" to "compound".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
🧹 Nitpick comments (12)
packages/web-platform/web-core-wasm/AGENTS.md (1)
53-65: Add language specifiers to fenced code blocks.The data flow diagrams (build-time and runtime) use fenced code blocks without language specifiers, which violates markdown linting rules (MD040).
🔎 Proposed fix
For the build-time flow diagram:
-``` +```text Template Definition (JS) → RawElementTemplate (Rust) → bincode::encode → Uint8Array (serialized)For the runtime flow diagram:
-``` +```text Serialized Template (Uint8Array) → bincode::decode → ElementTemplateSection → DecodedElementTemplate → Execute Leo ASM operations → DOM elementsAlso applies to: 69-81
packages/web-platform/web-core-wasm/Cargo.toml (1)
1-20: Package configuration looks correct for a WASM crate.The manifest is well-structured:
cdylibcrate-type is appropriate for WebAssembly targets- Feature flags (
client,encode,server) align with the documented architecture in AGENTS.mdweb-syscorrectly gated behind theclientfeature with necessary DOM API features- Workspace dependencies are properly leveraged
Direct dependency versions:
fnv1.0.7 is at the latest stable versionbincode2.0.1 has a newer major version available (3.0.0, released December 16, 2025). Consider evaluating the upgrade if breaking changes are acceptable.packages/web-platform/web-core-wasm/src/template/template_sections/style_info/flattened_style_info.rs (2)
24-28: Misleading comment about cycle detection.The comment states "we don't need to do cyclic detection," but the code actually handles cycles gracefully via Kahn's algorithm (nodes in cycles remain with non-zero in-degree and are excluded from the result). The test
test_flatten_style_info_cycleconfirms this behavior. Consider updating the comment to clarify that cycles are implicitly handled rather than stating they don't need detection.🔎 Suggested comment update
- /* - * kahn's algorithm - * 1. The RawStyleInfo is already equivalent to a adjacency list. (cssId, import) - * 2. The RawStyleInfo is a DAG therefore we don't need to do cyclic detection - */ + /* + * Kahn's algorithm + * 1. The RawStyleInfo is already equivalent to an adjacency list. (cssId, import) + * 2. Cycles are implicitly handled: nodes in cycles retain non-zero in-degree + * and are excluded from the result (yielding an empty flattened output for cycles). + */
67-77: Consider avoiding the clone for better performance.The
.clone()on line 71 allocates a new HashSet on each iteration. While correct, this could be optimized for large dependency graphs by restructuring the propagation logic to avoid intermediate clones.packages/web-platform/web-core-wasm/src/constants.rs (1)
1-3: Copyright year inconsistency.The copyright year is 2023, but other files in this PR use 2025. Consider updating for consistency.
🔎 Suggested fix
-// Copyright 2023 The Lynx Authors. All rights reserved. +// Copyright 2025 The Lynx Authors. All rights reserved.packages/web-platform/web-core-wasm/src/css_tokenizer/utils.rs (1)
76-82: Consider simplifying the offset adjustment.The pattern
offset = consume_escaped(source, offset) - 1; offset += 1;is functionally correct but could be simplified. The-1followed by+1is a no-op.🔎 Proposed simplification
// the stream starts with a valid escape if is_valid_escape(code, get_char_code(source, offset + 1)) { // Consume an escaped code point. Append the returned code point to result. - offset = consume_escaped(source, offset) - 1; - offset += 1; + offset = consume_escaped(source, offset); continue; }packages/web-platform/web-core-wasm/src/main_thread/element_apis/style_apis.rs (1)
56-60: Unnecessary clone ofvalue.
value.clone()creates an extra allocation when the value could be passed directly since it's already owned by the function.🔎 Proposed fix
pub fn set_inline_styles_number_key( &self, dom: &web_sys::HtmlElement, key: i32, value: Option<String>, ) { if let Some(style_property) = constants::STYLE_PROPERTY_MAP.get(key as usize) { - self.add_inline_style_raw_string_key(dom, style_property.to_string(), value.clone()); + self.add_inline_style_raw_string_key(dom, style_property.to_string(), value); } }packages/web-platform/web-core-wasm/src/template/template_sections/element_template/mod.rs (1)
52-60: Handle encoding errors instead of usingunwrap().The
encode()method usesunwrap()onbincode::encode_to_vec, which could panic if encoding fails. This is inconsistent withfrom_encoded()which properly handles errors viamap_err. Consider returning aResultfor consistency and safety.🔎 Proposed fix
#[cfg(feature = "encode")] #[wasm_bindgen] - pub fn encode(&self) -> js_sys::Uint8Array { - js_sys::Uint8Array::from( - bincode::encode_to_vec(self, bincode::config::standard()) - .unwrap() - .as_slice(), - ) + pub fn encode(&self) -> Result<js_sys::Uint8Array, wasm_bindgen::JsError> { + let encoded = bincode::encode_to_vec(self, bincode::config::standard()) + .map_err(|e| wasm_bindgen::JsError::new(&format!("Failed to encode ElementTemplateSection: {e}")))?; + Ok(js_sys::Uint8Array::from(encoded.as_slice())) }packages/web-platform/web-core-wasm/src/main_thread/main_thread_context.rs (1)
121-134: Consider indexingcomponent_idfor O(1) lookups.
get_unique_id_by_component_idperforms a linear scan of all elements, which becomes O(n) as elements grow. If this method is called frequently, consider maintaining a separateFnvHashMap<String, usize>for component_id → unique_id lookups.packages/web-platform/web-core-wasm/src/main_thread/element_apis/element_data.rs (1)
125-142: Remove redundant.to_owned()calls.
event_nameandevent_typeare alreadyStringtypes, so.to_owned()is unnecessary. This is inconsistent withreplace_framework_cross_thread_event_handlerwhich correctly uses the parameters directly.🔎 Proposed fix
pub(crate) fn replace_framework_run_worklet_event_handler( &mut self, event_name: String, event_type: String, mts_event_identifier: Option<wasm_bindgen::JsValue>, ) { let event_handlers_map = self.event_handlers_map.get_or_insert_default(); - let event_handler_store = event_handlers_map.entry(event_name.to_owned()).or_default(); + let event_handler_store = event_handlers_map.entry(event_name).or_default(); if let Some(identifier) = mts_event_identifier { event_handler_store .framework_run_worklet_identifier - .insert(event_type.to_owned(), identifier); + .insert(event_type, identifier); } else { event_handler_store .framework_run_worklet_identifier .remove(&event_type); } }packages/web-platform/web-core-wasm/src/main_thread/element_apis/event_apis.rs (1)
108-214: Consider caching cloned datasets to reduce redundant allocations.
target_element_dataset.clone()andcurrent_target_element_data.dataset.clone()are called multiple times within the loop (lines 170, 172, 190, 192, 200, 202). Since datasets don't change during event dispatch, cloning once before the handler calls would reduce allocations.packages/web-platform/web-core-wasm/src/style_transformer/transformer.rs (1)
173-189: Consider optimizing string allocations in the hot path.The code calls
to_string()twice per transformed declaration (lines 175-176 and 185-186), which allocates newStringinstances. If this transformer is used in performance-critical paths, consider:
- Using
Cow<'a, str>for property names and values when they don't need transformation- Reusing string buffers across declarations
- Accepting owned strings directly from
query_transform_rulesif it already allocatesThis is marked as optional since the current implementation is correct and the performance impact depends on usage patterns.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (39)
.github/workflows/rust.yml.vscode/settings.jsonAGENTS.mdCargo.tomlpackages/web-platform/web-core-wasm/AGENTS.mdpackages/web-platform/web-core-wasm/Cargo.tomlpackages/web-platform/web-core-wasm/src/constants.rspackages/web-platform/web-core-wasm/src/css_tokenizer/char_code_definitions.rspackages/web-platform/web-core-wasm/src/css_tokenizer/mod.rspackages/web-platform/web-core-wasm/src/css_tokenizer/token_types.rspackages/web-platform/web-core-wasm/src/css_tokenizer/tokenize.rspackages/web-platform/web-core-wasm/src/css_tokenizer/utils.rspackages/web-platform/web-core-wasm/src/js_binding/mod.rspackages/web-platform/web-core-wasm/src/js_binding/mts_js_binding.rspackages/web-platform/web-core-wasm/src/leo_asm/mod.rspackages/web-platform/web-core-wasm/src/leo_asm/operation.rspackages/web-platform/web-core-wasm/src/lib.rspackages/web-platform/web-core-wasm/src/main_thread/element_apis/component_apis.rspackages/web-platform/web-core-wasm/src/main_thread/element_apis/dataset_apis.rspackages/web-platform/web-core-wasm/src/main_thread/element_apis/element_data.rspackages/web-platform/web-core-wasm/src/main_thread/element_apis/element_template_apis.rspackages/web-platform/web-core-wasm/src/main_thread/element_apis/event_apis.rspackages/web-platform/web-core-wasm/src/main_thread/element_apis/mod.rspackages/web-platform/web-core-wasm/src/main_thread/element_apis/style_apis.rspackages/web-platform/web-core-wasm/src/main_thread/main_thread_context.rspackages/web-platform/web-core-wasm/src/main_thread/mod.rspackages/web-platform/web-core-wasm/src/style_transformer/inline_style.rspackages/web-platform/web-core-wasm/src/style_transformer/mod.rspackages/web-platform/web-core-wasm/src/style_transformer/rules.rspackages/web-platform/web-core-wasm/src/style_transformer/token_transformer.rspackages/web-platform/web-core-wasm/src/style_transformer/transformer.rspackages/web-platform/web-core-wasm/src/template/mod.rspackages/web-platform/web-core-wasm/src/template/template_sections/element_template/mod.rspackages/web-platform/web-core-wasm/src/template/template_sections/element_template/raw_element_template.rspackages/web-platform/web-core-wasm/src/template/template_sections/mod.rspackages/web-platform/web-core-wasm/src/template/template_sections/style_info/decoded_style_info.rspackages/web-platform/web-core-wasm/src/template/template_sections/style_info/flattened_style_info.rspackages/web-platform/web-core-wasm/src/template/template_sections/style_info/mod.rspackages/web-platform/web-core-wasm/src/template/template_sections/style_info/raw_style_info.rs
🧰 Additional context used
🧠 Learnings (17)
📓 Common learnings
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.847Z
Learning: In the lynx-family/lynx-stack repository, private packages (marked with "private": true in package.json) like lynx-js/react-transform don't require meaningful changeset entries even when their public APIs change, since they are not published externally and only affect internal development.
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1917
File: packages/mcp-servers/devtool-mcp-server/tsconfig.json:8-8
Timestamp: 2025-11-06T01:19:23.670Z
Learning: The lynx-js/devtool-mcp-server package in lynx-family/lynx-stack targets Node.js >=18.19 (specified in its package.json engines), which is different from the root project's requirement of Node.js ^22 || ^24. The package uses "lib": ["ES2024.Promise"] in its tsconfig.json because it manually includes polyfills for Promise.withResolvers while maintaining compatibility with Node.js v18.
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1770
File: packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts:316-318
Timestamp: 2025-09-18T08:12:56.802Z
Learning: In packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts, the current implementation uses cardStyleElement.textContent += for lazy component styles. While this could theoretically invalidate rule indices by reparsing the stylesheet, Sherry-hue indicated that UIDs don't repeat for the same element, making this approach acceptable for now. A future optimization to use separate style elements per entry was discussed but deferred to a separate PR to keep the current lazy bundle PR focused.
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1370
File: packages/webpack/cache-events-webpack-plugin/src/LynxCacheEventsRuntimeModule.ts:23-27
Timestamp: 2025-08-21T08:46:54.494Z
Learning: In Lynx webpack runtime modules, the team prioritizes performance and simplicity over defensive runtime error handling. They prefer relying on compile-time type safety (TypeScript) rather than adding runtime checks like try-catch blocks or type validation, especially for performance-critical code like cache event setup/cleanup functions.
📚 Learning: 2025-10-10T08:22:12.051Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1837
File: packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts:266-266
Timestamp: 2025-10-10T08:22:12.051Z
Learning: In packages/web-platform/web-mainthread-apis, the handleUpdatedData function returned from prepareMainThreadAPIs is internal-only, used to serve web-core. It does not require public documentation, type exports, or SSR support.
Applied to files:
packages/web-platform/web-core-wasm/AGENTS.mdpackages/web-platform/web-core-wasm/src/main_thread/mod.rspackages/web-platform/web-core-wasm/src/main_thread/main_thread_context.rspackages/web-platform/web-core-wasm/src/main_thread/element_apis/mod.rs
📚 Learning: 2025-08-21T08:46:54.494Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1370
File: packages/webpack/cache-events-webpack-plugin/src/LynxCacheEventsRuntimeModule.ts:23-27
Timestamp: 2025-08-21T08:46:54.494Z
Learning: In Lynx webpack runtime modules, the team prioritizes performance and simplicity over defensive runtime error handling. They prefer relying on compile-time type safety (TypeScript) rather than adding runtime checks like try-catch blocks or type validation, especially for performance-critical code like cache event setup/cleanup functions.
Applied to files:
packages/web-platform/web-core-wasm/AGENTS.md
📚 Learning: 2025-07-16T06:28:26.463Z
Learnt from: PupilTong
Repo: lynx-family/lynx-stack PR: 1029
File: packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts:95-99
Timestamp: 2025-07-16T06:28:26.463Z
Learning: In the lynx-stack codebase, CSS selectors in SSR hydration are generated by their own packages, ensuring a predictable format that makes simple string manipulation safe and preferable over regex for performance reasons.
Applied to files:
packages/web-platform/web-core-wasm/AGENTS.md
📚 Learning: 2025-09-18T08:12:56.802Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1770
File: packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts:316-318
Timestamp: 2025-09-18T08:12:56.802Z
Learning: In packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts, the current implementation uses cardStyleElement.textContent += for lazy component styles. While this could theoretically invalidate rule indices by reparsing the stylesheet, Sherry-hue indicated that UIDs don't repeat for the same element, making this approach acceptable for now. A future optimization to use separate style elements per entry was discussed but deferred to a separate PR to keep the current lazy bundle PR focused.
Applied to files:
packages/web-platform/web-core-wasm/AGENTS.mdpackages/web-platform/web-core-wasm/src/style_transformer/inline_style.rspackages/web-platform/web-core-wasm/src/style_transformer/mod.rspackages/web-platform/web-core-wasm/src/template/template_sections/style_info/flattened_style_info.rspackages/web-platform/web-core-wasm/src/main_thread/element_apis/style_apis.rspackages/web-platform/web-core-wasm/src/style_transformer/rules.rspackages/web-platform/web-core-wasm/src/style_transformer/transformer.rspackages/web-platform/web-core-wasm/src/template/template_sections/style_info/decoded_style_info.rspackages/web-platform/web-core-wasm/src/template/template_sections/style_info/mod.rspackages/web-platform/web-core-wasm/src/template/template_sections/style_info/raw_style_info.rs
📚 Learning: 2025-09-18T04:43:54.426Z
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1771
File: packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_component_with_static_sibling.js:2-2
Timestamp: 2025-09-18T04:43:54.426Z
Learning: In the lynx-family/lynx-stack repository, the `add_pure_comment` function in packages/react/transform/src/swc_plugin_compat/mod.rs (around lines 478-482) is specifically for `wrapWithLynxComponent` calls, not `createSnapshot` calls. The PURE comment injection for `createSnapshot` is handled separately in swc_plugin_snapshot/mod.rs.
Applied to files:
packages/web-platform/web-core-wasm/AGENTS.md
📚 Learning: 2025-08-27T12:42:01.095Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1616
File: packages/webpack/cache-events-webpack-plugin/test/cases/not-cache-events/lazy-bundle/index.js:3-3
Timestamp: 2025-08-27T12:42:01.095Z
Learning: In webpack, properties like __webpack_require__.lynx_ce are injected during compilation/build time when webpack processes modules and generates bundles, not at runtime when dynamic imports execute. Tests for such properties don't need to wait for dynamic imports to complete.
Applied to files:
packages/web-platform/web-core-wasm/AGENTS.md
📚 Learning: 2025-08-12T16:09:32.413Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1497
File: packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_full_static.js:9-10
Timestamp: 2025-08-12T16:09:32.413Z
Learning: In the Lynx stack, functions prefixed with `__` that are called in transformed code may be injected globally by the Lynx Engine at runtime rather than exported from the React runtime package. For example, `__CreateFrame` is injected globally by the Lynx Engine, not exported from lynx-js/react.
Applied to files:
packages/web-platform/web-core-wasm/AGENTS.md
📚 Learning: 2025-08-27T08:10:09.932Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1612
File: packages/rspeedy/create-rspeedy/template-react-vitest-rltl-ts/src/tsconfig.json:3-13
Timestamp: 2025-08-27T08:10:09.932Z
Learning: In the lynx-family/lynx-stack repository, Rspeedy templates use `lynx-js/rspeedy/client` types via `rspeedy-env.d.ts` instead of `vite/client` types. Rspeedy provides its own client-side environment type definitions and doesn't require direct Vite type references.
Applied to files:
packages/web-platform/web-core-wasm/AGENTS.md
📚 Learning: 2025-07-16T06:25:41.055Z
Learnt from: PupilTong
Repo: lynx-family/lynx-stack PR: 1029
File: packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts:214-217
Timestamp: 2025-07-16T06:25:41.055Z
Learning: In the lynx-stack codebase, CSS strings produced by `genCssContent` are considered trusted developer input, so additional sanitization/escaping is not required.
Applied to files:
packages/web-platform/web-core-wasm/AGENTS.md
📚 Learning: 2025-09-10T11:42:36.855Z
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1714
File: packages/react/transform/Cargo.toml:19-19
Timestamp: 2025-09-10T11:42:36.855Z
Learning: In packages/react/transform/Cargo.toml, the crate uses serde derive macros (#[derive(Serialize, Deserialize)]) in multiple files including src/esbuild.rs and src/swc_plugin_extract_str/mod.rs, so the "derive" feature is required when migrating to workspace dependencies.
Applied to files:
Cargo.tomlpackages/web-platform/web-core-wasm/Cargo.tomlpackages/web-platform/web-core-wasm/src/template/template_sections/element_template/raw_element_template.rs
📚 Learning: 2025-10-29T10:28:27.519Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1899
File: packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/lib.rs/should_static_extract_dynamic_inline_style.js:20-24
Timestamp: 2025-10-29T10:28:27.519Z
Learning: Files inside packages/react/transform/crates/swc_plugin_snapshot/tests/__swc_snapshots__/ are auto-generated test snapshot files and should not be manually updated. Any issues with the generated code should be addressed in the code generator/transform logic, not in the snapshots themselves.
Applied to files:
Cargo.toml
📚 Learning: 2025-11-05T03:26:52.546Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1916
File: packages/react/transform/crates/swc_plugin_snapshot/lib.rs:9-9
Timestamp: 2025-11-05T03:26:52.546Z
Learning: In the lynx-stack repository's swc_core v47 upgrade (PR #1916), the import `use swc_core::atoms as swc_atoms;` is required in files that use the `quote!` macro (e.g., packages/react/transform/crates/swc_plugin_snapshot/lib.rs, swc_plugin_list/lib.rs, swc_plugin_worklet/gen_stmt.rs) even though swc_atoms may not appear explicitly in the source code. This is because the quote! macro generates code that internally references swc_atoms. Removing this import causes compiler error: "failed to resolve: use of unresolved module or unlinked crate `swc_atoms`".
Applied to files:
Cargo.toml
📚 Learning: 2025-08-21T07:21:51.621Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1562
File: packages/react/transform/src/swc_plugin_snapshot/jsx_helpers.rs:261-283
Timestamp: 2025-08-21T07:21:51.621Z
Learning: In packages/react/transform/src/swc_plugin_snapshot/jsx_helpers.rs, the team prefers to keep the original unreachable! logic for JSXSpreadChild in jsx_is_children_full_dynamic function rather than implementing defensive error handling.
Applied to files:
Cargo.toml
📚 Learning: 2025-08-11T05:59:28.530Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1305
File: packages/react/testing-library/src/plugins/vitest.ts:4-6
Timestamp: 2025-08-11T05:59:28.530Z
Learning: In the lynx-family/lynx-stack repository, the `packages/react/testing-library` package does not have `vite` as a direct dependency. It relies on `vitest` being available from the monorepo root and accesses Vite types through re-exports from `vitest/node`. Direct imports from `vite` should not be suggested for this package.
Applied to files:
Cargo.toml
📚 Learning: 2025-09-12T09:43:04.847Z
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.847Z
Learning: In the lynx-family/lynx-stack repository, private packages (marked with "private": true in package.json) like lynx-js/react-transform don't require meaningful changeset entries even when their public APIs change, since they are not published externally and only affect internal development.
Applied to files:
Cargo.toml
📚 Learning: 2025-09-28T08:46:43.177Z
Learnt from: f0rdream
Repo: lynx-family/lynx-stack PR: 1835
File: packages/react/worklet-runtime/src/workletRuntime.ts:52-55
Timestamp: 2025-09-28T08:46:43.177Z
Learning: The legacy worklet path with `_lepusWorkletHash` in `packages/react/worklet-runtime/src/workletRuntime.ts` is preserved for compatibility with MTS (Mini-app Threading Service) that doesn't support Initial Frame Rendering. This path will not be touched in current implementations.
Applied to files:
packages/web-platform/web-core-wasm/src/js_binding/mts_js_binding.rs
🧬 Code graph analysis (15)
packages/web-platform/web-core-wasm/src/style_transformer/inline_style.rs (1)
packages/web-platform/web-core-wasm/src/style_transformer/transformer.rs (5)
push_transform_kids_style(45-45)push_transform_kids_style(218-220)push_transformed_style(44-44)push_transformed_style(221-223)new(150-159)
packages/web-platform/web-core-wasm/src/main_thread/element_apis/component_apis.rs (2)
packages/web-platform/web-core-wasm/src/main_thread/element_apis/element_data.rs (1)
new(27-41)packages/web-platform/web-core-wasm/src/main_thread/main_thread_context.rs (1)
new(49-68)
packages/web-platform/web-core-wasm/src/css_tokenizer/tokenize.rs (2)
packages/web-platform/web-core-wasm/src/css_tokenizer/utils.rs (5)
consume_number(92-158)consume_name(63-89)find_white_space_end(10-20)consume_escaped(35-59)consume_bad_url_remnants(163-185)packages/web-platform/web-core-wasm/src/css_tokenizer/char_code_definitions.rs (8)
is_identifier_start(103-120)get_char_code(167-173)char_code_category(142-156)is_newline(71-73)get_new_line_length(176-184)is_valid_escape(85-87)is_name(58-60)is_number_start(124-138)
packages/web-platform/web-core-wasm/src/leo_asm/operation.rs (2)
packages/web-platform/web-core-wasm/src/template/template_sections/element_template/mod.rs (1)
bincode(34-34)packages/web-platform/web-core-wasm/src/template/template_sections/style_info/mod.rs (2)
bincode(43-43)bincode(87-87)
packages/web-platform/web-core-wasm/src/css_tokenizer/utils.rs (1)
packages/web-platform/web-core-wasm/src/css_tokenizer/char_code_definitions.rs (8)
get_char_code(167-173)is_white_space(77-79)is_digit(13-15)is_hex_digit(20-24)get_new_line_length(176-184)is_name(58-60)is_valid_escape(85-87)cmp_char(159-164)
packages/web-platform/web-core-wasm/src/main_thread/main_thread_context.rs (2)
packages/web-platform/web-core-wasm/src/main_thread/element_apis/element_data.rs (1)
new(27-41)packages/web-platform/web-core-wasm/src/main_thread/element_apis/element_template_apis.rs (2)
document(126-130)dom(155-155)
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/flattened_style_info.rs (3)
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/mod.rs (2)
from(29-36)new(42-49)packages/web-platform/web-core-wasm/src/template/template_sections/style_info/decoded_style_info.rs (1)
new(36-59)packages/web-platform/web-core-wasm/src/template/template_sections/style_info/raw_style_info.rs (2)
new(112-114)new(167-182)
packages/web-platform/web-core-wasm/src/main_thread/element_apis/style_apis.rs (4)
packages/web-platform/web-core-wasm/src/style_transformer/rules.rs (1)
query_transform_rules(246-307)packages/web-platform/web-core-wasm/src/style_transformer/inline_style.rs (1)
transform_inline_style_string(19-26)packages/react/transform/crates/swc_plugin_snapshot/lib.rs (1)
css_id(1089-1090)packages/web-platform/web-core-wasm/src/main_thread/element_apis/element_template_apis.rs (1)
dom(155-155)
packages/web-platform/web-core-wasm/src/css_tokenizer/mod.rs (3)
packages/web-platform/web-core-wasm/src/css_tokenizer/tokenize.rs (4)
tokenize(252-537)new(547-549)on_token(249-249)on_token(552-554)packages/web-platform/web-core-wasm/src/css_tokenizer/char_code_definitions.rs (18)
is_digit(13-15)is_hex_digit(20-24)is_uppercase_letter(28-30)is_lowercase_letter(34-36)is_letter(40-42)is_non_ascii(46-48)is_name_start(52-54)is_name(58-60)is_non_printable(65-67)is_newline(71-73)is_white_space(77-79)is_valid_escape(85-87)is_identifier_start(103-120)is_number_start(124-138)char_code_category(142-156)cmp_char(159-164)get_char_code(167-173)get_new_line_length(176-184)packages/web-platform/web-core-wasm/src/css_tokenizer/utils.rs (7)
cmp_str(3-8)find_white_space_end(10-20)find_decimal_number_end(22-32)consume_number(92-158)consume_name(63-89)consume_escaped(35-59)consume_bad_url_remnants(163-185)
packages/web-platform/web-core-wasm/src/main_thread/element_apis/element_template_apis.rs (4)
packages/web-platform/web-core-wasm/src/main_thread/element_apis/element_data.rs (1)
new(27-41)packages/web-platform/web-core-wasm/src/main_thread/main_thread_context.rs (1)
new(49-68)packages/web-platform/web-core-wasm/src/template/template_sections/element_template/mod.rs (1)
new(26-28)packages/web-platform/web-core-wasm/src/template/template_sections/element_template/raw_element_template.rs (1)
new(29-31)
packages/web-platform/web-core-wasm/src/style_transformer/transformer.rs (3)
packages/web-platform/web-core-wasm/src/style_transformer/rules.rs (1)
query_transform_rules(246-307)packages/web-platform/web-core-wasm/src/style_transformer/inline_style.rs (2)
push_transformed_style(15-17)push_transform_kids_style(12-14)packages/web-platform/web-core-wasm/src/style_transformer/token_transformer.rs (1)
transform_one_token(15-29)
packages/web-platform/web-core-wasm/src/template/template_sections/element_template/raw_element_template.rs (1)
packages/web-platform/web-core-wasm/src/template/template_sections/element_template/mod.rs (2)
bincode(34-34)new(26-28)
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/decoded_style_info.rs (3)
packages/web-platform/web-core-wasm/src/style_transformer/transformer.rs (1)
new(150-159)packages/web-platform/web-core-wasm/src/template/template_sections/style_info/mod.rs (1)
new(42-49)packages/web-platform/web-core-wasm/src/template/template_sections/style_info/raw_style_info.rs (3)
new(112-114)new(167-182)new(244-248)
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/mod.rs (3)
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/flattened_style_info.rs (1)
from(22-96)packages/web-platform/web-core-wasm/src/template/template_sections/style_info/decoded_style_info.rs (1)
new(36-59)packages/web-platform/web-core-wasm/src/template/template_sections/style_info/raw_style_info.rs (3)
new(112-114)new(167-182)new(244-248)
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/raw_style_info.rs (3)
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/mod.rs (4)
bincode(43-43)bincode(87-87)new(42-49)from(29-36)packages/web-platform/web-core-wasm/src/template/template_sections/style_info/decoded_style_info.rs (1)
new(36-59)packages/web-platform/web-core-wasm/src/template/template_sections/style_info/flattened_style_info.rs (1)
from(22-96)
🪛 GitHub Check: test-typos
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/decoded_style_info.rs
[warning] 114-114:
"compond" should be "compound".
[warning] 112-112:
"compond" should be "compound".
[warning] 111-111:
"compond" should be "compound".
[warning] 81-81:
"tranformed" should be "transformed".
packages/web-platform/web-core-wasm/src/main_thread/element_apis/event_apis.rs
[warning] 224-224:
"catched" should be "caught".
[warning] 223-223:
"catched" should be "caught".
[warning] 209-209:
"catched" should be "caught".
[warning] 184-184:
"catched" should be "caught".
[warning] 163-163:
"catched" should be "caught".
[warning] 131-131:
"catched" should be "caught".
🪛 markdownlint-cli2 (0.18.1)
packages/web-platform/web-core-wasm/AGENTS.md
53-53: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
69-69: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: test-rust / Test (Ubuntu)
- GitHub Check: test-rust / clippy
🔇 Additional comments (52)
.github/workflows/rust.yml (1)
51-51: LGTM! Consistent feature flag coverage across CI.The addition of
--all-featuresto both test coverage and linting ensures that all feature combinations in the workspace (including the newweb-core-wasmpackage'sclient,encode, andserverfeatures) are properly validated.Also applies to: 93-93
AGENTS.md (1)
177-177: LGTM! Documentation aligns with CI changes.The update to include
--all-featuresin the validation pipeline documentation matches the workflow changes and ensures developers replicate CI behavior locally..vscode/settings.json (1)
41-42: LGTM! IDE configuration aligns with CI feature coverage.These rust-analyzer settings ensure developers get accurate diagnostics for all feature flags during development, matching the CI's
--all-featuresapproach.packages/web-platform/web-core-wasm/AGENTS.md (1)
1-250: Excellent documentation for the new package.This AGENTS.md file provides comprehensive guidance covering architecture, features, dependencies, data flows, and development guidelines. The structure is clear and will be valuable for both human developers and AI agents working with this codebase.
Key strengths:
- Clear feature flag documentation
- Detailed module descriptions with file locations
- Performance and best practices guidelines
- Comprehensive data flow diagrams
Cargo.toml (1)
9-9: Verify workspace dependency versions.The addition of the new workspace member and
web-sysdependency looks correct. The version is left unfeaturized at the workspace level, which is appropriate sincepackages/web-platform/web-core-wasm/Cargo.tomlspecifies its required features.Confirmed:
web-sysversion 0.3.83 is the latest stable version and has no known security advisories as of December 2025.packages/web-platform/web-core-wasm/src/style_transformer/token_transformer.rs (2)
15-29: LGTM! Clean implementation with efficientCowusage.The function correctly handles rpx unit transformation with proper bounds checking. The
len() > 3check ensures there's at least one character for the numeric value before the "rpx" suffix.
31-64: Good test coverage.Tests cover the main scenarios: basic rpx, float rpx, non-rpx dimensions, case-insensitivity, and non-dimension tokens.
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/flattened_style_info.rs (1)
99-331: Excellent test coverage.The tests thoroughly cover simple chains, empty input, diamond dependencies, disjoint graphs, and cycle handling. The assertions properly verify
imported_bysets.packages/web-platform/web-core-wasm/src/css_tokenizer/token_types.rs (1)
1-29: LGTM! Clean token type definitions.The constants are well-documented with their corresponding CSS token labels and follow the CSS Syntax Module Level 3 specification.
packages/web-platform/web-core-wasm/src/constants.rs (2)
22-240: STYLE_PROPERTY_MAP is well-structured.The empty string at index 0 serves as a placeholder for 1-based indexing, which is a common pattern for property ID mappings.
242-294: Tag mappings are well-organized.The bidirectional tag maps and dynamic loading configuration are clearly documented with references to the corresponding TypeScript source.
packages/web-platform/web-core-wasm/src/style_transformer/rules.rs (2)
10-224: LGTM! Comprehensive CSS transformation rules.The rename and replace rules cover the necessary CSS property transformations including display modes, linear layout properties, and gravity mappings.
308-472: Thorough test coverage for transformation rules.Tests validate rename rules, replace rules, color transformations (both gradient and solid), and linear-weight special cases.
packages/web-platform/web-core-wasm/src/css_tokenizer/char_code_definitions.rs (3)
1-6: LGTM! Clear category constant definitions.The category constants are well-defined with appropriate values for the tokenizer state machine.
101-138: Well-structured identifier and number start detection.The
is_identifier_startandis_number_startfunctions correctly implement the CSS Syntax Module Level 3 specification for three-code-point lookahead.
158-184: LGTM! Utility functions are safe and efficient.The
cmp_char,get_char_code, andget_new_line_lengthfunctions handle bounds checking correctly and useinline(always)appropriately for hot-path code.packages/web-platform/web-core-wasm/src/main_thread/mod.rs (1)
1-10: LGTM! Clear module organization with helpful documentation.The module documentation effectively describes the purpose and key components of the main thread module.
packages/web-platform/web-core-wasm/src/lib.rs (1)
1-9: LGTM! Well-organized crate structure.The module declarations are clean with appropriate feature gating for client-specific functionality. The
js_bindingandmain_threadmodules are correctly guarded behind theclientfeature flag.packages/web-platform/web-core-wasm/src/template/template_sections/mod.rs (1)
1-2: LGTM!Clean module organization with appropriate crate-private visibility.
packages/web-platform/web-core-wasm/src/template/mod.rs (1)
1-10: LGTM!The module documentation clearly explains the purpose and key components. The crate-private visibility is appropriate for internal template handling.
packages/web-platform/web-core-wasm/src/main_thread/element_apis/mod.rs (1)
1-9: LGTM!Clean module organization with appropriate visibility levels. The
pub(super)re-exports correctly expose key types to the parent module while keeping implementation details private.packages/web-platform/web-core-wasm/src/js_binding/mod.rs (1)
1-10: LGTM!Clear documentation explaining the wasm-bindgen interface for Rust-to-JS communication. The crate-private visibility is appropriate for internal binding usage.
packages/web-platform/web-core-wasm/src/leo_asm/mod.rs (1)
1-12: LGTM!The module documentation clearly explains the Leo ASM instruction set. The feature-gated re-exports appropriately restrict
LEOAsmOpcodeto encode/client features while keepingOperationavailable more broadly.packages/web-platform/web-core-wasm/src/css_tokenizer/mod.rs (2)
1-13: LGTM!Clean module organization with good documentation. The note about being a port of the
css-treetokenizer provides helpful context for maintainability.
15-756: Excellent test coverage!The test suite is comprehensive and well-organized, covering:
- Character classification and utility functions
- Various token types (ident-like, strings, URLs, numbers, comments)
- Edge cases (escaping, EOF, whitespace handling)
- Multiple tokenizer scenarios
The
TokenCollectorhelper provides a clean testing pattern for verifying token emission.packages/web-platform/web-core-wasm/src/style_transformer/inline_style.rs (3)
8-18: LGTM!The
InlineStyleGeneratorimplementation is clean and focused. Ignoring nested style pushes (line 12-14) is appropriate for inline style transformation since inline styles don't have nested rules.
19-26: LGTM!The transformation function is straightforward. The capacity pre-allocation (
source.len() + 16) is a reasonable heuristic, as transformed styles may be slightly longer than the input due to vendor prefixes and custom properties.
28-234: Excellent test coverage!The test suite thoroughly validates the inline style transformation logic across various scenarios:
- Basic property preservation
- Lynx-specific transformations (display:linear → flex)
- Color transformations with gradient handling
- Important flag propagation
- Flex shorthand variations
- Linear layout properties
The tests provide good documentation of the transformation rules.
packages/web-platform/web-core-wasm/src/style_transformer/mod.rs (1)
1-26: LGTM!The module organization is well-structured with appropriate feature gating:
- Core transformation types (
Generator,ParsedDeclaration,StyleTransformer) are always available- Client-specific functionality (
inline_style,query_transform_rules) is properly gated behind theclientfeatureThe documentation clearly explains the module's responsibilities and key components.
packages/web-platform/web-core-wasm/src/leo_asm/operation.rs (1)
1-32: LGTM!The module cleanly defines the LEO ASM operation types with appropriate feature-gating for encode/wasm_bindgen functionality. The explicit discriminant values in
LEOAsmOpcode(with gaps at 2, 4, 9) suggest reserved or deprecated opcodes, which is a reasonable design choice for maintaining wire compatibility.packages/web-platform/web-core-wasm/src/css_tokenizer/utils.rs (1)
1-185: LGTM overall!The CSS tokenization utilities correctly implement the CSS specification sections (§4.3.7, §4.3.11, §4.3.12, §4.3.14) with proper bounds checking and escape handling throughout.
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/mod.rs (1)
80-96: LGTM on decode_into implementation.Good error handling with
map_errto convert bincode errors toJsError. The encode/decode round-trip is correctly implemented.packages/web-platform/web-core-wasm/src/main_thread/element_apis/style_apis.rs (1)
62-72: Good optimization for style string comparison.The early return when
transformed_style_str == stylesavoids unnecessary DOM updates and JS string copies. The comment explains the rationale well.packages/web-platform/web-core-wasm/src/template/template_sections/style_info/decoded_style_info.rs (1)
345-849: Comprehensive test coverage.The test module provides good coverage for font-face, rpx units, CSS-OG mappings, lazy bundles, keyframes, type selectors, multiple selectors, and pseudo-class handling.
packages/web-platform/web-core-wasm/src/js_binding/mts_js_binding.rs (1)
1-45: LGTM!Clean wasm_bindgen extern bindings for the main thread context. The JS method name mappings (e.g.,
runWorklet,publishEvent) follow the expected JavaScript API conventions, and parameter types are appropriate for the FFI boundary.packages/web-platform/web-core-wasm/src/template/template_sections/element_template/mod.rs (1)
1-20: LGTM!The struct definition with conditional derive macros for the
encodefeature and the proper use ofFnvHashMapfor performance-sensitive lookups is well-designed.packages/web-platform/web-core-wasm/src/main_thread/main_thread_context.rs (2)
46-68: LGTM!The constructor properly initializes all fields with sensible defaults. Starting
unique_id_to_element_mapwithvec![None]ensures unique IDs start at 1, avoiding potential confusion with 0 as a sentinel value.
75-114: LGTM!The
create_element_commonmethod correctly handles CSS ID inheritance from parent components and conditionally sets attributes based on configuration. The approach of usingVec::len()as the unique ID generator is simple and effective.packages/web-platform/web-core-wasm/src/css_tokenizer/tokenize.rs (3)
539-586: LGTM!The test module provides good coverage for BOM handling edge cases, including UTF-8 BOM (U+FEFF) and the little-endian marker (U+FFFE). The
TokenStreamRecordertest helper is well-structured.
248-251: LGTM!The
Parsertrait provides a clean callback interface for token consumption, enabling flexible parsing strategies without tight coupling to specific implementations.
393-418: The loop bound is correct; no off-by-one error exists.The range
offset + 2..source_length - 1correctly iterates through all valid character pairs. In Rust,a..bmeans froma(inclusive) tob(exclusive), so the loop's last iteration hasii = source_length - 2, which safely accesses bothsource[source_length - 2]andsource[source_length - 1]. This handles comments at the very end of the source correctly. Unclosed comments are also handled properly by the fallbackoffset = source_lengthcase whenis_foundremains false.Likely an incorrect or invalid review comment.
packages/web-platform/web-core-wasm/src/main_thread/element_apis/element_data.rs (1)
43-58: LGTM!The
clone_nodemethod correctly usesObject::assignfor shallow cloning JavaScript objects while properly handling the component_id clone and event handlers map. This aligns with DOM cloning semantics.packages/web-platform/web-core-wasm/src/template/template_sections/element_template/raw_element_template.rs (2)
17-52: LGTM!The
RawElementTemplatestruct and its builder methods provide a clean API for constructing element templates. The feature gating with#[cfg(feature = "encode")]is correctly applied, and the operations are well-structured with appropriate opcodes.
84-98: LGTM!The
set_cross_thread_eventmethod correctly packages event information into an operation. The parameter order (element_id, event_type, event_name, event_value) is logical and consistent with other methods.packages/web-platform/web-core-wasm/src/main_thread/element_apis/element_template_apis.rs (1)
266-272: Recursive call is intentional but could be simplified.The recursive call to
element_from_binaryafter caching the template is a valid pattern to reuse the cloning logic. However, this adds an extra function call overhead. Consider extracting the cloning logic into a helper method if performance is critical.packages/web-platform/web-core-wasm/src/template/template_sections/style_info/raw_style_info.rs (2)
299-341: LGTM!The
generate_to_string_bufmethod correctly handles all selector types with appropriate CSS syntax (.for class,#for id,[]for attribute, etc.). The combinator handling with surrounding spaces is also correct for CSS output.
342-356: LGTM!The
DeclarationParserimplementation cleanly integrates with the tokenizer'sParsertrait, collecting tokens for CSS declaration values.packages/web-platform/web-core-wasm/src/style_transformer/transformer.rs (5)
11-28: LGTM! Well-designed declaration structure.The
ParsedDeclarationstruct is clean and itsgenerate_to_string_bufmethod correctly handles all components of a CSS declaration including the!importantflag.
33-46: LGTM! Clean trait design.The
Generatortrait provides a clear interface for handling both current-element styles and child-element styles, and theStyleTransformerstruct appropriately maintains parsing state.
122-131: Verify limited!importantdetection is acceptable.The implementation only recognizes
!importantwhen the previous token isDELIM_TOKEN('!') and the current token is "important". The comments (lines 126-129) note that edge cases likeheight: 1px !important 2px;are accepted "for performance consideration."This means:
- ✅
red !importantworks- ✅
red!importantworks- ❌
red ! important(with space) won't be recognized as important⚠️ red !important extrawill be recognized as important and include "extra" in the valuePlease confirm this behavior aligns with the team's CSS parsing requirements and performance trade-offs.
162-167: LGTM! Correctly handles missing trailing semicolon.The
parsemethod ensures that declarations without a trailing semicolon are still processed by injecting a synthetic semicolon token when needed.
199-666: LGTM! Comprehensive test coverage.The test suite thoroughly covers:
- Core parsing functionality (basic declarations, multiple declarations, complex values)
- Edge cases (!important variations, whitespace handling, missing semicolons)
- Invalid input handling (bad declarations, malformed CSS)
- Special cases (BOM, comments, escaped characters, numeric/string/URL/function values)
The tests also document expected behavior for edge cases like
!importantdetection (lines 435-441), which aligns with the implementation's known limitations.
packages/web-platform/web-core-wasm/src/css_tokenizer/char_code_definitions.rs
Show resolved
Hide resolved
packages/web-platform/web-core-wasm/src/main_thread/element_apis/component_apis.rs
Outdated
Show resolved
Hide resolved
packages/web-platform/web-core-wasm/src/main_thread/element_apis/dataset_apis.rs
Outdated
Show resolved
Hide resolved
packages/web-platform/web-core-wasm/src/main_thread/element_apis/element_template_apis.rs
Outdated
Show resolved
Hide resolved
packages/web-platform/web-core-wasm/src/main_thread/element_apis/element_template_apis.rs
Outdated
Show resolved
Hide resolved
...s/web-platform/web-core-wasm/src/template/template_sections/style_info/decoded_style_info.rs
Outdated
Show resolved
Hide resolved
...s/web-platform/web-core-wasm/src/template/template_sections/style_info/decoded_style_info.rs
Outdated
Show resolved
Hide resolved
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/mod.rs
Show resolved
Hide resolved
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/raw_style_info.rs
Show resolved
Hide resolved
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/raw_style_info.rs
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
packages/web-platform/web-core-wasm/src/main_thread/element_apis/event_apis.rs (2)
62-75: Unwrap() on element lookup still present - was this intentional?Line 68 uses
.unwrap()which will panic if theunique_iddoesn't exist. This was flagged in a previous review as a major issue. Since this is a public API exposed to JavaScript, panicking could crash the WASM module.Based on learnings, if this element lookup is expected to always succeed in normal operation (i.e., the caller always provides valid IDs), the "let it crash" approach may be acceptable. However, if there's any chance of invalid IDs from JavaScript, consider returning
JsValue::NULLorJsValue::UNDEFINEDinstead.Can you confirm whether callers of
__GetEventalways guarantee validunique_idvalues, or if this API should handle invalid IDs gracefully?
78-106: Unwrap() on element lookup still present - returning empty vec would be safer.Line 81 uses
.unwrap()which was flagged in a previous review. Since this method returnsVec<EventInfo>, returning an empty vector when the element doesn't exist would be safer than panicking, especially for a public API.If the "let it crash" philosophy applies here (i.e., callers must provide valid IDs), please document this precondition.
Should
__GetEventsgracefully handle invalidunique_idby returning an empty vector?packages/web-platform/web-core-wasm/src/template/template_sections/style_info/decoded_style_info.rs (4)
81-81: Typo: "tranformed" should be "transformed".The comment still contains the typo mentioned in the PR description.
- the declarations should be tranformed by calling transform_one_declaration function. + the declarations should be transformed by calling transform_one_declaration function.
111-114: Typo: "compond" should be "compound" (variable name and comment).The variable name
compond_selector_start_indexshould becompound_selector_start_index.🔎 Proposed fix
- let mut compond_selector_start_index = simple_selector_index; - while compond_selector_start_index > 0 { + let mut compound_selector_start_index = simple_selector_index; + while compound_selector_start_index > 0 { let prev_simple_selector = - &selector.simple_selectors[compond_selector_start_index - 1]; + &selector.simple_selectors[compound_selector_start_index - 1]; if prev_simple_selector.selector_type == OneSimpleSelectorType::Combinator { break; } - compond_selector_start_index -= 1; + compound_selector_start_index -= 1; } // move the current simple selector to the compound selector start index let root_simple_selector = selector.simple_selectors.remove(simple_selector_index); selector .simple_selectors - .insert(compond_selector_start_index, root_simple_selector); + .insert(compound_selector_start_index, root_simple_selector);
244-247:assert!in decode path could crash on malformed input.If malformed keyframes data contains multiple selectors, this assertion will panic rather than handling the error gracefully.
🔎 Suggested approach
Consider returning a
Resultfrom the decode methods and propagating this as a recoverable error, or at minimum use.expect()with a descriptive message to clarify the invariant.- assert!( - style_rule.prelude.selector_list.len() == 1, - "KeyFrames rule should have only one selector" - ); + if style_rule.prelude.selector_list.len() != 1 { + // Handle error gracefully or return Result + }
331-331: Typo: "strint_buf" should be "string_buf".- let strint_buf = class_selector_map + let string_buf = class_selector_map .entry(class_selector_name.clone()) .or_default(); - declaration.generate_to_string_buf(strint_buf); + declaration.generate_to_string_buf(string_buf);
🧹 Nitpick comments (3)
packages/web-platform/web-core-wasm/src/main_thread/element_apis/event_apis.rs (1)
108-214: Consider documenting preconditions fordispatch_event_by_path.Lines 120 and 144 use
.unwrap()on element lookups, which assumes allunique_idvalues in thebubble_unique_id_pathare valid. Since this internal method is called by the publiccommon_event_handler, invalid paths from JavaScript could cause panics.If this is intentional (i.e., the caller must ensure valid paths), consider adding a doc comment stating this precondition. Alternatively, you could add debug assertions or early validation in
common_event_handler.Based on learnings, if path validity is guaranteed by the system design, the current implementation is acceptable.
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/decoded_style_info.rs (2)
273-291: Consider extracting the buffer selection logic.The pattern of selecting between
font_face_contentandstyle_contentbased onis_processing_font_faceis duplicated. This is a minor refactoring opportunity.🔎 Optional refactor
fn generate_one_declaration_block(&mut self, declaration_block: DeclarationBlock) { - (if self.is_processing_font_face { - &mut self.font_face_content - } else { - &mut self.style_content - }) - .push('{'); + let buffer = if self.is_processing_font_face { + &mut self.font_face_content + } else { + &mut self.style_content + }; + buffer.push('{'); let mut transformer = StyleTransformer::new(self); for token in declaration_block.tokens.into_iter() { transformer.on_token(token.token_type, token.value.as_str()); } - (if self.is_processing_font_face { - &mut self.font_face_content - } else { - &mut self.style_content - }) - .push('}'); + buffer.push('}');
295-342: Consider extracting common css_og mapping logic.Both
push_transform_kids_styleandpush_transformed_stylecontain nearly identical logic for updating the css_og map whenconfig_enable_css_selectoris false. This duplication could be refactored into a helper method.🔎 Optional refactor
fn update_css_og_map(&mut self, declaration: &ParsedDeclaration) { if !self.config_enable_css_selector && !self.is_processing_font_face { let class_selector_map = self .css_og_css_id_to_class_selector_name_to_declarations_map .as_mut() .unwrap() .entry(self.css_og_current_processing_css_id.unwrap()) .or_default(); for class_selector_name in self .css_og_current_processing_class_selector_names .as_ref() .unwrap() .iter() { let string_buf = class_selector_map .entry(class_selector_name.clone()) .or_default(); declaration.generate_to_string_buf(string_buf); } } }Then use it in both methods. Note that
push_transform_kids_styledoesn't checkis_processing_font_face, so the condition would need adjustment.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/web-platform/web-core-wasm/src/main_thread/element_apis/event_apis.rspackages/web-platform/web-core-wasm/src/template/template_sections/style_info/decoded_style_info.rs
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1714
File: packages/react/transform/Cargo.toml:19-19
Timestamp: 2025-09-10T11:42:36.855Z
Learning: In packages/react/transform/Cargo.toml, the crate uses serde derive macros (#[derive(Serialize, Deserialize)]) in multiple files including src/esbuild.rs and src/swc_plugin_extract_str/mod.rs, so the "derive" feature is required when migrating to workspace dependencies.
📚 Learning: 2025-07-15T10:00:56.154Z
Learnt from: PupilTong
Repo: lynx-family/lynx-stack PR: 1292
File: packages/web-platform/web-core-server/src/createLynxView.ts:144-151
Timestamp: 2025-07-15T10:00:56.154Z
Learning: In the lynx-stack codebase, PupilTong prefers the "let it crash" approach over defensive null safety checks when the condition should never occur in normal operation. This applies to cases like the `element.getAttribute(lynxUniqueIdAttribute)!` call in SSR event capture where the attribute is expected to always be present.
Applied to files:
packages/web-platform/web-core-wasm/src/main_thread/element_apis/event_apis.rs
📚 Learning: 2025-09-18T08:12:56.802Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1770
File: packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts:316-318
Timestamp: 2025-09-18T08:12:56.802Z
Learning: In packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts, the current implementation uses cardStyleElement.textContent += for lazy component styles. While this could theoretically invalidate rule indices by reparsing the stylesheet, Sherry-hue indicated that UIDs don't repeat for the same element, making this approach acceptable for now. A future optimization to use separate style elements per entry was discussed but deferred to a separate PR to keep the current lazy bundle PR focused.
Applied to files:
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/decoded_style_info.rs
🧬 Code graph analysis (1)
packages/web-platform/web-core-wasm/src/main_thread/element_apis/event_apis.rs (2)
packages/web-platform/web-core-wasm/src/main_thread/element_apis/element_data.rs (1)
new(27-41)packages/web-platform/web-core-wasm/src/main_thread/main_thread_context.rs (1)
new(49-68)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: test-rust / clippy
- GitHub Check: test-rust / Test (Ubuntu)
🔇 Additional comments (6)
packages/web-platform/web-core-wasm/src/main_thread/element_apis/event_apis.rs (5)
8-15: LGTM - EventInfo struct is properly configured for WASM bindings.The struct correctly uses
getter_with_clonefor JS interop and has appropriate field types for returning event information to JavaScript.
20-38: LGTM - Proper error handling withif let Somepattern.The method correctly handles the case where an element might not exist, silently skipping handler registration. This is appropriate behavior for an event registration API.
41-59: LGTM - Consistent error handling pattern.This method follows the same safe pattern as
add_cross_thread_event, properly handling missing elements withif let Some.
217-227: LGTM - Correct two-phase event dispatch.The method properly implements capture and bubble phases, only proceeding to the bubble phase if the event wasn't caught during capture. The logic is correct.
245-250: LGTM - Efficient event enabling with deduplication.The method correctly checks if an event is already enabled before adding it, preventing duplicate event listeners. The implementation is clean and efficient.
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/decoded_style_info.rs (1)
345-849: Excellent test coverage!The test module provides comprehensive coverage of the style decoding functionality, including font-face generation, rpx transformation, CSS-OG class mapping, lazy bundles, keyframes, type selectors, and pseudo-class handling. The explicit expected output strings make it easy to verify behavior and catch regressions.
...s/web-platform/web-core-wasm/src/template/template_sections/style_info/decoded_style_info.rs
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
CodSpeed Performance ReportMerging #2039 will improve performance by 6.56%Comparing Summary
Benchmarks breakdown
Footnotes
|
React Example#6662 Bundle Size — 237.01KiB (0%).8280e80(current) vs abd5960 main#6648(baseline) Bundle metrics
|
| Current #6662 |
Baseline #6648 |
|
|---|---|---|
 Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
 Cache Invalidation |
0% |
38.47% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
162 |
162 |
| Duplicate Modules |
65 |
65 |
| Duplicate Code |
46.75% |
46.75% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #6662 |
Baseline #6648 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
91.25KiB |
91.25KiB |
Bundle analysis report Branch PupilTong:p/hw/web-core-wasm-rus... Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#6822 Bundle Size — 376.4KiB (0%).8280e80(current) vs abd5960 main#6808(baseline) Bundle metrics
|
| Current #6822 |
Baseline #6808 |
|
|---|---|---|
| Initial JS |
149.87KiB |
149.87KiB |
| Initial CSS |
32.4KiB |
32.4KiB |
| Cache Invalidation |
0% |
0% |
| Chunks |
8 |
8 |
| Assets |
8 |
8 |
| Modules |
232(+0.43%) |
231 |
| Duplicate Modules |
16 |
16 |
| Duplicate Code |
3.12% |
3.12% |
| Packages |
4 |
4 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #6822 |
Baseline #6808 |
|
|---|---|---|
| JS |
247.01KiB |
247.01KiB |
| Other |
96.98KiB |
96.98KiB |
| CSS |
32.4KiB |
32.4KiB |
Bundle analysis report Branch PupilTong:p/hw/web-core-wasm-rus... Project dashboard
Generated by RelativeCI Documentation Report issue
PupilTong
requested review from
Sherry-hue,
Yradex,
colinaaa,
gaoachao,
upupming and
zoolsher
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/decoded_style_info.rs (1)
323-326: Typo:strint_bufshould bestring_buf.This variable name has a typo that should be corrected for consistency with the identical pattern on line 304.
🔎 Proposed fix
- let strint_buf = class_selector_map + let string_buf = class_selector_map .entry(class_selector_name.clone()) .or_default(); - declaration.generate_to_string_buf(strint_buf); + declaration.generate_to_string_buf(string_buf);
🧹 Nitpick comments (1)
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/mod.rs (1)
51-59: Improved getter safety withunwrap_or_default().The change from
.take().unwrap()to.take().unwrap_or_default()prevents panics on repeated calls, which is safer. However, the getters are still semantically "consume once" - subsequent calls silently return empty strings. Consider whether the JS consumer expects this behavior or would benefit from either:
- Non-destructive access via
as_ref().cloned().unwrap_or_default()- Or a naming convention like
take_style_content()to signal consumption
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/decoded_style_info.rspackages/web-platform/web-core-wasm/src/template/template_sections/style_info/mod.rs
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1714
File: packages/react/transform/Cargo.toml:19-19
Timestamp: 2025-09-10T11:42:36.855Z
Learning: In packages/react/transform/Cargo.toml, the crate uses serde derive macros (#[derive(Serialize, Deserialize)]) in multiple files including src/esbuild.rs and src/swc_plugin_extract_str/mod.rs, so the "derive" feature is required when migrating to workspace dependencies.
📚 Learning: 2025-09-18T08:12:56.802Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1770
File: packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts:316-318
Timestamp: 2025-09-18T08:12:56.802Z
Learning: In packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts, the current implementation uses cardStyleElement.textContent += for lazy component styles. While this could theoretically invalidate rule indices by reparsing the stylesheet, Sherry-hue indicated that UIDs don't repeat for the same element, making this approach acceptable for now. A future optimization to use separate style elements per entry was discussed but deferred to a separate PR to keep the current lazy bundle PR focused.
Applied to files:
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/decoded_style_info.rspackages/web-platform/web-core-wasm/src/template/template_sections/style_info/mod.rs
📚 Learning: 2025-08-21T07:21:51.621Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1562
File: packages/react/transform/src/swc_plugin_snapshot/jsx_helpers.rs:261-283
Timestamp: 2025-08-21T07:21:51.621Z
Learning: In packages/react/transform/src/swc_plugin_snapshot/jsx_helpers.rs, the team prefers to keep the original unreachable! logic for JSXSpreadChild in jsx_is_children_full_dynamic function rather than implementing defensive error handling.
Applied to files:
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/decoded_style_info.rs
🧬 Code graph analysis (2)
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/decoded_style_info.rs (1)
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/raw_style_info.rs (4)
new(112-114)new(167-182)new(244-248)new(265-269)
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/mod.rs (3)
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/flattened_style_info.rs (1)
from(22-96)packages/web-platform/web-core-wasm/src/template/template_sections/style_info/decoded_style_info.rs (1)
new(36-59)packages/web-platform/web-core-wasm/src/template/template_sections/style_info/raw_style_info.rs (4)
new(112-114)new(167-182)new(244-248)new(265-269)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build / Build (Windows)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: test-rust / Test (Ubuntu)
🔇 Additional comments (6)
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/mod.rs (1)
80-96: Solid encode/decode pipeline with proper error handling.The
decode_intomethod correctly:
- Decodes raw binary to
RawStyleInfo- Transforms via
StyleInfoDecoder- Re-encodes to
Uint8ArrayError handling with
JsErroris appropriate for wasm-bindgen interop.packages/web-platform/web-core-wasm/src/template/template_sections/style_info/decoded_style_info.rs (5)
72-211: Well-documented selector transformation logic.The selector rewriting rules (
:root→[part="page"],::placeholderhandling, type selector mapping, entry-name/CSS-ID injection) are clearly documented in the inline comments. The test cases cover these transformations adequately.
240-264: KeyFrames processing looks correct.The keyframes block properly:
- Outputs
@keyframeswith the animation name- Iterates nested rules for keyframe selectors (0%, 100%, etc.)
- Generates declaration blocks for each keyframe
291-335: Generator trait implementation handles routing correctly.The dual-path logic for:
- Regular style output vs child rules buffer
- CSS-OG mapping when
config_enable_css_selectoris false- Font-face exclusion from CSS-OG mapping
is appropriately structured.
338-842: Comprehensive test coverage.The test suite covers key scenarios including font-face rules, rpx unit transformation, CSS-OG class mapping, lazy bundles, keyframes, type selectors, and pseudo-class handling. Good verification of the selector transformation rules.
61-63: The topologically sorted CSS IDs are lost when inserted intoFnvHashMap.The
From<RawStyleInfo>implementation computessorted_css_idsvia topological sort (Kahn's algorithm), but Step 3 inserts these into aFnvHashMap, which does not preserve order. Whendecode()callsinto_iter()on this HashMap, the CSS processing order becomes non-deterministic. While each stylesheet is scoped via theimported_byfield, rules across different stylesheets may be processed in unpredictable order, potentially affecting CSS cascade precedence. Consider preserving thesorted_css_idsVec or using an ordered map structure to maintain deterministic output.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/web-platform/web-core-wasm/src/main_thread/element_apis/element_template_apis.rs (2)
227-244: HandleReflect::seterrors consistently.Lines 235-239 ignore the result of
Reflect::setwithlet _ =, while lines 76-81 (in the cached path) properly handle errors from the same operation usingmap_err. This inconsistency means dataset setting failures will be silently ignored here, but would propagate errors in the cached template path.For consistency and robustness, both code paths should handle
Reflect::seterrors uniformly.🔎 Proposed fix
let element_data = id_to_prepared_element_data .get_mut(&element_id) .ok_or_else(|| JsError::new(&format!("Element {element_id} not found")))?; let dataset = element_data.dataset.get_or_insert_default(); - let _ = js_sys::Reflect::set( + js_sys::Reflect::set( dataset, &JsValue::from_str(data_name), &JsValue::from_str(data_value), - ); + ) + .map_err(|e| JsError::new(&format!("Failed to set dataset property: {e:?}")))?;
253-253: Handleappend_childerrors to prevent silent failures.The result of appending
template_root_domtoroot_nodeis ignored withlet _ =. If this operation fails, the template element won't be added to the DOM tree, which could lead to issues if subsequent code expects it to be present. Since the function already returnsResult, propagating this error ensures failures are visible.🔎 Proposed fix
- let _ = self.root_node.append_child(&template_root_dom); + self.root_node + .append_child(&template_root_dom) + .map_err(|e| JsError::new(&format!("Failed to append template to root: {e:?}")))?;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/web-platform/web-core-wasm/src/main_thread/element_apis/element_template_apis.rs
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1714
File: packages/react/transform/Cargo.toml:19-19
Timestamp: 2025-09-10T11:42:36.855Z
Learning: In packages/react/transform/Cargo.toml, the crate uses serde derive macros (#[derive(Serialize, Deserialize)]) in multiple files including src/esbuild.rs and src/swc_plugin_extract_str/mod.rs, so the "derive" feature is required when migrating to workspace dependencies.
📚 Learning: 2025-07-15T10:00:56.154Z
Learnt from: PupilTong
Repo: lynx-family/lynx-stack PR: 1292
File: packages/web-platform/web-core-server/src/createLynxView.ts:144-151
Timestamp: 2025-07-15T10:00:56.154Z
Learning: In the lynx-stack codebase, PupilTong prefers the "let it crash" approach over defensive null safety checks when the condition should never occur in normal operation. This applies to cases like the `element.getAttribute(lynxUniqueIdAttribute)!` call in SSR event capture where the attribute is expected to always be present.
Applied to files:
packages/web-platform/web-core-wasm/src/main_thread/element_apis/element_template_apis.rs
📚 Learning: 2025-08-21T07:21:51.621Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1562
File: packages/react/transform/src/swc_plugin_snapshot/jsx_helpers.rs:261-283
Timestamp: 2025-08-21T07:21:51.621Z
Learning: In packages/react/transform/src/swc_plugin_snapshot/jsx_helpers.rs, the team prefers to keep the original unreachable! logic for JSXSpreadChild in jsx_is_children_full_dynamic function rather than implementing defensive error handling.
Applied to files:
packages/web-platform/web-core-wasm/src/main_thread/element_apis/element_template_apis.rs
📚 Learning: 2025-09-18T04:43:54.426Z
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1771
File: packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_component_with_static_sibling.js:2-2
Timestamp: 2025-09-18T04:43:54.426Z
Learning: In packages/react/transform/src/swc_plugin_compat/mod.rs, the `add_pure_comment` function at lines 478-482 is specifically for `wrapWithLynxComponent` calls, not `createSnapshot` calls. The PURE comment injection for `createSnapshot` is handled separately in swc_plugin_snapshot/mod.rs. These are two distinct code paths that should be treated differently.
Applied to files:
packages/web-platform/web-core-wasm/src/main_thread/element_apis/element_template_apis.rs
📚 Learning: 2025-09-18T04:43:54.426Z
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1771
File: packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_component_with_static_sibling.js:2-2
Timestamp: 2025-09-18T04:43:54.426Z
Learning: In the lynx-family/lynx-stack repository, the `add_pure_comment` function in packages/react/transform/src/swc_plugin_compat/mod.rs (around lines 478-482) is specifically for `wrapWithLynxComponent` calls, not `createSnapshot` calls. The PURE comment injection for `createSnapshot` is handled separately in swc_plugin_snapshot/mod.rs.
Applied to files:
packages/web-platform/web-core-wasm/src/main_thread/element_apis/element_template_apis.rs
🧬 Code graph analysis (1)
packages/web-platform/web-core-wasm/src/main_thread/element_apis/element_template_apis.rs (3)
packages/web-platform/web-core-wasm/src/main_thread/element_apis/element_data.rs (1)
new(27-41)packages/web-platform/web-core-wasm/src/main_thread/main_thread_context.rs (1)
new(49-68)packages/web-platform/web-core-wasm/src/template/template_sections/element_template/mod.rs (1)
new(26-28)
packages/web-platform/web-core-wasm/src/main_thread/element_apis/element_template_apis.rs
Show resolved
Hide resolved
packages/web-platform/web-core-wasm/src/main_thread/element_apis/element_template_apis.rs
Show resolved
Hide resolved
packages/web-platform/web-core-wasm/src/main_thread/element_apis/element_template_apis.rs
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/web-platform/web-core-wasm/src/main_thread/element_apis/component_apis.rs (1)
7-16: Past panic risk has been addressed - error handling looks good.The previous review flagged
.unwrap()causing panics. The current implementation properly usesok_or_elseto return aJsErrorwhen an invalidunique_idis provided, preventing WASM module crashes.
🧹 Nitpick comments (4)
packages/web-platform/web-core-wasm/src/main_thread/element_apis/component_apis.rs (1)
31-34: Consider using Rust doc comment syntax.Block comments (
/** */) work but the Rust convention is to use///for documentation comments that should appear in generated docs.🔎 Suggested format
- /** - * key: String - * value: stringifyed js value - */ + /// Sets the component configuration. + /// + /// * `key`: String + /// * `value`: stringified js valuepackages/web-platform/web-core-wasm/src/template/template_sections/style_info/decoded_style_info.rs (3)
258-260: Nested rule handling could be more explicit.The code silently skips non-Declaration nested rules in keyframes. While this is safe, consider logging a warning or returning an error for unexpected rule types to aid debugging.
🔎 Optional: Add warning for unexpected nested rule types
for nested_rule in style_rule.nested_rules.into_iter() { for (selector_index, selector) in nested_rule.prelude.selector_list.iter().enumerate() { // add a comma separator if not the last selector if selector_index > 0 { self.style_content.push(','); } selector.generate_to_string_buf(&mut self.style_content); } - if nested_rule.rule_type == RuleType::Declaration { + match nested_rule.rule_type { + RuleType::Declaration => { self.generate_one_declaration_block(nested_rule.declaration_block); + } + _ => { + // Log or return error for unexpected nested rule types + #[cfg(debug_assertions)] + eprintln!("Unexpected nested rule type in keyframes: {:?}", nested_rule.rule_type); + } } }
270-288: Declaration block generation is correct but could reduce repetition.The buffer selection logic is duplicated at the beginning and end. Consider extracting to a helper method for cleaner code.
🔎 Optional: Extract buffer selection helper
+ fn get_current_buffer(&mut self) -> &mut String { + if self.is_processing_font_face { + &mut self.font_face_content + } else { + &mut self.style_content + } + } + fn generate_one_declaration_block(&mut self, declaration_block: DeclarationBlock) { - (if self.is_processing_font_face { - &mut self.font_face_content - } else { - &mut self.style_content - }) - .push('{'); + self.get_current_buffer().push('{'); let mut transformer = StyleTransformer::new(self); for token in declaration_block.tokens.into_iter() { transformer.on_token(token.token_type, token.value.as_str()); } - (if self.is_processing_font_face { - &mut self.font_face_content - } else { - &mut self.style_content - }) - .push('}'); + self.get_current_buffer().push('}'); }
291-336: Generator implementation has code duplication in CSS-OG handling.Both
push_transform_kids_styleandpush_transformed_stylecontain nearly identical CSS-OG map update logic (lines 295-309 and 314-328). Consider extracting this into a helper method.🔎 Proposed refactor to reduce duplication
+ fn update_css_og_map_if_needed(&mut self, declaration: &ParsedDeclaration) { + if !self.config_enable_css_selector && !self.is_processing_font_face { + if let (Some(map), Some(css_id), Some(names)) = ( + self + .css_og_css_id_to_class_selector_name_to_declarations_map + .as_mut(), + self.css_og_current_processing_css_id, + self.css_og_current_processing_class_selector_names.as_ref(), + ) { + let class_selector_map = map.entry(css_id).or_default(); + for class_selector_name in names.iter() { + let string_buf = class_selector_map + .entry(class_selector_name.clone()) + .or_default(); + declaration.generate_to_string_buf(string_buf); + } + } + } + } + impl Generator for StyleInfoDecoder { fn push_transform_kids_style(&mut self, declaration: ParsedDeclaration) { declaration.generate_to_string_buf(&mut self.temp_child_rules_buffer); - if !self.config_enable_css_selector { - if let (Some(map), Some(css_id), Some(names)) = ( - self - .css_og_css_id_to_class_selector_name_to_declarations_map - .as_mut(), - self.css_og_current_processing_css_id, - self.css_og_current_processing_class_selector_names.as_ref(), - ) { - let class_selector_map = map.entry(css_id).or_default(); - for class_selector_name in names.iter() { - let string_buf = class_selector_map - .entry(class_selector_name.clone()) - .or_default(); - declaration.generate_to_string_buf(string_buf); - } - } - } + self.update_css_og_map_if_needed(&declaration); } fn push_transformed_style(&mut self, declaration: ParsedDeclaration) { - if !self.config_enable_css_selector && !self.is_processing_font_face { - if let (Some(map), Some(css_id), Some(names)) = ( - self - .css_og_css_id_to_class_selector_name_to_declarations_map - .as_mut(), - self.css_og_current_processing_css_id, - self.css_og_current_processing_class_selector_names.as_ref(), - ) { - let class_selector_map = map.entry(css_id).or_default(); - for class_selector_name in names.iter() { - let string_buf = class_selector_map - .entry(class_selector_name.clone()) - .or_default(); - declaration.generate_to_string_buf(string_buf); - } - } - } + self.update_css_og_map_if_needed(&declaration); declaration.generate_to_string_buf(if self.is_processing_font_face { &mut self.font_face_content } else { &mut self.style_content }); } }Note: The refactored
update_css_og_map_if_neededincludes the!self.is_processing_font_facecheck that was only inpush_transformed_style, ensuring consistent behavior.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/web-platform/web-core-wasm/src/main_thread/element_apis/component_apis.rspackages/web-platform/web-core-wasm/src/template/template_sections/style_info/decoded_style_info.rs
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1714
File: packages/react/transform/Cargo.toml:19-19
Timestamp: 2025-09-10T11:42:36.855Z
Learning: In packages/react/transform/Cargo.toml, the crate uses serde derive macros (#[derive(Serialize, Deserialize)]) in multiple files including src/esbuild.rs and src/swc_plugin_extract_str/mod.rs, so the "derive" feature is required when migrating to workspace dependencies.
📚 Learning: 2025-09-18T08:12:56.802Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1770
File: packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts:316-318
Timestamp: 2025-09-18T08:12:56.802Z
Learning: In packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts, the current implementation uses cardStyleElement.textContent += for lazy component styles. While this could theoretically invalidate rule indices by reparsing the stylesheet, Sherry-hue indicated that UIDs don't repeat for the same element, making this approach acceptable for now. A future optimization to use separate style elements per entry was discussed but deferred to a separate PR to keep the current lazy bundle PR focused.
Applied to files:
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/decoded_style_info.rs
📚 Learning: 2025-08-21T07:21:51.621Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1562
File: packages/react/transform/src/swc_plugin_snapshot/jsx_helpers.rs:261-283
Timestamp: 2025-08-21T07:21:51.621Z
Learning: In packages/react/transform/src/swc_plugin_snapshot/jsx_helpers.rs, the team prefers to keep the original unreachable! logic for JSXSpreadChild in jsx_is_children_full_dynamic function rather than implementing defensive error handling.
Applied to files:
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/decoded_style_info.rs
📚 Learning: 2025-07-15T10:00:56.154Z
Learnt from: PupilTong
Repo: lynx-family/lynx-stack PR: 1292
File: packages/web-platform/web-core-server/src/createLynxView.ts:144-151
Timestamp: 2025-07-15T10:00:56.154Z
Learning: In the lynx-stack codebase, PupilTong prefers the "let it crash" approach over defensive null safety checks when the condition should never occur in normal operation. This applies to cases like the `element.getAttribute(lynxUniqueIdAttribute)!` call in SSR event capture where the attribute is expected to always be present.
Applied to files:
packages/web-platform/web-core-wasm/src/main_thread/element_apis/component_apis.rs
🧬 Code graph analysis (2)
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/decoded_style_info.rs (1)
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/raw_style_info.rs (4)
new(112-114)new(167-182)new(244-248)new(265-269)
packages/web-platform/web-core-wasm/src/main_thread/element_apis/component_apis.rs (2)
packages/web-platform/web-core-wasm/src/main_thread/element_apis/element_data.rs (1)
new(27-41)packages/web-platform/web-core-wasm/src/main_thread/main_thread_context.rs (1)
new(49-68)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test-rust / Test (Ubuntu)
- GitHub Check: code-style-check
🔇 Additional comments (9)
packages/web-platform/web-core-wasm/src/main_thread/element_apis/component_apis.rs (1)
45-57: Lazy initialization in getter mutates state.The
get_configmethod creates and stores a newjs_sys::Objectwhencomponent_configisNone. While this may be intentional for ergonomics (avoiding null checks in JS), it's a side effect that might surprise callers expecting a pure getter.Consider whether this lazy initialization aligns with your API design, or if returning
Result<Option<js_sys::Object>, JsError>(likeget_element_config) would be clearer.packages/web-platform/web-core-wasm/src/template/template_sections/style_info/decoded_style_info.rs (8)
19-33: Well-structured decoder with appropriate state management.The struct design cleanly separates style content from font-face content, and the optional CSS-OG mapping is appropriately gated by configuration.
36-59: LGTM: Constructor properly initializes state and pre-allocates buffers.The use of
style_content_str_size_hintfor capacity pre-allocation is a good optimization to reduce reallocation during string building.
88-96: CSS-OG single-class selector extraction is correct.The logic properly identifies single-class selectors when CSS selectors are disabled, extracts the class name, and skips CSS emission for them.
102-124: Root selector transformation is correct.The logic properly transforms
:rootto[part="page"]and moves it to the compound selector start. The Vec manipulation (remove + insert) is correct though slightly inefficient.
124-145: Selector transformations are correctly implemented.The
::placeholderenhancement and type selector mapping logic are both straightforward and correct.
182-210: Imported CSS ID handling is correct but complex.The logic properly handles multiple
imported_byentries by cloning selectors and adding:where([css-id="..."])pseudo-classes. The comma separator logic correctly accounts for both selector and import indices.
216-232: Child rules generation with> *selector is correctly implemented.The logic properly regenerates selectors with the child combinator when transformed child declarations are present.
338-842: Excellent test coverage validating key transformation scenarios.The test suite comprehensively covers font-face rules, rpx dimension handling, CSS-OG mode, lazy bundle behavior, keyframes, type selectors, multiple selectors, and pseudo-classes. Well done!
...s/web-platform/web-core-wasm/src/template/template_sections/style_info/decoded_style_info.rs
Outdated
Show resolved
Hide resolved
…` for element lookup failures and invalid string conversions.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/raw_style_info.rs (1)
171-188:panic!()on unknown rule type will crash the WASM module.The constructor panics on invalid
rule_typestrings. Since this is exposed viawasm_bindgen, invalid input from JavaScript will cause the entire WASM module to abort. Return aResultinstead.🔎 Proposed fix
#[cfg(feature = "encode")] #[wasm_bindgen(constructor)] - pub fn new(rule_type: String) -> Rule { + pub fn new(rule_type: String) -> Result<Rule, JsError> { let rule_type_enum = match rule_type.as_str() { "StyleRule" => RuleType::Declaration, "FontFaceRule" => RuleType::FontFace, "KeyframesRule" => RuleType::KeyFrames, - _ => panic!("Unknown rule type: {rule_type}"), + _ => return Err(JsError::new(&format!("Unknown rule type: {rule_type}"))), }; - Rule { + Ok(Rule { rule_type: rule_type_enum, prelude: RulePrelude { selector_list: vec![], }, declaration_block: DeclarationBlock { tokens: vec![] }, nested_rules: vec![], - } + }) }packages/web-platform/web-core-wasm/src/css_tokenizer/tokenize.rs (1)
582-591: Remove test for invalid BOM marker.This test validates handling of
\u{FFFE}, which is not a valid BOM. As noted in the past review comment onchar_code_definitions.rs(lines 95-105),\u{FFFE}is a noncharacter used to detect wrong endianness, not a BOM to skip. This test should be removed along with the fix toget_start_offset.packages/web-platform/web-core-wasm/src/css_tokenizer/char_code_definitions.rs (1)
95-105: Remove invalid BOM marker check.As noted in the previous review,
\u{FFFE}is not a valid BOM. It's a noncharacter used to detect wrong endianness when\u{FEFF}is misinterpreted. The function should only check for\u{FEFF}(the UTF-8 BOM which is 3 bytes: EF BB BF).🔎 Proposed fix
pub fn get_start_offset(source: &str) -> usize { let bom = "\u{FEFF}"; - let bom_le = "\u{FFFE}"; - if source.starts_with(bom) || source.starts_with(bom_le) { + if source.starts_with(bom) { 3usize // BOM found } else { 0usize } }packages/web-platform/web-core-wasm/src/main_thread/element_apis/event_apis.rs (2)
67-81:unwrap()will panic on invalidunique_idfrom JavaScript.Line 74 uses
unwrap()which will crash the WASM module if an invalidunique_idis passed from JavaScript. This was previously flagged but remains unfixed.Consider returning
JsValue::NULLor an error value when the element is not found.🔎 Suggested fix
#[wasm_bindgen(js_name = "__GetEvent")] pub fn get_event( &self, unique_id: usize, event_name: String, event_type: String, ) -> wasm_bindgen::JsValue { - let binding = self.get_element_data_by_unique_id(unique_id).unwrap(); + let Some(binding) = self.get_element_data_by_unique_id(unique_id) else { + return wasm_bindgen::JsValue::NULL; + }; let element_data = binding.borrow(); let event_name = event_name.to_ascii_lowercase(); let event_type = event_type.to_ascii_lowercase(); wasm_bindgen::JsValue::from( element_data.get_framework_cross_thread_event_handler(&event_name, &event_type), ) }
83-112:unwrap()will panic on invalidunique_idfrom JavaScript.Line 87 uses
unwrap()which will crash the WASM module if an invalidunique_idis passed from JavaScript. This was previously flagged but remains unfixed.Consider returning an empty
Vec<EventInfo>when the element is not found.🔎 Suggested fix
#[wasm_bindgen(js_name = "__GetEvents")] pub fn get_events(&self, unique_id: usize) -> Vec<EventInfo> { + let Some(binding) = self.get_element_data_by_unique_id(unique_id) else { + return vec![]; + }; let mut event_infos: Vec<EventInfo> = vec![]; let event_types = vec!["bindevent", "capture-bind", "catchevent", "capture-catch"]; - let binding = self.get_element_data_by_unique_id(unique_id).unwrap(); let element_data = binding.borrow();
🧹 Nitpick comments (7)
packages/web-platform/web-core-wasm/src/constants.rs (1)
25-26: Placeholder at index 0 is intentional?The empty string at index 0 suggests 1-based indexing when looking up property names. Consider adding a brief comment to clarify this is intentional, preventing future maintainers from removing it.
packages/web-platform/web-core-wasm/src/css_tokenizer/utils.rs (1)
85-86: Simplify offset arithmetic.Lines 85-86 subtract 1 and then add 1, which cancels out. This can be simplified for clarity.
🔎 Proposed refactor
// Consume an escaped code point. Append the returned code point to result. - offset = consume_escaped(source, offset) - 1; - offset += 1; + offset = consume_escaped(source, offset); continue;packages/web-platform/web-core-wasm/src/css_tokenizer/tokenize.rs (3)
87-161: Consider simplifying offset handling in escape processing.Line 152 subtracts 1 from the offset returned by
consume_escaped, but Line 159 immediately adds 1. This pattern makes the control flow harder to follow. Consider restructuring to avoid the offsetting arithmetic.💡 Alternative approach
One option is to use
continueto skip the increment:} else if is_valid_escape(code, next_code) { // Otherwise, (the stream starts with a valid escape) consume // an escaped code point and append the returned code point to // the <string-token>'s value. - *offset = consume_escaped(source, *offset) - 1; + *offset = consume_escaped(source, *offset); + continue; }
228-236: Consider simplifying offset handling in escape processing.Lines 233-234 use the same
-1then+1pattern seen inconsume_string_token. This could be simplified by usingcontinueto skip the loop increment.💡 Alternative approach
if is_valid_escape(code, get_char_code(source, (*offset) + 1)) { - *offset = consume_escaped(source, *offset) - 1; - *offset += 1; + *offset = consume_escaped(source, *offset); continue; }
258-543: LGTM! Consider extracting some match arms for maintainability.The tokenize function correctly implements the CSS tokenization algorithm. The main state machine is comprehensive and handles all token types properly. As an optional improvement, some of the longer match arms (e.g.,
U+002F SOLIDUSorU+002D HYPHEN-MINUS) could be extracted into helper functions to improve readability.packages/web-platform/web-core-wasm/src/main_thread/element_apis/element_data.rs (2)
131-148: Consider removing unnecessaryto_owned()calls.Lines 138 and 142 call
to_owned()onevent_nameandevent_typeparameters that are already ownedStringtypes. You could move these values directly instead of cloning them, or restructure the branching logic to avoid clones.🔎 Suggested optimization
pub(crate) fn replace_framework_run_worklet_event_handler( &mut self, event_name: String, event_type: String, mts_event_identifier: Option<wasm_bindgen::JsValue>, ) { let event_handlers_map = self.event_handlers_map.get_or_insert_default(); - let event_handler_store = event_handlers_map.entry(event_name.to_owned()).or_default(); + let event_handler_store = event_handlers_map.entry(event_name).or_default(); if let Some(identifier) = mts_event_identifier { event_handler_store .framework_run_worklet_identifier - .insert(event_type.to_owned(), identifier); + .insert(event_type, identifier); } else { event_handler_store .framework_run_worklet_identifier .remove(&event_type); } }Note: This requires adjusting the
elsebranch to borrowevent_type. Alternatively, clone only where needed in theifbranch.
150-180: Consider removing commented-out code.Large blocks of commented code are better tracked in version control history rather than kept in the source. If this code serves as documentation for alternative approaches, consider adding a brief comment explaining why it's preserved.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
packages/web-platform/web-core-wasm/src/constants.rspackages/web-platform/web-core-wasm/src/css_tokenizer/char_code_definitions.rspackages/web-platform/web-core-wasm/src/css_tokenizer/mod.rspackages/web-platform/web-core-wasm/src/css_tokenizer/token_types.rspackages/web-platform/web-core-wasm/src/css_tokenizer/tokenize.rspackages/web-platform/web-core-wasm/src/css_tokenizer/utils.rspackages/web-platform/web-core-wasm/src/js_binding/mod.rspackages/web-platform/web-core-wasm/src/js_binding/mts_js_binding.rspackages/web-platform/web-core-wasm/src/leo_asm/mod.rspackages/web-platform/web-core-wasm/src/lib.rspackages/web-platform/web-core-wasm/src/main_thread/element_apis/component_apis.rspackages/web-platform/web-core-wasm/src/main_thread/element_apis/dataset_apis.rspackages/web-platform/web-core-wasm/src/main_thread/element_apis/element_data.rspackages/web-platform/web-core-wasm/src/main_thread/element_apis/event_apis.rspackages/web-platform/web-core-wasm/src/main_thread/element_apis/mod.rspackages/web-platform/web-core-wasm/src/main_thread/mod.rspackages/web-platform/web-core-wasm/src/template/mod.rspackages/web-platform/web-core-wasm/src/template/template_sections/mod.rspackages/web-platform/web-core-wasm/src/template/template_sections/style_info/mod.rspackages/web-platform/web-core-wasm/src/template/template_sections/style_info/raw_style_info.rs
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/web-platform/web-core-wasm/src/template/template_sections/mod.rs
- packages/web-platform/web-core-wasm/src/js_binding/mts_js_binding.rs
- packages/web-platform/web-core-wasm/src/css_tokenizer/mod.rs
- packages/web-platform/web-core-wasm/src/js_binding/mod.rs
- packages/web-platform/web-core-wasm/src/css_tokenizer/token_types.rs
- packages/web-platform/web-core-wasm/src/lib.rs
- packages/web-platform/web-core-wasm/src/main_thread/element_apis/dataset_apis.rs
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1714
File: packages/react/transform/Cargo.toml:19-19
Timestamp: 2025-09-10T11:42:36.855Z
Learning: In packages/react/transform/Cargo.toml, the crate uses serde derive macros (#[derive(Serialize, Deserialize)]) in multiple files including src/esbuild.rs and src/swc_plugin_extract_str/mod.rs, so the "derive" feature is required when migrating to workspace dependencies.
📚 Learning: 2025-10-10T08:22:12.051Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1837
File: packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts:266-266
Timestamp: 2025-10-10T08:22:12.051Z
Learning: In packages/web-platform/web-mainthread-apis, the handleUpdatedData function returned from prepareMainThreadAPIs is internal-only, used to serve web-core. It does not require public documentation, type exports, or SSR support.
Applied to files:
packages/web-platform/web-core-wasm/src/main_thread/mod.rspackages/web-platform/web-core-wasm/src/main_thread/element_apis/mod.rs
📚 Learning: 2025-07-15T10:00:56.154Z
Learnt from: PupilTong
Repo: lynx-family/lynx-stack PR: 1292
File: packages/web-platform/web-core-server/src/createLynxView.ts:144-151
Timestamp: 2025-07-15T10:00:56.154Z
Learning: In the lynx-stack codebase, PupilTong prefers the "let it crash" approach over defensive null safety checks when the condition should never occur in normal operation. This applies to cases like the `element.getAttribute(lynxUniqueIdAttribute)!` call in SSR event capture where the attribute is expected to always be present.
Applied to files:
packages/web-platform/web-core-wasm/src/main_thread/element_apis/component_apis.rspackages/web-platform/web-core-wasm/src/main_thread/element_apis/event_apis.rs
📚 Learning: 2025-08-12T16:09:32.413Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1497
File: packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_full_static.js:9-10
Timestamp: 2025-08-12T16:09:32.413Z
Learning: In the Lynx stack, functions prefixed with `__` that are called in transformed code may be injected globally by the Lynx Engine at runtime rather than exported from the React runtime package. For example, `__CreateFrame` is injected globally by the Lynx Engine, not exported from lynx-js/react.
Applied to files:
packages/web-platform/web-core-wasm/src/main_thread/element_apis/mod.rs
📚 Learning: 2025-08-21T08:46:54.494Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1370
File: packages/webpack/cache-events-webpack-plugin/src/LynxCacheEventsRuntimeModule.ts:23-27
Timestamp: 2025-08-21T08:46:54.494Z
Learning: In Lynx webpack runtime modules, the team prioritizes performance and simplicity over defensive runtime error handling. They prefer relying on compile-time type safety (TypeScript) rather than adding runtime checks like try-catch blocks or type validation, especially for performance-critical code like cache event setup/cleanup functions.
Applied to files:
packages/web-platform/web-core-wasm/src/main_thread/element_apis/mod.rs
📚 Learning: 2025-11-05T03:26:52.546Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1916
File: packages/react/transform/crates/swc_plugin_snapshot/lib.rs:9-9
Timestamp: 2025-11-05T03:26:52.546Z
Learning: In the lynx-stack repository's swc_core v47 upgrade (PR #1916), the import `use swc_core::atoms as swc_atoms;` is required in files that use the `quote!` macro (e.g., packages/react/transform/crates/swc_plugin_snapshot/lib.rs, swc_plugin_list/lib.rs, swc_plugin_worklet/gen_stmt.rs) even though swc_atoms may not appear explicitly in the source code. This is because the quote! macro generates code that internally references swc_atoms. Removing this import causes compiler error: "failed to resolve: use of unresolved module or unlinked crate `swc_atoms`".
Applied to files:
packages/web-platform/web-core-wasm/src/main_thread/element_apis/mod.rs
📚 Learning: 2025-09-18T08:12:56.802Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1770
File: packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts:316-318
Timestamp: 2025-09-18T08:12:56.802Z
Learning: In packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts, the current implementation uses cardStyleElement.textContent += for lazy component styles. While this could theoretically invalidate rule indices by reparsing the stylesheet, Sherry-hue indicated that UIDs don't repeat for the same element, making this approach acceptable for now. A future optimization to use separate style elements per entry was discussed but deferred to a separate PR to keep the current lazy bundle PR focused.
Applied to files:
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/raw_style_info.rs
📚 Learning: 2025-09-10T11:42:36.855Z
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1714
File: packages/react/transform/Cargo.toml:19-19
Timestamp: 2025-09-10T11:42:36.855Z
Learning: In packages/react/transform/Cargo.toml, the crate uses serde derive macros (#[derive(Serialize, Deserialize)]) in multiple files including src/esbuild.rs and src/swc_plugin_extract_str/mod.rs, so the "derive" feature is required when migrating to workspace dependencies.
Applied to files:
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/raw_style_info.rs
🧬 Code graph analysis (3)
packages/web-platform/web-core-wasm/src/css_tokenizer/utils.rs (1)
packages/web-platform/web-core-wasm/src/css_tokenizer/char_code_definitions.rs (8)
get_char_code(173-179)is_white_space(83-85)is_digit(19-21)is_hex_digit(26-30)get_new_line_length(182-190)is_name(64-66)is_valid_escape(91-93)cmp_char(165-170)
packages/web-platform/web-core-wasm/src/main_thread/element_apis/component_apis.rs (2)
packages/web-platform/web-core-wasm/src/main_thread/element_apis/element_data.rs (1)
new(33-47)packages/web-platform/web-core-wasm/src/main_thread/main_thread_context.rs (1)
new(49-68)
packages/web-platform/web-core-wasm/src/css_tokenizer/tokenize.rs (3)
packages/web-platform/web-core-wasm/src/css_tokenizer/utils.rs (6)
consume_number(98-164)consume_name(69-95)cmp_str(9-14)find_white_space_end(16-26)consume_escaped(41-65)consume_bad_url_remnants(169-191)packages/web-platform/web-core-wasm/src/css_tokenizer/char_code_definitions.rs (9)
is_identifier_start(109-126)get_char_code(173-179)char_code_category(148-162)is_newline(77-79)get_new_line_length(182-190)is_valid_escape(91-93)get_start_offset(97-105)is_name(64-66)is_number_start(130-144)packages/web-platform/web-core-wasm/src/css_tokenizer/mod.rs (12)
on_token(245-247)on_token(339-341)on_token(395-397)on_token(475-477)on_token(587-589)on_token(686-688)new(239-241)new(333-335)new(389-391)new(469-471)new(581-583)new(680-682)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build / Build (Windows)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: test-rust / clippy
- GitHub Check: test-rust / Test (Ubuntu)
🔇 Additional comments (33)
packages/web-platform/web-core-wasm/src/leo_asm/mod.rs (2)
1-14: LGTM! Clean module documentation.The copyright header and module documentation are well-structured. The documentation clearly explains the Leo Assembly module's purpose and key components.
15-18: Well-structured module with proper feature gating.The module declaration and re-exports follow Rust best practices. The conditional compilation for
LEOAsmOpcodebehind feature flags is implemented correctly, and the use ofpub(crate)visibility is appropriate for internal crate APIs.packages/web-platform/web-core-wasm/src/template/mod.rs (1)
7-16: LGTM!The module documentation clearly describes the template structure and its key components. The
pub(crate)visibility appropriately restricts access to within the crate.packages/web-platform/web-core-wasm/src/constants.rs (2)
99-99: Verifyline-spacingproperty name.Standard CSS uses
line-heightrather thanline-spacing. Isline-spacinga Lynx-specific property, or should this beline-height? Note thatline-heightis already listed at line 71.
244-296: LGTM!The lazy_static maps are well-structured. The inverse map derivation is correct, and grouping related components under the same dynamic load ID is a sensible approach for bundle loading.
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/mod.rs (3)
57-65: Previous concern addressed.The getters now use
unwrap_or_default()instead ofunwrap(), which returns an empty string instead of panicking on second call. This is a reasonable fix.
67-84: LGTM!The query logic correctly handles missing keys by returning an empty string. The nested lookups are appropriate for the map structure.
86-102: LGTM!The decode-transform-encode pipeline correctly handles errors at each stage. The
?operator properly propagates theJsErrorfromStyleInfoDecoder::new.packages/web-platform/web-core-wasm/src/template/template_sections/style_info/raw_style_info.rs (5)
22-28: LGTM!The
RawStyleInfostruct correctly usesFnvHashMapfor performance with integer keys. Thepub(super)visibility appropriately restricts access to the parent module.
282-311: Previous concern addressed.The method now returns
Result<(), JsError>instead of panicking on unknown selector types. This is the correct approach for WASM-exposed APIs.
149-162: LGTM!The encode flow correctly computes the size hint before serialization. The clone is necessary since
StyleInfoDecoder::newconsumes the input.
314-356: LGTM!The selector string generation correctly maps each selector type to its CSS syntax. The combinator handling with surrounding spaces is appropriate.
357-370: LGTM!The
DeclarationParsercleanly implements theParsertrait to accumulate tokens during CSS value parsing.packages/web-platform/web-core-wasm/src/css_tokenizer/utils.rs (5)
16-26: LGTM!The function correctly advances through whitespace characters with proper bounds checking.
28-38: LGTM!The function correctly advances through digit characters with proper bounds checking.
40-65: LGTM!The escape sequence consumption correctly implements CSS spec § 4.3.7, including hex digit handling and trailing whitespace consumption.
97-164: Note the TODO comments for type tracking.The function correctly parses numeric tokens, but Lines 124 and 154 contain TODO comments for setting the number type flag. While the offset advancement logic is correct, ensure type tracking is implemented if needed for proper token classification.
166-191: LGTM!The function correctly implements CSS spec § 4.3.14 for consuming bad URL remnants, including proper escape handling.
packages/web-platform/web-core-wasm/src/css_tokenizer/tokenize.rs (2)
17-46: LGTM!The function correctly implements CSS spec § 4.3.3 for numeric token consumption, properly distinguishing between dimension, percentage, and number tokens.
48-85: LGTM!The function correctly implements CSS spec § 4.3.4, including the special handling for
url()tokens with quoted strings.packages/web-platform/web-core-wasm/src/css_tokenizer/char_code_definitions.rs (6)
7-11: LGTM!The category constants are well-defined and provide a clear classification scheme for the tokenizer.
13-93: LGTM!The character classification predicates are well-implemented with appropriate use of
const fnandinlineattributes. The logic correctly implements CSS syntax character categories.
107-126: LGTM!The function correctly implements the CSS identifier start check, including proper handling of hyphen-minus, name-start characters, and escape sequences.
128-144: LGTM!The function correctly implements the CSS number start check, covering all valid patterns including sign prefixes, decimal points, and leading digits.
146-162: LGTM!The categorization function correctly classifies character codes for the tokenizer. The use of match guards is appropriate, and returning the raw character code for punctuation allows efficient direct matching in the tokenizer.
164-190: LGTM!The helper functions are well-implemented with proper bounds checking. The CRLF handling in
get_new_line_lengthcorrectly treats\r\nas a single newline sequence.packages/web-platform/web-core-wasm/src/main_thread/mod.rs (1)
1-16: LGTM! Clean module organization.The module documentation clearly describes the main thread subsystem and its key components. The structure is well-organized.
packages/web-platform/web-core-wasm/src/main_thread/element_apis/mod.rs (1)
1-15: LGTM! Appropriate module organization and visibility controls.The module structure properly wires up the element APIs surface with controlled visibility using
pub(super)re-exports for core types.packages/web-platform/web-core-wasm/src/main_thread/element_apis/element_data.rs (1)
1-79: LGTM! Well-structured element data management.The
EventHandlerandLynxElementDatastructures provide a clean API for managing per-element state, event handlers, and exposure detection. Theclone_nodemethod correctly usesjs_sys::Object::assignto clone JS objects.packages/web-platform/web-core-wasm/src/main_thread/element_apis/component_apis.rs (1)
10-77: LGTM! Proper error handling implemented.All methods correctly return
Result<T, JsError>and useok_or_else()to handle missing elements gracefully, preventing panics when invalidunique_idvalues are passed from JavaScript. The error messages are clear and descriptive.packages/web-platform/web-core-wasm/src/main_thread/element_apis/event_apis.rs (3)
10-21: LGTM! Clean wasm_bindgen struct definition.The
EventInfostruct properly exposes event data to JavaScript with appropriate getter attributes and field renaming.
23-65: LGTM! Safe handling of invalid unique_id.Both
add_cross_thread_eventandadd_run_worklet_eventcorrectly use theif let Somepattern to gracefully handle cases where the element is not found, avoiding panics.
236-257: LGTM! Clean event enablement helper.The
enable_eventhelper correctly registers event listeners on first use and maintains a set of enabled events.
…start > end` validation to `cmp_str`.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/raw_style_info.rs (2)
155-162: Consider the performance impact of cloning for size hint computation.The
encode()method clones the entireRawStyleInfostructure just to computestyle_content_str_size_hint. For large stylesheets with many rules, this clone could be expensive. However, this appears to be a one-time encoding operation (likely during build/initialization), so the impact may be acceptable.If encoding performance becomes a concern, consider computing the size hint incrementally during mutation (in
append_import,push_rule) or providing a separate size estimation method that doesn't require full decoding.
344-346: The UniversalSelector ignores its value field, which is inconsistent with the public API.The
UniversalSelectorcase ingenerate_to_string_bufoutputs only'*'and discardsselector.value, while the publicpush_one_selector_sectionAPI (line 286) accepts a value parameter for all selector types, includingUniversalSelector. This is semantically correct for CSS (universal selectors are just*), but creates a design inconsistency where the API allows setting a value that will be silently ignored.Add validation in
push_one_selector_sectionto ensure the value is empty whenUniversalSelectoris created, or document this behavior explicitly.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/web-platform/web-core-wasm/src/css_tokenizer/utils.rspackages/web-platform/web-core-wasm/src/template/template_sections/style_info/raw_style_info.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/web-platform/web-core-wasm/src/css_tokenizer/utils.rs
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1714
File: packages/react/transform/Cargo.toml:19-19
Timestamp: 2025-09-10T11:42:36.855Z
Learning: In packages/react/transform/Cargo.toml, the crate uses serde derive macros (#[derive(Serialize, Deserialize)]) in multiple files including src/esbuild.rs and src/swc_plugin_extract_str/mod.rs, so the "derive" feature is required when migrating to workspace dependencies.
📚 Learning: 2025-09-18T08:12:56.802Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1770
File: packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts:316-318
Timestamp: 2025-09-18T08:12:56.802Z
Learning: In packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts, the current implementation uses cardStyleElement.textContent += for lazy component styles. While this could theoretically invalidate rule indices by reparsing the stylesheet, Sherry-hue indicated that UIDs don't repeat for the same element, making this approach acceptable for now. A future optimization to use separate style elements per entry was discussed but deferred to a separate PR to keep the current lazy bundle PR focused.
Applied to files:
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/raw_style_info.rs
🔇 Additional comments (2)
packages/web-platform/web-core-wasm/src/template/template_sections/style_info/raw_style_info.rs (2)
171-190: Previous critical issues successfully resolved.Both previously flagged panic conditions have been properly fixed:
Rule::newnow returnsResult<Rule, JsError>and handles unknown rule types gracefullypush_one_selector_sectionnow returnsResult<(), JsError>and handles unknown selector types gracefullyThe error handling is well-implemented with descriptive error messages.
Also applies to: 286-313
18-154: Well-structured WASM bindings with clean API design.The structure definitions and core implementations demonstrate several good practices:
- Proper feature gating with
#[cfg(feature = "encode")]- Appropriate visibility modifiers (
pub(super)for internal fields, public for WASM-exposed structs)- Clean use of
or_default()for lazy initialization (lines 132, 145)- Comprehensive documentation comments for all public methods
- Efficient
FnvHashMapfor integer key lookupsThe API surface is intuitive and follows common builder patterns.
Also applies to: 248-267
2 participants
pick rust files
This is a part of #1937
Summary by CodeRabbit
New Features
Tests
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.
Checklist