-
Notifications
You must be signed in to change notification settings - Fork 111
feat(web): inline-style-parse uses JsString instead of [u16] #1367
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: a4f176f The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThis change refactors the inline style parsing and transformation pipeline across Rust and TypeScript to use Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🔭 Outside diff range comments (6)
packages/web-platform/web-mainthread-apis/src/utils/tokenizer.ts (2)
59-102: Remove commented UTF-16 memory management code.The function correctly uses direct string passing, but the commented UTF-16 code should be removed for cleaner maintainability.
export function transformParsedStyles( styles: [string, string][], ): { childStyle: [string, string][]; transformedStyle: [string, string][] } { let childStyle: [string, string][] = []; let transformedStyle: [string, string][] = []; for (const [property, value] of styles) { - // const { ptr: propertyPtr, len: propertyLen } = stringToUTF16(property); - // const { ptr: valuePtr, len: valueLen } = stringToUTF16(value); try { const transformedResult = wasm .transform_raw_u16_inline_style_ptr_parsed( - // propertyPtr, - // propertyLen, property, property.length, - // valuePtr, - // valueLen, value, value.length, ); - // wasm.free(propertyPtr, propertyLen << 1); - // wasm.free(valuePtr, valueLen << 1); if (transformedResult) { const [transformedStyleForCurrent, childStyleForCurrent] = transformedResult; transformedStyle = transformedStyle.concat(transformedStyleForCurrent); if (childStyleForCurrent) { childStyle = childStyle.concat(childStyleForCurrent); } } else { // If the transformation fails, we keep the original style transformedStyle.push([property, value]); } } catch (e) { - // wasm.free(propertyPtr, propertyLen << 1); - // wasm.free(valuePtr, valueLen << 1); throw e; } } return { childStyle, transformedStyle, }; }
4-4: Remove unused UTF-16 conversion utilities.The
HEAPU16variable andstringToUTF16function are no longer used and should be removed.let wasm: Awaited<ReturnType<typeof init>>; -let HEAPU16: Uint16Array | undefined; var ENVIRONMENT_IS_NODE = typeof process == 'object' && typeof process.versions == 'object' && typeof process.versions.node == 'string';-const stringToUTF16 = (str: string) => { - const len = str.length; - const ptr = wasm.malloc(len << 1); - if (!HEAPU16 || HEAPU16.byteLength == 0) { - HEAPU16 = new Uint16Array(wasm.memory.buffer); - } - for (let i = 0; i < len; i++) { - HEAPU16[(ptr >> 1) + i] = str.charCodeAt(i); - } - return { ptr, len }; -};Also applies to: 30-40
packages/web-platform/web-style-transformer/src/transformer/transformer.rs (3)
36-47: Fix macro implementation for JsStringThe
is_digit_only!macro attempts to iterate over a JsString, but JsString doesn't implement Iterator. You need to iterate through the characters using index-based access.macro_rules! is_digit_only { ($source:expr, $start:expr, $end:expr) => {{ let mut result = true; - for code in $source.slice($start, $end).iter() { - if code > b'9' as u16 || code < b'0' as u16 { + for i in $start..$end { + let code = $source.char_code_at(i) as u32; + if code > '9' as u32 || code < '0' as u32 { result = false; break; } } result }}; }
172-172: Potential issue with is_digit_only! macro usageThis line uses the
is_digit_only!macro which has the iteration issue mentioned above. This will cause a runtime error.
216-216: Another usage of broken is_digit_only! macroThis is another instance where the broken
is_digit_only!macro is used.packages/web-platform/web-style-transformer/src/transformer/constants.rs (1)
45-216: Apply consistent constant definitionsSome constants use direct string assignment while others still use the
str_to_u16_slice!macro. After removing the macro, ensure all constants use direct string assignment for consistency.
🧹 Nitpick comments (12)
packages/web-platform/web-style-transformer/src/transformer/rules.rs (1)
264-290: Macros correctly handle UTF-16 to String conversion.The macro implementations properly convert UTF-16 slices to
Stringobjects for hash map lookups. However, note that this approach involves string conversion overhead on each lookup.Consider caching converted strings or optimizing the lookup mechanism if performance becomes a concern.
packages/web-platform/inline-style-parser/src/utils.rs (3)
70-70: Remove redundant cast to u32The
get_char_code!macro already returns au32value, so the additional cast is unnecessary.- let code = get_char_code!(source, source_length, offset) as u32; + let code = get_char_code!(source, source_length, offset);
114-115: Remove commented-out codeThis commented code is not needed and should be removed to improve code cleanliness.
- // let mut code: u16 = source[offset]; let mut code = source.char_code_at(offset) as u32;
146-146: Use a named constant instead of magic numberConsider using a named constant for the character code 101 (lowercase 'e') for better readability.
- if cmp_char!(source, source_length, offset, 101 /* e */) != 0 { + if cmp_char!(source, source_length, offset, 'e' as u32) != 0 {packages/web-platform/web-style-transformer/src/lib.rs (3)
13-44: Remove large blocks of commented codeThese commented code blocks should be removed as they're no longer needed after the refactoring to JsString. If this code is needed for reference, it should be preserved in version control history instead.
13-14: Use English comments for consistencyPlease translate the Chinese comments to English for consistency across the codebase.
-// 将参数从 (ptr: *const u16, len: usize) 改为直接接收 JsString +// Changed parameters from (ptr: *const u16, len: usize) to directly receive JsString
92-92: Remove unnecessary castThe cast
0 as u32is redundant since0is already inferred asu32from the function signature.- &name_ptr, 0 as u32, name_len, &value_ptr, 0 as u32, value_len, + &name_ptr, 0, name_len, &value_ptr, 0, value_len,packages/web-platform/inline-style-parser/src/tokenize.rs (1)
408-410: Simplify character code comparisonsThe current casting pattern is unnecessarily complex. Since
char_code_atreturnsf64, you can compare directly with numeric literals.- if source.char_code_at(ii) == ('*' as u16 as f64) - && source.char_code_at(ii + 1) == ('/' as u16 as f64) + if source.char_code_at(ii) == 42.0 // '*' + && source.char_code_at(ii + 1) == 47.0 // '/'packages/web-platform/web-style-transformer/src/transformer/transformer.rs (3)
3-3: Remove unused importThe
web_sys::consoleimport is not used in this file and should be removed.-use web_sys::console;
79-80: Remove commented codePlease remove this commented code to maintain code cleanliness.
- // let value_str = &value.as_string().to_stri; if let Some(renamed_value) = get_rename_rule_value!(name, name_start, name_end) {
342-342: Remove large blocks of commented codePlease remove all the commented code blocks throughout this file. They clutter the codebase and make it harder to read.
Also applies to: 401-415
packages/web-platform/web-style-transformer/src/transformer/constants.rs (1)
24-102: Remove commented-out constant definitionsPlease remove all the commented-out constant definitions. They're no longer needed and clutter the code.
For example:
-// pub const IMPORTANT_STR_U16: &str = str_to_u16_slice!(" !important"); pub const IMPORTANT_STR_U16: &str = " !important";
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
.changeset/cyan-comics-glow.md(1 hunks)packages/web-platform/inline-style-parser/Cargo.toml(1 hunks)packages/web-platform/inline-style-parser/src/char_code_definitions.rs(8 hunks)packages/web-platform/inline-style-parser/src/lib.rs(33 hunks)packages/web-platform/inline-style-parser/src/parse_inline_style.rs(4 hunks)packages/web-platform/inline-style-parser/src/tokenize.rs(9 hunks)packages/web-platform/inline-style-parser/src/types.rs(1 hunks)packages/web-platform/inline-style-parser/src/utils.rs(9 hunks)packages/web-platform/web-mainthread-apis/src/utils/tokenizer.ts(3 hunks)packages/web-platform/web-style-transformer/Cargo.toml(1 hunks)packages/web-platform/web-style-transformer/package.json(1 hunks)packages/web-platform/web-style-transformer/src/lib.rs(3 hunks)packages/web-platform/web-style-transformer/src/transformer/constants.rs(4 hunks)packages/web-platform/web-style-transformer/src/transformer/rules.rs(1 hunks)packages/web-platform/web-style-transformer/src/transformer/transformer.rs(19 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.
Learnt from: PupilTong
PR: lynx-family/lynx-stack#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.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:26:16.722Z
Learning: In the lynx-family/lynx-stack repository, CI checks require changesets when files matching the pattern "src/**" are modified (as configured in .changeset/config.json). For internal changes that don't need meaningful changesets, an empty changeset file is used to satisfy the CI requirement while not generating any release notes.
Learnt from: PupilTong
PR: lynx-family/lynx-stack#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.
.changeset/cyan-comics-glow.md (5)
Learnt from: colinaaa
PR: #1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.
Learnt from: colinaaa
PR: #1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:26:16.722Z
Learning: In the lynx-family/lynx-stack repository, CI checks require changesets when files matching the pattern "src/**" are modified (as configured in .changeset/config.json). For internal changes that don't need meaningful changesets, an empty changeset file is used to satisfy the CI requirement while not generating any release notes.
Learnt from: PupilTong
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.
Learnt from: PupilTong
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.
Learnt from: PupilTong
PR: #1029
File: packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts:95-99
Timestamp: 2025-07-16T05:57:29.837Z
Learning: In the lynx-stack codebase, PupilTong prefers avoiding regex for parsing in performance-critical code paths like SSR hydration, preferring simple string manipulation operations even if they're less robust, when the input format is predictable and controlled.
packages/web-platform/web-style-transformer/src/lib.rs (1)
Learnt from: PupilTong
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.
packages/web-platform/web-style-transformer/src/transformer/constants.rs (2)
Learnt from: PupilTong
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.
Learnt from: PupilTong
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.
🔇 Additional comments (18)
packages/web-platform/inline-style-parser/src/types.rs (1)
4-5: LGTM! Appropriate import for string interop.The addition of
JsStringimport supports the package-wide refactoring from UTF-16 slices to JavaScript string interop types, which improves safety and idiomatic WebAssembly usage.packages/web-platform/web-style-transformer/Cargo.toml (1)
14-14: LGTM! Console feature enables enhanced debugging.The addition of
web-sysdependency with the "console" feature aligns with the safer string handling refactor and enables better debugging capabilities in the WebAssembly environment..changeset/cyan-comics-glow.md (1)
1-7: Changeset appropriately documents the string handling refactor.The changeset correctly identifies this as a feature change affecting both packages involved in the JsString adoption. Based on the retrieved learnings, this represents a meaningful improvement to the string handling architecture that warrants a changeset entry.
packages/web-platform/web-style-transformer/package.json (1)
25-25: Verify use of--debugin production buildI didn’t find any separate “release” or “prod” build task in packages/web-platform/web-style-transformer/package.json or in its turbo.json—you're only invoking:
• packages/web-platform/web-style-transformer/package.json, line 25
"build": "wasm-pack build --debug …"Using
--debugincreases bundle size and may impact runtime performance. Please confirm whether this is intentional for your production pipeline, or consider switching to a release‐optimized build (e.g.wasm-pack build --release) if you’re targeting production environments.packages/web-platform/inline-style-parser/src/parse_inline_style.rs (4)
8-11: LGTM!The conversion of
IMPORTANT_STRfrom[u16]to[u32]correctly maintains the same character codes while aligning with the newu32-based character handling.
13-23: LGTM!The struct field updates correctly transition from UTF-16 slice to
JsStringand useu32indices, which is appropriate for JavaScript string indexing.
82-86: LGTM - Correct JsString character access.The change from array indexing to
char_code_atmethod correctly adapts to theJsStringAPI while maintaining the same whitespace trimming logic.
132-159: LGTM!The trait and function signature updates correctly use
u32indices andJsString, with proper method calls likelength()for string length queries.packages/web-platform/web-style-transformer/src/transformer/rules.rs (2)
5-28: All RENAME_RULE entries have been migratedThe active
RENAME_RULEin
packages/web-platform/web-style-transformer/src/transformer/rules.rs
now defines the same eight mappings that were present in the commented-out UTF-16 version (lines 19–26), so no entries are missing.
30-261: Missing CSS transformation rules in REPLACE_RULE
An audit ofrules.rsshows 58 UTF-16 mappings still commented out versus 43 active string-based mappings—about 15 rule insertions appear not yet ported. Please confirm whether those omitted rules are intentionally dropped or need migrating, and ensure thatREPLACE_RULEcovers all required CSS transformations.• File: packages/web-platform/web-style-transformer/src/transformer/rules.rs
• Location: lines 30–261packages/web-platform/inline-style-parser/src/lib.rs (4)
12-65: LGTM - Comprehensive migration to JsString API.The test infrastructure correctly migrates from UTF-16 to
JsString:
- Declaration fields appropriately use
u32indices- Helper methods properly use
slice()andJsString::from()- Test setup function correctly creates
JsStringinstancesThe migration maintains all functionality while adopting the new API.
484-497: LGTM - Tests correctly adapted to JsString API.The test updates properly migrate from UTF-16 vectors to
JsStringwhile maintaining identical test logic and expectations. This validates that the API migration preserves functionality.
610-632: LGTM - Utility function tests properly updated.The utility function tests correctly migrate to
u32character codes andJsStringusage while maintaining comprehensive validation of core parsing functionality including string comparison and character classification.
715-802: LGTM - Tokenizer tests comprehensively updated.The tokenizer tests properly migrate to the new API:
TokenCollectorcorrectly usesu32indices- All test inputs properly converted to
JsString- Test coverage maintained for various token types and edge cases
This ensures the core tokenization functionality works correctly with the new string representation.
packages/web-platform/inline-style-parser/src/char_code_definitions.rs (4)
2-34: LGTM - Character category constants correctly migrated to u32.The migration from
u16tou32for character categories is comprehensive and correct:
- All constants properly updated to
u32- The
category_map_value_constfunction correctly handlesu32input- Array initialization and indexing properly handles the type conversion
This provides the foundation for consistent
u32-based character handling throughout the parser.
40-197: LGTM - Character classification macros properly updated.The migration of character classification macros from
u16tou32is correctly implemented:
- All numeric literals properly updated (removed
u16suffixes)- Macro logic preserved while working with
u32character codes- Comprehensive coverage of CSS character classification requirements
200-211: LGTM - Character category lookup correctly handles type conversion.The
char_code_category!macro properly casts theu32character code tousizefor array indexing while maintaining the same ASCII/non-ASCII classification logic.
213-251: LGTM - String access macros correctly adapted for JsString.The string access macros properly migrate to the
JsStringAPI:
cmp_char!correctly useschar_code_at()method and handles case-insensitive comparisonget_char_code!properly retrieves character codes with bounds checkingget_new_line_length!maintains correct CRLF handling logicThese macros provide the essential foundation for character-level operations in the parser.
| [depedencies] | ||
| js-sys = "0.3.77" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix the typo in the dependencies section header.
There's a typo in the section header that will cause the dependency to be ignored by Cargo.
-[depedencies]
-js-sys = "0.3.77"
+[dependencies]
+js-sys = "0.3.77"🤖 Prompt for AI Agents
In packages/web-platform/inline-style-parser/Cargo.toml at lines 6 to 7, fix the
typo in the section header by changing "[depedencies]" to "[dependencies]" so
that Cargo correctly recognizes and processes the dependency.
| [dependencies] | ||
| js-sys = "0.3.77" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Remove the duplicate dependencies section.
The correctly spelled [dependencies] section already includes the js-sys dependency, making the misspelled section redundant.
-[dependencies]
-js-sys = "0.3.77"Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/web-platform/inline-style-parser/Cargo.toml at lines 9 to 10, there
is a duplicate dependencies section with a misspelled header. Remove the
duplicate or incorrectly spelled dependencies section so that only the correctly
spelled [dependencies] section remains, which already includes the js-sys
dependency.
| export function transformInlineStyleString(str: string): string { | ||
| const { ptr, len } = stringToUTF16(str); | ||
| console.log(0, str); | ||
| // const { ptr, len } = stringToUTF16(str); | ||
| try { | ||
| const transformedStyle = wasm.transform_raw_u16_inline_style_ptr(ptr, len) | ||
| ?? str; | ||
| wasm.free(ptr, len << 1); | ||
| return transformedStyle; | ||
| console.log('start parse', str); | ||
| const res = wasm.transform_inline_style(str) ?? str; | ||
|
|
||
| // const transformedStyle = wasm.transform_raw_u16_inline_style_ptr(ptr, len) | ||
| // ?? str; | ||
| // wasm.free(ptr, len << 1); | ||
| console.log('return-value', res); | ||
| return res; | ||
| } catch (e) { | ||
| wasm.free(ptr, len << 1); | ||
| // wasm.free(ptr, len << 1); | ||
| throw e; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Clean up commented code and debug logging.
The function correctly transitions to direct string passing, but needs cleanup:
- Remove commented UTF-16 memory management code (lines 43, 48-50, 54)
- Remove debug console.log statements (lines 42, 45, 51) - these should not be in production code
- Remove unused
stringToUTF16function (lines 30-40) since it's no longer used
export function transformInlineStyleString(str: string): string {
- console.log(0, str);
- // const { ptr, len } = stringToUTF16(str);
try {
- console.log('start parse', str);
const res = wasm.transform_inline_style(str) ?? str;
-
- // const transformedStyle = wasm.transform_raw_u16_inline_style_ptr(ptr, len)
- // ?? str;
- // wasm.free(ptr, len << 1);
- console.log('return-value', res);
return res;
} catch (e) {
- // wasm.free(ptr, len << 1);
throw e;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function transformInlineStyleString(str: string): string { | |
| const { ptr, len } = stringToUTF16(str); | |
| console.log(0, str); | |
| // const { ptr, len } = stringToUTF16(str); | |
| try { | |
| const transformedStyle = wasm.transform_raw_u16_inline_style_ptr(ptr, len) | |
| ?? str; | |
| wasm.free(ptr, len << 1); | |
| return transformedStyle; | |
| console.log('start parse', str); | |
| const res = wasm.transform_inline_style(str) ?? str; | |
| // const transformedStyle = wasm.transform_raw_u16_inline_style_ptr(ptr, len) | |
| // ?? str; | |
| // wasm.free(ptr, len << 1); | |
| console.log('return-value', res); | |
| return res; | |
| } catch (e) { | |
| wasm.free(ptr, len << 1); | |
| // wasm.free(ptr, len << 1); | |
| throw e; | |
| } | |
| } | |
| export function transformInlineStyleString(str: string): string { | |
| try { | |
| const res = wasm.transform_inline_style(str) ?? str; | |
| return res; | |
| } catch (e) { | |
| throw e; | |
| } | |
| } |
🤖 Prompt for AI Agents
In packages/web-platform/web-mainthread-apis/src/utils/tokenizer.ts from lines
30 to 57, remove the unused stringToUTF16 function (lines 30-40), delete all
commented-out UTF-16 memory management code (lines 43, 48-50, 54), and eliminate
all console.log debug statements (lines 42, 45, 51) to clean up the function for
production readiness.
| #[macro_export] | ||
| macro_rules! str_to_u16_slice { | ||
| ($s:expr) => {{ | ||
| const S: &str = $s; | ||
| const LEN: usize = S.len(); | ||
| const fn make_array() -> [u16; LEN] { | ||
| let bytes = S.as_bytes(); | ||
| let mut result = [0u16; LEN]; | ||
| let mut i = 0; | ||
| while i < LEN { | ||
| result[i] = bytes[i] as u16; | ||
| i += 1; | ||
| } | ||
| result | ||
| } | ||
| const ARRAY: [u16; LEN] = make_array(); | ||
| &ARRAY | ||
| }}; | ||
| ($s:expr) => { | ||
| // { | ||
| // const S: &str = $s; | ||
| // const LEN: usize = S.len(); | ||
| // const fn make_array() -> [u16; LEN] { | ||
| // let bytes = S.as_bytes(); | ||
| // let mut result = [0u16; LEN]; | ||
| // let mut i = 0; | ||
| // while i < LEN { | ||
| // result[i] = bytes[i] as u16; | ||
| // i += 1; | ||
| // } | ||
| // result | ||
| // } | ||
| // const ARRAY: [u16; LEN] = make_array(); | ||
| // &ARRAY | ||
| // } | ||
| $s | ||
| }; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Remove str_to_u16_slice! macro and its usage
Since the macro now just returns the input string unchanged, it serves no purpose. Remove the macro definition and its usage throughout the file for cleaner code.
-#[macro_export]
-macro_rules! str_to_u16_slice {
- ($s:expr) => {
- // {
- // const S: &str = $s;
- // const LEN: usize = S.len();
- // const fn make_array() -> [u16; LEN] {
- // let bytes = S.as_bytes();
- // let mut result = [0u16; LEN];
- // let mut i = 0;
- // while i < LEN {
- // result[i] = bytes[i] as u16;
- // i += 1;
- // }
- // result
- // }
- // const ARRAY: [u16; LEN] = make_array();
- // &ARRAY
- // }
- $s
- };
-}Then update all usages like:
-pub const LIST_MAIN_AXIS_GAP_STR_U16: &str = str_to_u16_slice!("list-main-axis-gap");
+pub const LIST_MAIN_AXIS_GAP_STR_U16: &str = "list-main-axis-gap";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #[macro_export] | |
| macro_rules! str_to_u16_slice { | |
| ($s:expr) => {{ | |
| const S: &str = $s; | |
| const LEN: usize = S.len(); | |
| const fn make_array() -> [u16; LEN] { | |
| let bytes = S.as_bytes(); | |
| let mut result = [0u16; LEN]; | |
| let mut i = 0; | |
| while i < LEN { | |
| result[i] = bytes[i] as u16; | |
| i += 1; | |
| } | |
| result | |
| } | |
| const ARRAY: [u16; LEN] = make_array(); | |
| &ARRAY | |
| }}; | |
| ($s:expr) => { | |
| // { | |
| // const S: &str = $s; | |
| // const LEN: usize = S.len(); | |
| // const fn make_array() -> [u16; LEN] { | |
| // let bytes = S.as_bytes(); | |
| // let mut result = [0u16; LEN]; | |
| // let mut i = 0; | |
| // while i < LEN { | |
| // result[i] = bytes[i] as u16; | |
| // i += 1; | |
| // } | |
| // result | |
| // } | |
| // const ARRAY: [u16; LEN] = make_array(); | |
| // &ARRAY | |
| // } | |
| $s | |
| }; | |
| } | |
| // (entire `str_to_u16_slice!` macro definition removed) |
🤖 Prompt for AI Agents
In packages/web-platform/web-style-transformer/src/transformer/constants.rs
lines 1 to 22, the str_to_u16_slice! macro is redundant as it only returns the
input string unchanged. Remove the entire macro definition and replace all its
usages in the file with the original string literals directly to simplify and
clean up the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR migrates the inline style parser from low-level UTF-16 slice handling to JavaScript string APIs, improving safety and simplifying the codebase. The change replaces [u16] slice operations with js_sys::JsString methods throughout the parsing pipeline.
- Replaces UTF-16 slice operations with JavaScript string APIs across the parser
- Updates function signatures to use
js_sys::JsStringandu32offsets instead of[u16]slices andusize - Simplifies memory management by eliminating unsafe pointer operations
Reviewed Changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| transformer.rs | Updates core transformation logic to use JsString APIs instead of UTF-16 slices |
| rules.rs | Converts rule matching from slice operations to string-based lookups |
| constants.rs | Changes string constants from UTF-16 arrays to string literals |
| lib.rs | Updates public API to accept JsString directly instead of raw pointers |
| tokenizer.ts | Removes UTF-16 encoding/decoding and direct pointer management |
| utils.rs | Converts utility functions to work with JsString instead of slices |
| parse_inline_style.rs | Updates parser state to use JsString and u32 offsets |
| tokenize.rs | Modifies tokenizer to work with JsString character access |
| char_code_definitions.rs | Updates character category definitions for u32 character codes |
| // 2. If the next input code point is U+002B PLUS SIGN (+) or U+002D HYPHEN-MINUS (-), | ||
| // consume it and append it to repr. | ||
| if code == 0x002B || code == 0x002D { | ||
| if code == 0x002B4 || code == 0x002D { |
Copilot
AI
Jul 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The constant 0x002B4 appears to be incorrect. It should likely be 0x002B (plus sign). The value 0x002B4 (692 decimal) doesn't correspond to a standard ASCII character.
| if code == 0x002B4 || code == 0x002D { | |
| if code == 0x002B || code == 0x002D { |
| // code.toLowerCase() for A..Z | ||
| if code == $reference_code || (is_uppercase_letter!(code) && ((code | 32) == $reference_code)) | ||
| { | ||
| if code == $reference_code || (is_uppercase_letter!(code) && (code == $reference_code)) { |
Copilot
AI
Jul 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic for case-insensitive comparison is incorrect. The second condition (code == $reference_code) will never be true if the first condition failed. It should be ((code | 32) == $reference_code) to convert uppercase to lowercase.
| if code == $reference_code || (is_uppercase_letter!(code) && (code == $reference_code)) { | |
| if (code == $reference_code) || ((code | 32) == $reference_code) { |
| [depedencies] | ||
| js-sys = "0.3.77" | ||
|
|
Copilot
AI
Jul 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Misspelled section header depedencies should be dependencies.
| [depedencies] | |
| js-sys = "0.3.77" | |
| # Removed the misspelled `[depedencies]` section as it is redundant and incorrect. |
| ["--lynx-display-toggle", "var(--lynx-display-linear)"], | ||
| ["--lynx-display", "linear"], | ||
| ["display", "flex"], | ||
| ]); |
Copilot
AI
Jul 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The REPLACE_RULE HashMap is being returned with only a partial display_map entry. The commented out code suggests there should be more entries (direction_map, linear_orientation_map, etc.), but they are not being added to the map, which will break rule replacement functionality.
| ]); | |
| ]); | |
| map.insert("display", display_map); | |
| // Direction rules | |
| let mut direction_map = HashMap::new(); | |
| direction_map.insert("row", &[ | |
| ["--lynx-direction", "row"], | |
| ["direction", "ltr"], | |
| ]); | |
| direction_map.insert("column", &[ | |
| ["--lynx-direction", "column"], | |
| ["direction", "ttb"], | |
| ]); | |
| map.insert("direction", direction_map); | |
| // Linear orientation rules | |
| let mut linear_orientation_map = HashMap::new(); | |
| linear_orientation_map.insert("horizontal", &[ | |
| ["--lynx-orientation", "horizontal"], | |
| ["orientation", "landscape"], | |
| ]); | |
| linear_orientation_map.insert("vertical", &[ | |
| ["--lynx-orientation", "vertical"], | |
| ["orientation", "portrait"], | |
| ]); | |
| map.insert("linear-orientation", linear_orientation_map); |
| let mut result_children: Vec<(&'a [u16], usize, usize, &'a [u16], usize, usize)> = Vec::new(); | ||
| let mut result: Vec<(String, u32, u32, String, u32, u32)> = Vec::new(); | ||
| let mut result_children: Vec<(String, u32, u32, String, u32, u32)> = Vec::new(); | ||
| // let value_str = &value.as_string().to_stri; |
Copilot
AI
Jul 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incomplete commented code should be removed as it adds no value and may confuse developers.
| // let value_str = &value.as_string().to_stri; | |
| value_ptr: js_sys::JsString, | ||
| value_len: u32, | ||
| ) -> Option<js_sys::Array> { | ||
| unsafe { |
Copilot
AI
Jul 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function is marked as unsafe but no longer performs unsafe operations since it now uses JsString instead of raw pointers. The unsafe block should be removed.
7a1286d to
b4a072f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🔭 Outside diff range comments (3)
packages/web-platform/inline-style-parser/src/lib.rs (2)
507-528: Fix inconsistent type usage in character classification testsThe character classification macros are still being tested with
u16casts, but based on the PR objectives and changes in other files, these should be usingu32to align with the newchar_code_atreturn type.- assert!(is_digit!('0' as u16)); - assert!(is_digit!('9' as u16)); - assert!(!is_digit!('a' as u16)); + assert!(is_digit!('0' as u32)); + assert!(is_digit!('9' as u32)); + assert!(!is_digit!('a' as u32)); - assert!(is_hex_digit!('0' as u16)); - assert!(is_hex_digit!('A' as u16)); - assert!(is_hex_digit!('f' as u16)); - assert!(!is_hex_digit!('g' as u16)); + assert!(is_hex_digit!('0' as u32)); + assert!(is_hex_digit!('A' as u32)); + assert!(is_hex_digit!('f' as u32)); + assert!(!is_hex_digit!('g' as u32)); - assert!(is_uppercase_letter!('A' as u16)); - assert!(is_uppercase_letter!('Z' as u16)); - assert!(!is_uppercase_letter!('a' as u16)); + assert!(is_uppercase_letter!('A' as u32)); + assert!(is_uppercase_letter!('Z' as u32)); + assert!(!is_uppercase_letter!('a' as u32)); - assert!(is_lowercase_letter!('a' as u16)); - assert!(is_lowercase_letter!('z' as u16)); - assert!(!is_lowercase_letter!('A' as u16)); + assert!(is_lowercase_letter!('a' as u32)); + assert!(is_lowercase_letter!('z' as u32)); + assert!(!is_lowercase_letter!('A' as u32)); - assert!(is_letter!('A' as u16)); - assert!(is_letter!('z' as u16)); - assert!(!is_letter!('1' as u16)); + assert!(is_letter!('A' as u32)); + assert!(is_letter!('z' as u32)); + assert!(!is_letter!('1' as u32));
535-544: Fix inconsistent type usage in name classification testsContinue updating character classification tests to use
u32instead ofu16.- assert!(is_name_start!('a' as u16)); - assert!(is_name_start!('_' as u16)); + assert!(is_name_start!('a' as u32)); + assert!(is_name_start!('_' as u32)); assert!(is_name_start!(0x0080)); - assert!(!is_name_start!('1' as u16)); + assert!(!is_name_start!('1' as u32)); - assert!(is_name!('a' as u16)); - assert!(is_name!('1' as u16)); - assert!(is_name!('-' as u16)); - assert!(!is_name!(' ' as u16)); + assert!(is_name!('a' as u32)); + assert!(is_name!('1' as u32)); + assert!(is_name!('-' as u32)); + assert!(!is_name!(' ' as u32));packages/web-platform/web-style-transformer/src/transformer/transform.rs (1)
36-47: Fix incorrect implementation of is_digit_only macroThe
is_digit_onlymacro is incorrectly using.iter()on aJsStringslice and comparing withu16values.JsStringdoesn't have an.iter()method that returns character codes.macro_rules! is_digit_only { ($source:expr, $start:expr, $end:expr) => {{ let mut result = true; - for code in $source.slice($start, $end).iter() { - if code > b'9' as u16 || code < b'0' as u16 { + for i in $start..$end { + let code = $source.char_code_at(i) as u32; + if code > b'9' as u32 || code < b'0' as u32 { result = false; break; } } result }}; }
🧹 Nitpick comments (2)
packages/web-platform/web-style-transformer/src/transformer/transform.rs (2)
49-64: Rename macro to reflect current string typesThe macro name
push_u16_decl_pairsis misleading since it no longer works with u16 arrays but with string slices.-macro_rules! push_u16_decl_pairs { +macro_rules! push_str_decl_pairs { ($vec:expr, $pairs:expr) => { $vec.extend($pairs.iter().map(|replaced| { let decl_name = replaced[0]; let decl_value = replaced[1]; ( String::from(decl_name), 0 as u32, decl_name.len() as u32, String::from(decl_value), 0 as u32, decl_value.len() as u32, ) })); }; }Also update all usages of this macro throughout the file.
261-275: Avoid unnecessary String allocations in flex value transformationThe code creates temporary String objects from JsString slices only to immediately borrow them as &str. This causes unnecessary allocations.
Consider storing the slice bounds and creating strings only when needed:
// flex: <flex-grow> <flex-shrink> <flex-basis> - // &value[val_fields[0]..val_fields[1]] - let transformed_flex_values = &[ - &[ - FLEX_GROW_CSS_VAR_NAME, - &String::from(value.slice(val_fields[0], val_fields[1])), - ], - &[ - FLEX_SHRINK_CSS_VAR_NAME, - &String::from(value.slice(val_fields[2], val_fields[3])), - ], - &[ - FLEX_BASIS_CSS_VAR_NAME, - &String::from(value.slice(val_fields[4], val_fields[5])), - ], - ]; - push_u16_decl_pairs!(result, transformed_flex_values); + result.push(( + String::from(FLEX_GROW_CSS_VAR_NAME), + 0, + FLEX_GROW_CSS_VAR_NAME.len() as u32, + String::from(value), + val_fields[0], + val_fields[1], + )); + result.push(( + String::from(FLEX_SHRINK_CSS_VAR_NAME), + 0, + FLEX_SHRINK_CSS_VAR_NAME.len() as u32, + String::from(value), + val_fields[2], + val_fields[3], + )); + result.push(( + String::from(FLEX_BASIS_CSS_VAR_NAME), + 0, + FLEX_BASIS_CSS_VAR_NAME.len() as u32, + String::from(value), + val_fields[4], + val_fields[5], + ));
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
.changeset/config.json(1 hunks).changeset/cyan-comics-glow.md(1 hunks)packages/web-platform/inline-style-parser/Cargo.toml(1 hunks)packages/web-platform/inline-style-parser/src/char_code_definitions.rs(7 hunks)packages/web-platform/inline-style-parser/src/lib.rs(33 hunks)packages/web-platform/inline-style-parser/src/parse_inline_style.rs(4 hunks)packages/web-platform/inline-style-parser/src/tokenize.rs(9 hunks)packages/web-platform/inline-style-parser/src/types.rs(1 hunks)packages/web-platform/inline-style-parser/src/utils.rs(9 hunks)packages/web-platform/web-mainthread-apis/src/utils/tokenizer.ts(3 hunks)packages/web-platform/web-style-transformer/Cargo.toml(1 hunks)packages/web-platform/web-style-transformer/src/lib.rs(3 hunks)packages/web-platform/web-style-transformer/src/transformer/constants.rs(2 hunks)packages/web-platform/web-style-transformer/src/transformer/rules.rs(1 hunks)packages/web-platform/web-style-transformer/src/transformer/transform.rs(19 hunks)
✅ Files skipped from review due to trivial changes (3)
- packages/web-platform/inline-style-parser/src/types.rs
- .changeset/config.json
- packages/web-platform/inline-style-parser/Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (10)
- .changeset/cyan-comics-glow.md
- packages/web-platform/web-style-transformer/Cargo.toml
- packages/web-platform/web-mainthread-apis/src/utils/tokenizer.ts
- packages/web-platform/inline-style-parser/src/utils.rs
- packages/web-platform/inline-style-parser/src/parse_inline_style.rs
- packages/web-platform/web-style-transformer/src/transformer/rules.rs
- packages/web-platform/web-style-transformer/src/lib.rs
- packages/web-platform/inline-style-parser/src/char_code_definitions.rs
- packages/web-platform/inline-style-parser/src/tokenize.rs
- packages/web-platform/web-style-transformer/src/transformer/constants.rs
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.
Learnt from: PupilTong
PR: lynx-family/lynx-stack#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.
Learnt from: PupilTong
PR: lynx-family/lynx-stack#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.
Learnt from: PupilTong
PR: lynx-family/lynx-stack#1029
File: packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts:95-99
Timestamp: 2025-07-16T05:57:29.837Z
Learning: In the lynx-stack codebase, PupilTong prefers avoiding regex for parsing in performance-critical code paths like SSR hydration, preferring simple string manipulation operations even if they're less robust, when the input format is predictable and controlled.
🧬 Code Graph Analysis (1)
packages/web-platform/inline-style-parser/src/lib.rs (3)
packages/web-platform/inline-style-parser/src/utils.rs (7)
cmp_str(3-26)find_white_space_end(28-38)find_decimal_number_end(40-50)consume_number(110-177)consume_name(81-107)consume_escaped(53-77)consume_bad_url_remnants(182-204)packages/web-platform/inline-style-parser/src/parse_inline_style.rs (1)
on_token(26-129)packages/web-platform/inline-style-parser/src/tokenize.rs (2)
on_token(254-254)tokenize(257-544)
packages/web-platform/web-style-transformer/src/transformer/transform.rs
Show resolved
Hide resolved
packages/web-platform/web-style-transformer/src/transformer/transform.rs
Outdated
Show resolved
Hide resolved
b4a072f to
3446aac
Compare
Web Explorer#3418 Bundle Size — 395.55KiB (+12.21%).5f3a2c7(current) vs 54eb5bf main#3410(baseline) Bundle metrics
Bundle size by type
Bundle analysis report Branch Sherry-hue:feat/style-parser-str... Project dashboard Generated by RelativeCI Documentation Report issue |
React Example#3428 Bundle Size — 235.18KiB (0%).5f3a2c7(current) vs 54eb5bf main#3420(baseline) Bundle metrics
|
| Current #3428 |
Baseline #3420 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
4 |
4 |
|
156 |
156 |
|
63 |
63 |
|
45.94% |
45.94% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #3428 |
Baseline #3420 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
89.42KiB |
89.42KiB |
Bundle analysis report Branch Sherry-hue:feat/style-parser-str... Project dashboard
Generated by RelativeCI Documentation Report issue
3446aac to
5f3a2c7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🔭 Outside diff range comments (2)
packages/web-platform/web-style-transformer/src/lib.rs (1)
121-139: Remove unusedmalloc/freeAPIs from web-style-transformerThe RG search shows no live uses of these functions (only commented-out frees in
tokenizer.ts), so after theJsStringmigration we can drop both Rust and TS bits:• In
packages/web-platform/web-style-transformer/src/lib.rs
– Removepub fn malloc(size: usize)…
– Remove#[wasm_bindgen] pub unsafe fn free(ptr: *mut u8, size: usize)…• In
packages/web-platform/web-style-transformer/standard.d.ts
– Delete theexport function malloc(size: number): number;andexport function free(ptr: number, size: number): void;declarationsThis cleans up dead code and simplifies the public API.
packages/web-platform/web-style-transformer/src/transformer/transform.rs (1)
36-47: Fix potential type mismatch in digit checking macro.The
is_digit_onlymacro compares character codes withu16byte values, butJsString::char_code_at()returnsf64. This could cause incorrect comparisons.macro_rules! is_digit_only { ($source:expr, $start:expr, $end:expr) => {{ let mut result = true; - for code in $source.slice($start, $end).iter() { - if code > b'9' as u16 || code < b'0' as u16 { + for i in $start..$end { + let code = $source.char_code_at(i) as u32; + if code > b'9' as u32 || code < b'0' as u32 { result = false; break; } } result }}; }
♻️ Duplicate comments (5)
packages/web-platform/web-style-transformer/src/transformer/rules.rs (1)
31-42: Critical issue: REPLACE_RULE map is incomplete and will break functionality.The
REPLACE_RULEmap only contains display rules, but all other transformation rules (direction, linear-orientation, linear-direction, linear-gravity, etc.) that were previously defined are now commented out. This will cause rule replacement functionality to fail for most CSS properties.You need to either:
- Migrate the commented UTF-16 rules to string-based equivalents, or
- Ensure this is intentional and update dependent code to handle missing rules gracefully
map.insert("display", display_map); + + // Add the missing rule maps that were commented out + // Direction rules, linear-orientation rules, etc. need to be restored + // as string-based equivalentspackages/web-platform/web-style-transformer/src/lib.rs (1)
97-118: Remove unnecessary unsafe block.The
unsafeblock is no longer needed since the function now usesJsStringparameters instead of raw pointers, and no unsafe operations are performed within the block.- unsafe { - // let name_slice = core::slice::from_raw_parts(name_ptr, name_len); - // let value_slice = core::slice::from_raw_parts(value_ptr, value_len); // Call the tokenize function with our data and callback let (result, children_result) = transformer::transform::query_transform_rules( - &name_ptr, 0 as u32, name_len, &value_ptr, 0 as u32, value_len, + &name_ptr, 0, name_len, &value_ptr, 0, value_len, ); // ... rest of the function Some(ret) - }packages/web-platform/web-style-transformer/src/transformer/transform.rs (3)
348-355: Fix incorrect string slicing in Transformer implementation.The code attempts to slice
Stringobjects using array indexing, which doesn't work in Rust. Strings must be sliced using byte indices, and the current approach may cause panics.// Append the declaration name and colon - self.transformed_source.push(String::from( - &decl_name[*decl_name_start as usize..*decl_name_end as usize], - )); + self.transformed_source.push( + decl_name[*decl_name_start as usize..*decl_name_end as usize].to_string() + ); self.transformed_source.push(String::from(":")); // Append the declaration value - self.transformed_source.push(String::from( - &decl_value[*decl_value_start as usize..*decl_value_end as usize], - )); + self.transformed_source.push( + decl_value[*decl_value_start as usize..*decl_value_end as usize].to_string() + );
370-377: Fix string slicing in children styles as well.The same string slicing issue exists in the children styles processing code.
Apply the same fix as above:
// Append the declaration name and colon - self.extra_children_styles.push(String::from( - &decl_name[*decl_name_start as usize..*decl_name_end as usize], - )); + self.extra_children_styles.push( + decl_name[*decl_name_start as usize..*decl_name_end as usize].to_string() + ); self.extra_children_styles.push(String::from(":")); // Append the declaration value - self.extra_children_styles.push(String::from( - &decl_value[*decl_value_start as usize..*decl_value_end as usize], - )); + self.extra_children_styles.push( + decl_value[*decl_value_start as usize..*decl_value_end as usize].to_string() + );
406-411: Optimize string concatenation and handle remaining content.The current implementation uses
flat_mapover characters which is inefficient for string concatenation, and doesn't handle remaining content after the last declaration.pub fn transform_inline_style_string<'a>(source: &'a js_sys::JsString) -> String { let mut transformer: TransformerData<'a> = TransformerData { source, transformed_source: Vec::new(), offset: 0, extra_children_styles: Vec::new(), }; parse_inline_style::parse_inline_style(source, &mut transformer); - // if transformer.offset != 0 { - // // append the remaining part of the source - // transformer - // .transformed_source - // .extend_from_slice(&source[transformer.offset..]); - // } - // String::from_utf16(&transformer.transformed_source).unwrap() - // console::log_1(&transformer.transformed_source); - transformer - .transformed_source - .iter() - .flat_map(|s| s.chars()) - .collect() - // transformer.transformed_source + + // Append any remaining content after the last declaration + if transformer.offset < source.length() { + transformer.transformed_source.push( + String::from(source.slice(transformer.offset, source.length())) + ); + } + + // Efficiently join all strings + transformer.transformed_source.join("") }
🧹 Nitpick comments (2)
packages/web-platform/web-style-transformer/src/transformer/rules.rs (1)
268-291: Optimize macro string conversions for better performance.The macros are converting
JsStringslices toStringobjects for every lookup, which creates unnecessary allocations. Consider caching the string conversion or using a more efficient lookup approach.macro_rules! get_rename_rule_value { ($name_slice:expr, $start:expr, $end:expr) => { crate::transformer::rules::RENAME_RULE - .get(&*String::from( - $name_slice.slice($start as u32, $end as u32), - )) + .get($name_slice.slice($start as u32, $end as u32).as_string()?.as_str()) .copied() }; }packages/web-platform/web-style-transformer/src/lib.rs (1)
71-72: Optimize macro string slicing for better performance.The macro creates string slices that could be optimized to avoid unnecessary allocations when creating JsString objects from string slices.
- let k = js_sys::JsString::from(&name_source[name_start as usize..name_end as usize]); - let v = js_sys::JsString::from(&value_source[value_start as usize..value_end as usize]); + let k = js_sys::JsString::from(&name_source[name_start as usize..name_end as usize]); + let v = js_sys::JsString::from(&value_source[value_start as usize..value_end as usize]);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
.changeset/config.json(1 hunks).changeset/cyan-comics-glow.md(1 hunks)packages/web-platform/inline-style-parser/Cargo.toml(1 hunks)packages/web-platform/inline-style-parser/src/char_code_definitions.rs(7 hunks)packages/web-platform/inline-style-parser/src/lib.rs(33 hunks)packages/web-platform/inline-style-parser/src/parse_inline_style.rs(4 hunks)packages/web-platform/inline-style-parser/src/tokenize.rs(9 hunks)packages/web-platform/inline-style-parser/src/utils.rs(9 hunks)packages/web-platform/web-mainthread-apis/src/utils/tokenizer.ts(3 hunks)packages/web-platform/web-style-transformer/Cargo.toml(1 hunks)packages/web-platform/web-style-transformer/src/lib.rs(3 hunks)packages/web-platform/web-style-transformer/src/transformer/constants.rs(2 hunks)packages/web-platform/web-style-transformer/src/transformer/rules.rs(1 hunks)packages/web-platform/web-style-transformer/src/transformer/transform.rs(19 hunks)packages/web-platform/web-style-transformer/standard.d.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/web-platform/inline-style-parser/Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (11)
- .changeset/cyan-comics-glow.md
- packages/web-platform/web-mainthread-apis/src/utils/tokenizer.ts
- packages/web-platform/web-style-transformer/Cargo.toml
- packages/web-platform/inline-style-parser/src/utils.rs
- .changeset/config.json
- packages/web-platform/web-style-transformer/standard.d.ts
- packages/web-platform/inline-style-parser/src/parse_inline_style.rs
- packages/web-platform/inline-style-parser/src/tokenize.rs
- packages/web-platform/web-style-transformer/src/transformer/constants.rs
- packages/web-platform/inline-style-parser/src/lib.rs
- packages/web-platform/inline-style-parser/src/char_code_definitions.rs
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.
Learnt from: PupilTong
PR: lynx-family/lynx-stack#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.
packages/web-platform/web-style-transformer/src/lib.rs (1)
Learnt from: PupilTong
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.
packages/web-platform/web-style-transformer/src/transformer/transform.rs (1)
Learnt from: PupilTong
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.
🔇 Additional comments (1)
packages/web-platform/web-style-transformer/src/transformer/transform.rs (1)
418-662: Excellent test coverage for the refactored functionality.The comprehensive test suite has been successfully updated to work with
js_sys::JsStringinputs and covers various transformation scenarios including flex properties, color handling, and complex CSS rules. This maintains good test coverage during the migration.
| // pub static ref REPLACE_RULE: HashMap<&'static [u16], HashMap<&'static [u16], &'static [[&'static [u16]; 2]]>> = { | ||
| // let mut map = HashMap::new(); | ||
|
|
||
| // Linear orientation rules | ||
| let mut linear_orientation_map = HashMap::new(); | ||
| linear_orientation_map.insert(HORIZONTAL_STR_U16, &[ | ||
| [LYNX_LINEAR_ORIENTATION_STR_U16, HORIZONTAL_STR_U16], | ||
| [LYNX_LINEAR_ORIENTATION_TOGGLE_STR_U16, LYNX_LINEAR_ORIENTATION_TOGGLE_HORIZONTAL_VALUE_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| linear_orientation_map.insert(HORIZONTAL_REVERSE_STR_U16, &[ | ||
| [LYNX_LINEAR_ORIENTATION_STR_U16, HORIZONTAL_REVERSE_STR_U16], | ||
| [LYNX_LINEAR_ORIENTATION_TOGGLE_STR_U16, LYNX_LINEAR_ORIENTATION_TOGGLE_HORIZONTAL_REVERSE_VALUE_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| linear_orientation_map.insert(VERTICAL_STR_U16, &[ | ||
| [LYNX_LINEAR_ORIENTATION_STR_U16, VERTICAL_STR_U16], | ||
| [LYNX_LINEAR_ORIENTATION_TOGGLE_STR_U16, LYNX_LINEAR_ORIENTATION_TOGGLE_VERTICAL_VALUE_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| linear_orientation_map.insert(VERTICAL_REVERSE_STR_U16, &[ | ||
| [LYNX_LINEAR_ORIENTATION_STR_U16, VERTICAL_REVERSE_STR_U16], | ||
| [LYNX_LINEAR_ORIENTATION_TOGGLE_STR_U16, LYNX_LINEAR_ORIENTATION_TOGGLE_VERTICAL_REVERSE_VALUE_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| // // Display rules | ||
| // let mut display_map = HashMap::new(); | ||
| // display_map.insert(LINEAR_STR_U16, &[ | ||
| // [LYNX_DISPLAY_TOGGLE_STR_U16, LYNX_DISPLAY_LINEAR_VAR_STR_U16], | ||
| // [LYNX_DISPLAY_STR_U16, LINEAR_STR_U16], | ||
| // [DISPLAY_STR_U16, FLEX_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
| // display_map.insert(FLEX_STR_U16, &[ | ||
| // [LYNX_DISPLAY_TOGGLE_STR_U16, LYNX_DISPLAY_FLEX_VAR_STR_U16], | ||
| // [LYNX_DISPLAY_STR_U16, FLEX_STR_U16], | ||
| // [DISPLAY_STR_U16, FLEX_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
|
|
||
| // Linear direction rules | ||
| let mut linear_direction_map = HashMap::new(); | ||
| linear_direction_map.insert(ROW_STR_U16, &[ | ||
| [LYNX_LINEAR_ORIENTATION_STR_U16, HORIZONTAL_STR_U16], | ||
| [LYNX_LINEAR_ORIENTATION_TOGGLE_STR_U16, LYNX_LINEAR_ORIENTATION_TOGGLE_HORIZONTAL_VALUE_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| linear_direction_map.insert(ROW_REVERSE_STR_U16, &[ | ||
| [LYNX_LINEAR_ORIENTATION_STR_U16, HORIZONTAL_REVERSE_STR_U16], | ||
| [LYNX_LINEAR_ORIENTATION_TOGGLE_STR_U16, LYNX_LINEAR_ORIENTATION_TOGGLE_HORIZONTAL_REVERSE_VALUE_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| linear_direction_map.insert(COLUMN_STR_U16, &[ | ||
| [LYNX_LINEAR_ORIENTATION_STR_U16, VERTICAL_STR_U16], | ||
| [LYNX_LINEAR_ORIENTATION_TOGGLE_STR_U16, LYNX_LINEAR_ORIENTATION_TOGGLE_VERTICAL_VALUE_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| linear_direction_map.insert(COLUMN_REVERSE_STR_U16, &[ | ||
| [LYNX_LINEAR_ORIENTATION_STR_U16, VERTICAL_REVERSE_STR_U16], | ||
| [LYNX_LINEAR_ORIENTATION_TOGGLE_STR_U16, LYNX_LINEAR_ORIENTATION_TOGGLE_VERTICAL_REVERSE_VALUE_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| // // Direction rules | ||
| // let mut direction_map = HashMap::new(); | ||
| // direction_map.insert(LYNX_RTL_STR_U16, &[ | ||
| // [DIRECTION_STR_U16, RTL_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
|
|
||
| // Linear gravity rules | ||
| let mut linear_gravity_map = HashMap::new(); | ||
| linear_gravity_map.insert(TOP_STR_U16, &[ | ||
| [JUSTIFY_CONTENT_COLUMN_STR_U16, FLEX_START_STR_U16], | ||
| [JUSTIFY_CONTENT_COLUMN_REVERSE_STR_U16, FLEX_END_STR_U16], | ||
| [JUSTIFY_CONTENT_ROW_STR_U16, FLEX_START_STR_U16], | ||
| [JUSTIFY_CONTENT_ROW_REVERSE_STR_U16, FLEX_START_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| linear_gravity_map.insert(BOTTOM_STR_U16, &[ | ||
| [JUSTIFY_CONTENT_COLUMN_STR_U16, FLEX_END_STR_U16], | ||
| [JUSTIFY_CONTENT_COLUMN_REVERSE_STR_U16, FLEX_START_STR_U16], | ||
| [JUSTIFY_CONTENT_ROW_STR_U16, FLEX_START_STR_U16], | ||
| [JUSTIFY_CONTENT_ROW_REVERSE_STR_U16, FLEX_START_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| linear_gravity_map.insert(LEFT_STR_U16, &[ | ||
| [JUSTIFY_CONTENT_COLUMN_STR_U16, FLEX_START_STR_U16], | ||
| [JUSTIFY_CONTENT_COLUMN_REVERSE_STR_U16, FLEX_START_STR_U16], | ||
| [JUSTIFY_CONTENT_ROW_STR_U16, FLEX_START_STR_U16], | ||
| [JUSTIFY_CONTENT_ROW_REVERSE_STR_U16, FLEX_END_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| linear_gravity_map.insert(RIGHT_STR_U16, &[ | ||
| [JUSTIFY_CONTENT_COLUMN_STR_U16, FLEX_START_STR_U16], | ||
| [JUSTIFY_CONTENT_COLUMN_REVERSE_STR_U16, FLEX_START_STR_U16], | ||
| [JUSTIFY_CONTENT_ROW_STR_U16, FLEX_END_STR_U16], | ||
| [JUSTIFY_CONTENT_ROW_REVERSE_STR_U16, FLEX_START_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| linear_gravity_map.insert(CENTER_VERTICAL_STR_U16, &[ | ||
| [JUSTIFY_CONTENT_COLUMN_STR_U16, CENTER_STR_U16], | ||
| [JUSTIFY_CONTENT_COLUMN_REVERSE_STR_U16, CENTER_STR_U16], | ||
| [JUSTIFY_CONTENT_ROW_STR_U16, FLEX_START_STR_U16], | ||
| [JUSTIFY_CONTENT_ROW_REVERSE_STR_U16, FLEX_START_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| linear_gravity_map.insert(CENTER_HORIZONTAL_STR_U16, &[ | ||
| [JUSTIFY_CONTENT_COLUMN_STR_U16, FLEX_START_STR_U16], | ||
| [JUSTIFY_CONTENT_COLUMN_REVERSE_STR_U16, FLEX_START_STR_U16], | ||
| [JUSTIFY_CONTENT_ROW_STR_U16, CENTER_STR_U16], | ||
| [JUSTIFY_CONTENT_ROW_REVERSE_STR_U16, CENTER_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| linear_gravity_map.insert(START_STR_U16, &[ | ||
| [JUSTIFY_CONTENT_COLUMN_STR_U16, FLEX_START_STR_U16], | ||
| [JUSTIFY_CONTENT_COLUMN_REVERSE_STR_U16, FLEX_START_STR_U16], | ||
| [JUSTIFY_CONTENT_ROW_STR_U16, FLEX_START_STR_U16], | ||
| [JUSTIFY_CONTENT_ROW_REVERSE_STR_U16, FLEX_START_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| linear_gravity_map.insert(END_STR_U16, &[ | ||
| [JUSTIFY_CONTENT_COLUMN_STR_U16, FLEX_END_STR_U16], | ||
| [JUSTIFY_CONTENT_COLUMN_REVERSE_STR_U16, FLEX_END_STR_U16], | ||
| [JUSTIFY_CONTENT_ROW_STR_U16, FLEX_END_STR_U16], | ||
| [JUSTIFY_CONTENT_ROW_REVERSE_STR_U16, FLEX_END_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| linear_gravity_map.insert(CENTER_STR_U16, &[ | ||
| [JUSTIFY_CONTENT_COLUMN_STR_U16, CENTER_STR_U16], | ||
| [JUSTIFY_CONTENT_COLUMN_REVERSE_STR_U16, CENTER_STR_U16], | ||
| [JUSTIFY_CONTENT_ROW_STR_U16, CENTER_STR_U16], | ||
| [JUSTIFY_CONTENT_ROW_REVERSE_STR_U16, CENTER_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| linear_gravity_map.insert(SPACE_BETWEEN_STR_U16, &[ | ||
| [JUSTIFY_CONTENT_COLUMN_STR_U16, SPACE_BETWEEN_STR_U16], | ||
| [JUSTIFY_CONTENT_COLUMN_REVERSE_STR_U16, SPACE_BETWEEN_STR_U16], | ||
| [JUSTIFY_CONTENT_ROW_STR_U16, SPACE_BETWEEN_STR_U16], | ||
| [JUSTIFY_CONTENT_ROW_REVERSE_STR_U16, SPACE_BETWEEN_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| // // Linear orientation rules | ||
| // let mut linear_orientation_map = HashMap::new(); | ||
| // linear_orientation_map.insert(HORIZONTAL_STR_U16, &[ | ||
| // [LYNX_LINEAR_ORIENTATION_STR_U16, HORIZONTAL_STR_U16], | ||
| // [LYNX_LINEAR_ORIENTATION_TOGGLE_STR_U16, LYNX_LINEAR_ORIENTATION_TOGGLE_HORIZONTAL_VALUE_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
| // linear_orientation_map.insert(HORIZONTAL_REVERSE_STR_U16, &[ | ||
| // [LYNX_LINEAR_ORIENTATION_STR_U16, HORIZONTAL_REVERSE_STR_U16], | ||
| // [LYNX_LINEAR_ORIENTATION_TOGGLE_STR_U16, LYNX_LINEAR_ORIENTATION_TOGGLE_HORIZONTAL_REVERSE_VALUE_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
| // linear_orientation_map.insert(VERTICAL_STR_U16, &[ | ||
| // [LYNX_LINEAR_ORIENTATION_STR_U16, VERTICAL_STR_U16], | ||
| // [LYNX_LINEAR_ORIENTATION_TOGGLE_STR_U16, LYNX_LINEAR_ORIENTATION_TOGGLE_VERTICAL_VALUE_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
| // linear_orientation_map.insert(VERTICAL_REVERSE_STR_U16, &[ | ||
| // [LYNX_LINEAR_ORIENTATION_STR_U16, VERTICAL_REVERSE_STR_U16], | ||
| // [LYNX_LINEAR_ORIENTATION_TOGGLE_STR_U16, LYNX_LINEAR_ORIENTATION_TOGGLE_VERTICAL_REVERSE_VALUE_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
|
|
||
| // Linear cross gravity rules | ||
| let mut linear_cross_gravity_map = HashMap::new(); | ||
| linear_cross_gravity_map.insert(START_STR_U16, &[ | ||
| [ALIGN_ITEMS_STR_U16, START_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| linear_cross_gravity_map.insert(END_STR_U16, &[ | ||
| [ALIGN_ITEMS_STR_U16, END_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| linear_cross_gravity_map.insert(CENTER_STR_U16, &[ | ||
| [ALIGN_ITEMS_STR_U16, CENTER_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| linear_cross_gravity_map.insert(STRETCH_STR_U16, &[ | ||
| [ALIGN_ITEMS_STR_U16, STRETCH_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| // // Linear direction rules | ||
| // let mut linear_direction_map = HashMap::new(); | ||
| // linear_direction_map.insert(ROW_STR_U16, &[ | ||
| // [LYNX_LINEAR_ORIENTATION_STR_U16, HORIZONTAL_STR_U16], | ||
| // [LYNX_LINEAR_ORIENTATION_TOGGLE_STR_U16, LYNX_LINEAR_ORIENTATION_TOGGLE_HORIZONTAL_VALUE_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
| // linear_direction_map.insert(ROW_REVERSE_STR_U16, &[ | ||
| // [LYNX_LINEAR_ORIENTATION_STR_U16, HORIZONTAL_REVERSE_STR_U16], | ||
| // [LYNX_LINEAR_ORIENTATION_TOGGLE_STR_U16, LYNX_LINEAR_ORIENTATION_TOGGLE_HORIZONTAL_REVERSE_VALUE_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
| // linear_direction_map.insert(COLUMN_STR_U16, &[ | ||
| // [LYNX_LINEAR_ORIENTATION_STR_U16, VERTICAL_STR_U16], | ||
| // [LYNX_LINEAR_ORIENTATION_TOGGLE_STR_U16, LYNX_LINEAR_ORIENTATION_TOGGLE_VERTICAL_VALUE_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
| // linear_direction_map.insert(COLUMN_REVERSE_STR_U16, &[ | ||
| // [LYNX_LINEAR_ORIENTATION_STR_U16, VERTICAL_REVERSE_STR_U16], | ||
| // [LYNX_LINEAR_ORIENTATION_TOGGLE_STR_U16, LYNX_LINEAR_ORIENTATION_TOGGLE_VERTICAL_REVERSE_VALUE_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
|
|
||
| // Linear layout gravity rules | ||
| let mut linear_layout_gravity_map = HashMap::new(); | ||
| linear_layout_gravity_map.insert(NONE_STR_U16, &[ | ||
| [ALIGN_SELF_ROW_STR_U16, AUTO_STR_U16], | ||
| [ALIGN_SELF_COLUMN_STR_U16, AUTO_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| linear_layout_gravity_map.insert(STRETCH_STR_U16, &[ | ||
| [ALIGN_SELF_ROW_STR_U16, STRETCH_STR_U16], | ||
| [ALIGN_SELF_COLUMN_STR_U16, STRETCH_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| linear_layout_gravity_map.insert(TOP_STR_U16, &[ | ||
| [ALIGN_SELF_ROW_STR_U16, START_STR_U16], | ||
| [ALIGN_SELF_COLUMN_STR_U16, AUTO_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| linear_layout_gravity_map.insert(BOTTOM_STR_U16, &[ | ||
| [ALIGN_SELF_ROW_STR_U16, END_STR_U16], | ||
| [ALIGN_SELF_COLUMN_STR_U16, AUTO_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| linear_layout_gravity_map.insert(LEFT_STR_U16, &[ | ||
| [ALIGN_SELF_ROW_STR_U16, AUTO_STR_U16], | ||
| [ALIGN_SELF_COLUMN_STR_U16, START_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| linear_layout_gravity_map.insert(RIGHT_STR_U16, &[ | ||
| [ALIGN_SELF_ROW_STR_U16, AUTO_STR_U16], | ||
| [ALIGN_SELF_COLUMN_STR_U16, END_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| linear_layout_gravity_map.insert(START_STR_U16, &[ | ||
| [ALIGN_SELF_ROW_STR_U16, START_STR_U16], | ||
| [ALIGN_SELF_COLUMN_STR_U16, START_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| linear_layout_gravity_map.insert(END_STR_U16, &[ | ||
| [ALIGN_SELF_ROW_STR_U16, END_STR_U16], | ||
| [ALIGN_SELF_COLUMN_STR_U16, END_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| linear_layout_gravity_map.insert(CENTER_STR_U16, &[ | ||
| [ALIGN_SELF_ROW_STR_U16, CENTER_STR_U16], | ||
| [ALIGN_SELF_COLUMN_STR_U16, CENTER_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| linear_layout_gravity_map.insert(CENTER_VERTICAL_STR_U16, &[ | ||
| [ALIGN_SELF_ROW_STR_U16, CENTER_STR_U16], | ||
| [ALIGN_SELF_COLUMN_STR_U16, START_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| linear_layout_gravity_map.insert(CENTER_HORIZONTAL_STR_U16, &[ | ||
| [ALIGN_SELF_ROW_STR_U16, START_STR_U16], | ||
| [ALIGN_SELF_COLUMN_STR_U16, CENTER_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| linear_layout_gravity_map.insert(FILL_VERTICAL_STR_U16, &[ | ||
| [ALIGN_SELF_ROW_STR_U16, STRETCH_STR_U16], | ||
| [ALIGN_SELF_COLUMN_STR_U16, AUTO_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| linear_layout_gravity_map.insert(FILL_HORIZONTAL_STR_U16, &[ | ||
| [ALIGN_SELF_ROW_STR_U16, AUTO_STR_U16], | ||
| [ALIGN_SELF_COLUMN_STR_U16, STRETCH_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| // // Linear gravity rules | ||
| // let mut linear_gravity_map = HashMap::new(); | ||
| // linear_gravity_map.insert(TOP_STR_U16, &[ | ||
| // [JUSTIFY_CONTENT_COLUMN_STR_U16, FLEX_START_STR_U16], | ||
| // [JUSTIFY_CONTENT_COLUMN_REVERSE_STR_U16, FLEX_END_STR_U16], | ||
| // [JUSTIFY_CONTENT_ROW_STR_U16, FLEX_START_STR_U16], | ||
| // [JUSTIFY_CONTENT_ROW_REVERSE_STR_U16, FLEX_START_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
| // linear_gravity_map.insert(BOTTOM_STR_U16, &[ | ||
| // [JUSTIFY_CONTENT_COLUMN_STR_U16, FLEX_END_STR_U16], | ||
| // [JUSTIFY_CONTENT_COLUMN_REVERSE_STR_U16, FLEX_START_STR_U16], | ||
| // [JUSTIFY_CONTENT_ROW_STR_U16, FLEX_START_STR_U16], | ||
| // [JUSTIFY_CONTENT_ROW_REVERSE_STR_U16, FLEX_START_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
| // linear_gravity_map.insert(LEFT_STR_U16, &[ | ||
| // [JUSTIFY_CONTENT_COLUMN_STR_U16, FLEX_START_STR_U16], | ||
| // [JUSTIFY_CONTENT_COLUMN_REVERSE_STR_U16, FLEX_START_STR_U16], | ||
| // [JUSTIFY_CONTENT_ROW_STR_U16, FLEX_START_STR_U16], | ||
| // [JUSTIFY_CONTENT_ROW_REVERSE_STR_U16, FLEX_END_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
| // linear_gravity_map.insert(RIGHT_STR_U16, &[ | ||
| // [JUSTIFY_CONTENT_COLUMN_STR_U16, FLEX_START_STR_U16], | ||
| // [JUSTIFY_CONTENT_COLUMN_REVERSE_STR_U16, FLEX_START_STR_U16], | ||
| // [JUSTIFY_CONTENT_ROW_STR_U16, FLEX_END_STR_U16], | ||
| // [JUSTIFY_CONTENT_ROW_REVERSE_STR_U16, FLEX_START_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
| // linear_gravity_map.insert(CENTER_VERTICAL_STR_U16, &[ | ||
| // [JUSTIFY_CONTENT_COLUMN_STR_U16, CENTER_STR_U16], | ||
| // [JUSTIFY_CONTENT_COLUMN_REVERSE_STR_U16, CENTER_STR_U16], | ||
| // [JUSTIFY_CONTENT_ROW_STR_U16, FLEX_START_STR_U16], | ||
| // [JUSTIFY_CONTENT_ROW_REVERSE_STR_U16, FLEX_START_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
| // linear_gravity_map.insert(CENTER_HORIZONTAL_STR_U16, &[ | ||
| // [JUSTIFY_CONTENT_COLUMN_STR_U16, FLEX_START_STR_U16], | ||
| // [JUSTIFY_CONTENT_COLUMN_REVERSE_STR_U16, FLEX_START_STR_U16], | ||
| // [JUSTIFY_CONTENT_ROW_STR_U16, CENTER_STR_U16], | ||
| // [JUSTIFY_CONTENT_ROW_REVERSE_STR_U16, CENTER_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
| // linear_gravity_map.insert(START_STR_U16, &[ | ||
| // [JUSTIFY_CONTENT_COLUMN_STR_U16, FLEX_START_STR_U16], | ||
| // [JUSTIFY_CONTENT_COLUMN_REVERSE_STR_U16, FLEX_START_STR_U16], | ||
| // [JUSTIFY_CONTENT_ROW_STR_U16, FLEX_START_STR_U16], | ||
| // [JUSTIFY_CONTENT_ROW_REVERSE_STR_U16, FLEX_START_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
| // linear_gravity_map.insert(END_STR_U16, &[ | ||
| // [JUSTIFY_CONTENT_COLUMN_STR_U16, FLEX_END_STR_U16], | ||
| // [JUSTIFY_CONTENT_COLUMN_REVERSE_STR_U16, FLEX_END_STR_U16], | ||
| // [JUSTIFY_CONTENT_ROW_STR_U16, FLEX_END_STR_U16], | ||
| // [JUSTIFY_CONTENT_ROW_REVERSE_STR_U16, FLEX_END_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
| // linear_gravity_map.insert(CENTER_STR_U16, &[ | ||
| // [JUSTIFY_CONTENT_COLUMN_STR_U16, CENTER_STR_U16], | ||
| // [JUSTIFY_CONTENT_COLUMN_REVERSE_STR_U16, CENTER_STR_U16], | ||
| // [JUSTIFY_CONTENT_ROW_STR_U16, CENTER_STR_U16], | ||
| // [JUSTIFY_CONTENT_ROW_REVERSE_STR_U16, CENTER_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
| // linear_gravity_map.insert(SPACE_BETWEEN_STR_U16, &[ | ||
| // [JUSTIFY_CONTENT_COLUMN_STR_U16, SPACE_BETWEEN_STR_U16], | ||
| // [JUSTIFY_CONTENT_COLUMN_REVERSE_STR_U16, SPACE_BETWEEN_STR_U16], | ||
| // [JUSTIFY_CONTENT_ROW_STR_U16, SPACE_BETWEEN_STR_U16], | ||
| // [JUSTIFY_CONTENT_ROW_REVERSE_STR_U16, SPACE_BETWEEN_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
|
|
||
| // Justify content rules | ||
| let mut justify_content_map = HashMap::new(); | ||
| justify_content_map.insert(START_STR_U16, &[ | ||
| [JUSTIFY_CONTENT_STR_U16, FLEX_START_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| justify_content_map.insert(END_STR_U16, &[ | ||
| [JUSTIFY_CONTENT_STR_U16, FLEX_END_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| justify_content_map.insert(LEFT_STR_U16, &[ | ||
| [INVALID_STR_U16,INVALID_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| justify_content_map.insert(RIGHT_STR_U16, &[ | ||
| [INVALID_STR_U16,INVALID_STR_U16], | ||
| ] as &[[&[u16]; 2]]); | ||
| // // Linear cross gravity rules | ||
| // let mut linear_cross_gravity_map = HashMap::new(); | ||
| // linear_cross_gravity_map.insert(START_STR_U16, &[ | ||
| // [ALIGN_ITEMS_STR_U16, START_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
| // linear_cross_gravity_map.insert(END_STR_U16, &[ | ||
| // [ALIGN_ITEMS_STR_U16, END_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
| // linear_cross_gravity_map.insert(CENTER_STR_U16, &[ | ||
| // [ALIGN_ITEMS_STR_U16, CENTER_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
| // linear_cross_gravity_map.insert(STRETCH_STR_U16, &[ | ||
| // [ALIGN_ITEMS_STR_U16, STRETCH_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
|
|
||
| map.insert(DISPLAY_STR_U16, display_map); | ||
| map.insert(DIRECTION_STR_U16, direction_map); | ||
| map.insert(LINEAR_ORIENTATION_STR_U16, linear_orientation_map); | ||
| map.insert(LINEAR_DIRECTION_STR_U16, linear_direction_map); | ||
| map.insert(LINEAR_GRAVITY_STR_U16, linear_gravity_map); | ||
| map.insert(LINEAR_CROSS_GRAVITY_STR_U16, linear_cross_gravity_map); | ||
| map.insert(LINEAR_LAYOUT_GRAVITY_STR_U16, linear_layout_gravity_map); | ||
| map.insert(JUSTIFY_CONTENT_STR_U16, justify_content_map); | ||
| // // Linear layout gravity rules | ||
| // let mut linear_layout_gravity_map = HashMap::new(); | ||
| // linear_layout_gravity_map.insert(NONE_STR_U16, &[ | ||
| // [ALIGN_SELF_ROW_STR_U16, AUTO_STR_U16], | ||
| // [ALIGN_SELF_COLUMN_STR_U16, AUTO_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
| // linear_layout_gravity_map.insert(STRETCH_STR_U16, &[ | ||
| // [ALIGN_SELF_ROW_STR_U16, STRETCH_STR_U16], | ||
| // [ALIGN_SELF_COLUMN_STR_U16, STRETCH_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
| // linear_layout_gravity_map.insert(TOP_STR_U16, &[ | ||
| // [ALIGN_SELF_ROW_STR_U16, START_STR_U16], | ||
| // [ALIGN_SELF_COLUMN_STR_U16, AUTO_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
| // linear_layout_gravity_map.insert(BOTTOM_STR_U16, &[ | ||
| // [ALIGN_SELF_ROW_STR_U16, END_STR_U16], | ||
| // [ALIGN_SELF_COLUMN_STR_U16, AUTO_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
| // linear_layout_gravity_map.insert(LEFT_STR_U16, &[ | ||
| // [ALIGN_SELF_ROW_STR_U16, AUTO_STR_U16], | ||
| // [ALIGN_SELF_COLUMN_STR_U16, START_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
| // linear_layout_gravity_map.insert(RIGHT_STR_U16, &[ | ||
| // [ALIGN_SELF_ROW_STR_U16, AUTO_STR_U16], | ||
| // [ALIGN_SELF_COLUMN_STR_U16, END_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
| // linear_layout_gravity_map.insert(START_STR_U16, &[ | ||
| // [ALIGN_SELF_ROW_STR_U16, START_STR_U16], | ||
| // [ALIGN_SELF_COLUMN_STR_U16, START_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
| // linear_layout_gravity_map.insert(END_STR_U16, &[ | ||
| // [ALIGN_SELF_ROW_STR_U16, END_STR_U16], | ||
| // [ALIGN_SELF_COLUMN_STR_U16, END_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
| // linear_layout_gravity_map.insert(CENTER_STR_U16, &[ | ||
| // [ALIGN_SELF_ROW_STR_U16, CENTER_STR_U16], | ||
| // [ALIGN_SELF_COLUMN_STR_U16, CENTER_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
| // linear_layout_gravity_map.insert(CENTER_VERTICAL_STR_U16, &[ | ||
| // [ALIGN_SELF_ROW_STR_U16, CENTER_STR_U16], | ||
| // [ALIGN_SELF_COLUMN_STR_U16, START_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
| // linear_layout_gravity_map.insert(CENTER_HORIZONTAL_STR_U16, &[ | ||
| // [ALIGN_SELF_ROW_STR_U16, START_STR_U16], | ||
| // [ALIGN_SELF_COLUMN_STR_U16, CENTER_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
| // linear_layout_gravity_map.insert(FILL_VERTICAL_STR_U16, &[ | ||
| // [ALIGN_SELF_ROW_STR_U16, STRETCH_STR_U16], | ||
| // [ALIGN_SELF_COLUMN_STR_U16, AUTO_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
| // linear_layout_gravity_map.insert(FILL_HORIZONTAL_STR_U16, &[ | ||
| // [ALIGN_SELF_ROW_STR_U16, AUTO_STR_U16], | ||
| // [ALIGN_SELF_COLUMN_STR_U16, STRETCH_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
|
|
||
| map | ||
| }; | ||
| // // Justify content rules | ||
| // let mut justify_content_map = HashMap::new(); | ||
| // justify_content_map.insert(START_STR_U16, &[ | ||
| // [JUSTIFY_CONTENT_STR_U16, FLEX_START_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
| // justify_content_map.insert(END_STR_U16, &[ | ||
| // [JUSTIFY_CONTENT_STR_U16, FLEX_END_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
| // justify_content_map.insert(LEFT_STR_U16, &[ | ||
| // [INVALID_STR_U16,INVALID_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
| // justify_content_map.insert(RIGHT_STR_U16, &[ | ||
| // [INVALID_STR_U16,INVALID_STR_U16], | ||
| // ] as &[[&[u16]; 2]]); | ||
|
|
||
| // map.insert(DISPLAY_STR_U16, display_map); | ||
| // map.insert(DIRECTION_STR_U16, direction_map); | ||
| // map.insert(LINEAR_ORIENTATION_STR_U16, linear_orientation_map); | ||
| // map.insert(LINEAR_DIRECTION_STR_U16, linear_direction_map); | ||
| // map.insert(LINEAR_GRAVITY_STR_U16, linear_gravity_map); | ||
| // map.insert(LINEAR_CROSS_GRAVITY_STR_U16, linear_cross_gravity_map); | ||
| // map.insert(LINEAR_LAYOUT_GRAVITY_STR_U16, linear_layout_gravity_map); | ||
| // map.insert(JUSTIFY_CONTENT_STR_U16, justify_content_map); | ||
|
|
||
| // map | ||
| // }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Remove extensive commented code blocks.
The large blocks of commented UTF-16 code (200+ lines) should be removed if the migration to string-based approach is complete. Keeping this much commented code makes the file difficult to maintain and review.
Consider removing the commented blocks entirely since:
- The old implementation is preserved in version control
- The commented code makes the file unnecessarily long and hard to read
- It creates confusion about which implementation is active
🤖 Prompt for AI Agents
In packages/web-platform/web-style-transformer/src/transformer/rules.rs from
lines 44 to 262, remove the large blocks of commented-out UTF-16 code related to
the old implementation. Since the migration to the string-based approach is
complete, delete these commented sections entirely to improve code readability,
reduce file length, and avoid confusion about the active implementation. The old
code remains accessible in version control if needed.
| // #[cfg(test)] | ||
| // mod tests { | ||
| // use std::str::from_utf8; | ||
|
|
||
| use crate::str_to_u16_slice; | ||
| // use crate::str_to_u16_slice; | ||
|
|
||
| #[test] | ||
| fn test_rename_rule_flex_direction() { | ||
| let source = "flex-direction:row".as_bytes(); | ||
| let source: Vec<u16> = source.iter().map(|&b| b as u16).collect(); | ||
| let name_start = 0; | ||
| let name_end = source.len() - 4; | ||
| let result: &'static [u16] = get_rename_rule_value!(&source, name_start, name_end).unwrap(); | ||
| assert_eq!(result, str_to_u16_slice!("--flex-direction")); | ||
| } | ||
| #[test] | ||
| fn test_rename_rule_flex_direction_at_mid() { | ||
| let source = "height:1px;flex-direction:row".as_bytes(); | ||
| let offset = "height:1px;".len(); | ||
| let source: Vec<u16> = source.iter().map(|&b| b as u16).collect(); | ||
| let name_start = offset; | ||
| let name_end = source.len() - 4; | ||
| let result = get_rename_rule_value!(&source, name_start, name_end).unwrap(); | ||
| assert_eq!(result, str_to_u16_slice!("--flex-direction")); | ||
| } | ||
| #[test] | ||
| fn test_replace_rule_display_linear() { | ||
| let source = "display:linear".as_bytes(); | ||
| let source: Vec<u16> = source.iter().map(|&b| b as u16).collect(); | ||
| let name_start = 0; | ||
| let name_end = 7; | ||
| let value_start = 8; | ||
| let value_end = source.len(); | ||
| let result = get_replace_rule_value!( | ||
| &source, | ||
| name_start, | ||
| name_end, | ||
| &source, | ||
| value_start, | ||
| value_end | ||
| ) | ||
| .unwrap() | ||
| .iter() | ||
| .map(|pair| { | ||
| let key = pair[0].iter().map(|&c| c as u8).collect::<Vec<u8>>(); | ||
| let value = pair[1].iter().map(|&c| c as u8).collect::<Vec<u8>>(); | ||
| format!( | ||
| "{}:{}", | ||
| from_utf8(&key).unwrap(), | ||
| from_utf8(&value).unwrap() | ||
| ) | ||
| }) | ||
| .collect::<Vec<_>>() | ||
| .join(";"); | ||
| assert_eq!( | ||
| result, | ||
| "--lynx-display-toggle:var(--lynx-display-linear);--lynx-display:linear;display:flex" | ||
| ); | ||
| } | ||
| #[test] | ||
| fn test_replace_rule_display_linear_at_mid() { | ||
| let source = "height:1px;display:linear".as_bytes(); | ||
| let source: Vec<u16> = source.iter().map(|&b| b as u16).collect(); | ||
| let offset = "height:1px;".len(); | ||
| let name_start = offset; | ||
| let name_end = offset + 7; | ||
| let value_start = offset + 8; | ||
| let value_end = source.len(); | ||
| let result = get_replace_rule_value!( | ||
| &source, | ||
| name_start, | ||
| name_end, | ||
| &source, | ||
| value_start, | ||
| value_end | ||
| ) | ||
| .unwrap() | ||
| .iter() | ||
| .map(|pair| { | ||
| let key = pair[0].iter().map(|&c| c as u8).collect::<Vec<u8>>(); | ||
| let value = pair[1].iter().map(|&c| c as u8).collect::<Vec<u8>>(); | ||
| format!( | ||
| "{}:{}", | ||
| from_utf8(&key).unwrap(), | ||
| from_utf8(&value).unwrap() | ||
| ) | ||
| }) | ||
| .collect::<Vec<_>>() | ||
| .join(";"); | ||
| assert_eq!( | ||
| result, | ||
| "--lynx-display-toggle:var(--lynx-display-linear);--lynx-display:linear;display:flex" | ||
| ); | ||
| } | ||
| // #[test] | ||
| // fn test_rename_rule_flex_direction() { | ||
| // let source = "flex-direction:row".as_bytes(); | ||
| // let source: Vec<u16> = source.iter().map(|&b| b as u16).collect(); | ||
| // let name_start = 0; | ||
| // let name_end = source.len() - 4; | ||
| // let result: &'static [u16] = get_rename_rule_value!(&source, name_start, name_end).unwrap(); | ||
| // assert_eq!(result, str_to_u16_slice!("--flex-direction")); | ||
| // } | ||
| // #[test] | ||
| // fn test_rename_rule_flex_direction_at_mid() { | ||
| // let source = "height:1px;flex-direction:row".as_bytes(); | ||
| // let offset = "height:1px;".len(); | ||
| // let source: Vec<u16> = source.iter().map(|&b| b as u16).collect(); | ||
| // let name_start = offset; | ||
| // let name_end = source.len() - 4; | ||
| // let result = get_rename_rule_value!(&source, name_start, name_end).unwrap(); | ||
| // assert_eq!(result, str_to_u16_slice!("--flex-direction")); | ||
| // } | ||
| // #[test] | ||
| // fn test_replace_rule_display_linear() { | ||
| // let source = "display:linear".as_bytes(); | ||
| // let source: Vec<u16> = source.iter().map(|&b| b as u16).collect(); | ||
| // let name_start = 0; | ||
| // let name_end = 7; | ||
| // let value_start = 8; | ||
| // let value_end = source.len(); | ||
| // let result = get_replace_rule_value!( | ||
| // &source, | ||
| // name_start, | ||
| // name_end, | ||
| // &source, | ||
| // value_start, | ||
| // value_end | ||
| // ) | ||
| // .unwrap() | ||
| // .iter() | ||
| // .map(|pair| { | ||
| // let key = pair[0].iter().map(|&c| c as u8).collect::<Vec<u8>>(); | ||
| // let value = pair[1].iter().map(|&c| c as u8).collect::<Vec<u8>>(); | ||
| // format!( | ||
| // "{}:{}", | ||
| // from_utf8(&key).unwrap(), | ||
| // from_utf8(&value).unwrap() | ||
| // ) | ||
| // }) | ||
| // .collect::<Vec<_>>() | ||
| // .join(";"); | ||
| // assert_eq!( | ||
| // result, | ||
| // "--lynx-display-toggle:var(--lynx-display-linear);--lynx-display:linear;display:flex" | ||
| // ); | ||
| // } | ||
| // #[test] | ||
| // fn test_replace_rule_display_linear_at_mid() { | ||
| // let source = "height:1px;display:linear".as_bytes(); | ||
| // let source: Vec<u16> = source.iter().map(|&b| b as u16).collect(); | ||
| // let offset = "height:1px;".len(); | ||
| // let name_start = offset; | ||
| // let name_end = offset + 7; | ||
| // let value_start = offset + 8; | ||
| // let value_end = source.len(); | ||
| // let result = get_replace_rule_value!( | ||
| // &source, | ||
| // name_start, | ||
| // name_end, | ||
| // &source, | ||
| // value_start, | ||
| // value_end | ||
| // ) | ||
| // .unwrap() | ||
| // .iter() | ||
| // .map(|pair| { | ||
| // let key = pair[0].iter().map(|&c| c as u8).collect::<Vec<u8>>(); | ||
| // let value = pair[1].iter().map(|&c| c as u8).collect::<Vec<u8>>(); | ||
| // format!( | ||
| // "{}:{}", | ||
| // from_utf8(&key).unwrap(), | ||
| // from_utf8(&value).unwrap() | ||
| // ) | ||
| // }) | ||
| // .collect::<Vec<_>>() | ||
| // .join(";"); | ||
| // assert_eq!( | ||
| // result, | ||
| // "--lynx-display-toggle:var(--lynx-display-linear);--lynx-display:linear;display:flex" | ||
| // ); | ||
| // } | ||
|
|
||
| #[test] | ||
| fn test_rename_rule_not_exist() { | ||
| let source = "background-image:url(\"https://example.com\")".as_bytes(); | ||
| let source: Vec<u16> = source.iter().map(|&b| b as u16).collect(); | ||
| let name_start = 0; | ||
| let name_end = "background-image".len(); | ||
| let result = get_rename_rule_value!(&source, name_start, name_end); | ||
| assert_eq!(result, None); | ||
| } | ||
| // #[test] | ||
| // fn test_rename_rule_not_exist() { | ||
| // let source = "background-image:url(\"https://example.com\")".as_bytes(); | ||
| // let source: Vec<u16> = source.iter().map(|&b| b as u16).collect(); | ||
| // let name_start = 0; | ||
| // let name_end = "background-image".len(); | ||
| // let result = get_rename_rule_value!(&source, name_start, name_end); | ||
| // assert_eq!(result, None); | ||
| // } | ||
|
|
||
| #[test] | ||
| fn test_replace_rule_value_not_match() { | ||
| let source = "display:grid".as_bytes(); | ||
| let source: Vec<u16> = source.iter().map(|&b| b as u16).collect(); | ||
| let name_start = 0; | ||
| let name_end = 7; | ||
| let value_start = 8; | ||
| let value_end = source.len(); | ||
| let result = get_replace_rule_value!( | ||
| &source, | ||
| name_start, | ||
| name_end, | ||
| &source, | ||
| value_start, | ||
| value_end | ||
| ); | ||
| assert_eq!(result, None); | ||
| } | ||
| // #[test] | ||
| // fn test_replace_rule_value_not_match() { | ||
| // let source = "display:grid".as_bytes(); | ||
| // let source: Vec<u16> = source.iter().map(|&b| b as u16).collect(); | ||
| // let name_start = 0; | ||
| // let name_end = 7; | ||
| // let value_start = 8; | ||
| // let value_end = source.len(); | ||
| // let result = get_replace_rule_value!( | ||
| // &source, | ||
| // name_start, | ||
| // name_end, | ||
| // &source, | ||
| // value_start, | ||
| // value_end | ||
| // ); | ||
| // assert_eq!(result, None); | ||
| // } | ||
|
|
||
| #[test] | ||
| fn test_replace_rule_name_not_match() { | ||
| let source = "height:1px".as_bytes(); | ||
| let source: Vec<u16> = source.iter().map(|&b| b as u16).collect(); | ||
| let name_start = 0; | ||
| let name_end = 6; | ||
| let value_start = 7; | ||
| let value_end = source.len(); | ||
| let result = get_replace_rule_value!( | ||
| &source, | ||
| name_start, | ||
| name_end, | ||
| &source, | ||
| value_start, | ||
| value_end | ||
| ); | ||
| assert_eq!(result, None); | ||
| } | ||
| } | ||
| // #[test] | ||
| // fn test_replace_rule_name_not_match() { | ||
| // let source = "height:1px".as_bytes(); | ||
| // let source: Vec<u16> = source.iter().map(|&b| b as u16).collect(); | ||
| // let name_start = 0; | ||
| // let name_end = 6; | ||
| // let value_start = 7; | ||
| // let value_end = source.len(); | ||
| // let result = get_replace_rule_value!( | ||
| // &source, | ||
| // name_start, | ||
| // name_end, | ||
| // &source, | ||
| // value_start, | ||
| // value_end | ||
| // ); | ||
| // assert_eq!(result, None); | ||
| // } | ||
| // } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Restore test coverage for the refactored functionality.
All tests have been commented out, which significantly reduces test coverage for the rule transformation logic. The tests should be updated to work with the new string-based API rather than being disabled.
The tests need to be adapted to use js_sys::JsString inputs and string-based assertions. Would you like me to help convert these tests to work with the new API?
🤖 Prompt for AI Agents
In packages/web-platform/web-style-transformer/src/transformer/rules.rs from
lines 293 to 435, the test functions are currently commented out and use a u16
slice-based API. The issue is that these tests need to be restored and updated
to work with the new string-based API using js_sys::JsString inputs and
string-based assertions. To fix this, uncomment the tests and refactor them to
create inputs as JsString instances, adjust the calls to the transformation
functions accordingly, and update the assertions to compare string outputs
instead of u16 slices.
❌ 7 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🔭 Outside diff range comments (3)
packages/web-platform/web-style-transformer/standard.d.ts (1)
6-20: Update outdated function documentationThe comments still reference "raw uint16 pointer" and pointer safety concerns, but the function signatures now accept string parameters. These comments should be updated to reflect the new string-based interface.
-/** - * lifted from the `console_log` example - * Accepts a raw uint16 pointer from JS and transforms the inline style string into a JS string. - * Returns `Some(JsString)` if the transformation was successful, or `None` if the input was empty or invalid. - * - * # Safety - * The caller must ensure that `ptr` is valid and points to a slice of `u16` of length `len`. - * This is a contract with the JavaScript side. Passing an invalid pointer or incorrect length may cause undefined behavior. - */ +/** + * Transforms an inline style string into a processed style string. + * Returns a transformed string if successful, or undefined if the input was empty or invalid. + */ export function transform_inline_style(ptr: string): string | undefined; /** - * Accepts raw uint16 pointers from JS and parses the inline style name and value into a JS array. - * Returns `Some(Array)` if parsing was successful, or `None` if both results are empty. - * - * # Safety - * The caller must ensure that `name_ptr` and `value_ptr` are valid and point to slices of `u16` of lengths `name_len` and `value_len` respectively. - * Passing invalid pointers or incorrect lengths may cause undefined behavior. + * Parses inline style name and value strings into a JS array. + * Returns an array if parsing was successful, or undefined if both results are empty. */packages/web-platform/inline-style-parser/src/tokenize.rs (1)
150-444: Complete the iterator-based tokenization implementationThe tokenize function has an incomplete migration - the first part (lines 156-236) uses the new iterator-based approach with
chars.peekable(), but the second part (lines 238-444) still uses the oldchar_code_atapproach. This inconsistency will cause the function to fail since the iterator won't be properly consumed throughout the entire source.The entire tokenize function needs to use a consistent approach. Either:
- Complete the iterator-based implementation for the entire function
- Revert to using indices throughout
The current mixed approach will not work correctly as the iterator position and offset tracking are not synchronized.
packages/web-platform/web-style-transformer/src/transformer/transform.rs (1)
36-47: Fix string iteration in is_digit_only macroThe macro attempts to call
.iter()on a string slice, which doesn't have this method. Use.chars()instead to iterate over characters.macro_rules! is_digit_only { ($source:expr, $start:expr, $end:expr) => {{ let mut result = true; - for code in $source.slice($start, $end).iter() { - if code > b'9' as u16 || code < b'0' as u16 { + for ch in $source.slice($start, $end).chars() { + let code = ch as u32; + if code > '9' as u32 || code < '0' as u32 { result = false; break; } } result }}; }
♻️ Duplicate comments (4)
packages/web-platform/web-style-transformer/src/lib.rs (1)
53-74: Remove unnecessary unsafe blockThe unsafe block is no longer needed since you're not performing any unsafe operations.
- unsafe { - // let name_slice = core::slice::from_raw_parts(name_ptr, name_len); - // let value_slice = core::slice::from_raw_parts(value_ptr, value_len); - // Call the tokenize function with our data and callback - let (result, children_result) = transformer::transform::query_transform_rules( - &name_ptr, 0 as u32, name_len, &value_ptr, 0 as u32, value_len, - ); - if result.is_empty() && children_result.is_empty() { - // if there are no results, we return None - return None; - } - // now we need to convert the result into a JS array - let ret = js_sys::Array::new(); - ret.push(&push_parsed_result_to_js_array!(result).into()); - if !children_result.is_empty() { - // if there are no children, we don't need to push an empty array - // but if there are children, we need to push them as well - ret.push(&push_parsed_result_to_js_array!(children_result).into()); - } - Some(ret) + // Call the tokenize function with our data and callback + let (result, children_result) = transformer::transform::query_transform_rules( + &name_ptr, 0, name_len, &value_ptr, 0, value_len, + ); + if result.is_empty() && children_result.is_empty() { + // if there are no results, we return None + return None; + } + // now we need to convert the result into a JS array + let ret = js_sys::Array::new(); + ret.push(&push_parsed_result_to_js_array!(result).into()); + if !children_result.is_empty() { + // if there are no children, we don't need to push an empty array + // but if there are children, we need to push them as well + ret.push(&push_parsed_result_to_js_array!(children_result).into()); } + Some(ret)packages/web-platform/web-style-transformer/src/transformer/transform.rs (2)
347-354: Fix incorrect string slicing in Transformer implementationThe code attempts to slice String objects using array indexing, which doesn't work in Rust. Based on the rest of the code, it appears
decl_nameanddecl_valueare already the full strings to be used.// Append the declaration name and colon - self.transformed_source.push(String::from( - &decl_name[*decl_name_start as usize..*decl_name_end as usize], - )); - self.transformed_source.push(String::from(":")); + self.transformed_source.push(decl_name); + self.transformed_source.push(":"); // Append the declaration value - self.transformed_source.push(String::from( - &decl_value[*decl_value_start as usize..*decl_value_end as usize], - )); + self.transformed_source.push(decl_value);The same fix should be applied to lines 370-377.
403-407: Optimize string concatenationUsing
flat_mapover characters is inefficient for string concatenation. Usejoin("")instead.- transformer - .transformed_source - .iter() - .flat_map(|s| s.chars()) - .collect() + transformer.transformed_source.join("")packages/web-platform/inline-style-parser/src/char_code_definitions.rs (1)
251-251: Fix incorrect case-insensitive comparison logicThe logic for case-insensitive comparison is incorrect. The condition
(code == $reference_code)will never be true if the first condition failed.- if code == $reference_code || (is_uppercase_letter!(code) && (code == $reference_code)) { + if code == $reference_code || (is_uppercase_letter!(code) && ((code | 32) == $reference_code)) {
🧹 Nitpick comments (3)
packages/web-platform/web-style-transformer/src/transformer/transform.rs (1)
3-3: Remove unused importThe
web_sys::consoleimport is not used in this file.-use web_sys::console;packages/web-platform/inline-style-parser/src/char_code_definitions.rs (2)
182-204: Remove commented-out codeLarge blocks of commented code should be removed to keep the codebase clean.
Remove the entire commented
is_identifier_startmacro implementation (lines 182-204) as it's been replaced by the new iterator-based version.
273-282: Remove commented-out codeRemove the commented
get_new_line_lengthmacro to keep the codebase clean.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/web-platform/web-tests/tests/react.spec.ts-snapshots/scroll-view/basic-element-scroll-view-fixed/initial-chromium-darwin.pngis excluded by!**/*.png
📒 Files selected for processing (7)
packages/web-platform/inline-style-parser/src/char_code_definitions.rs(7 hunks)packages/web-platform/inline-style-parser/src/parse_inline_style.rs(3 hunks)packages/web-platform/inline-style-parser/src/tokenize.rs(5 hunks)packages/web-platform/inline-style-parser/src/utils.rs(4 hunks)packages/web-platform/web-style-transformer/src/lib.rs(3 hunks)packages/web-platform/web-style-transformer/src/transformer/transform.rs(20 hunks)packages/web-platform/web-style-transformer/standard.d.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/web-platform/inline-style-parser/src/parse_inline_style.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/web-platform/inline-style-parser/src/utils.rs
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.
Learnt from: PupilTong
PR: lynx-family/lynx-stack#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.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:26:16.722Z
Learning: In the lynx-family/lynx-stack repository, CI checks require changesets when files matching the pattern "src/**" are modified (as configured in .changeset/config.json). For internal changes that don't need meaningful changesets, an empty changeset file is used to satisfy the CI requirement while not generating any release notes.
📚 Learning: in the lynx-stack codebase, css strings produced by `gencsscontent` are considered trusted developer...
Learnt from: PupilTong
PR: lynx-family/lynx-stack#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-style-transformer/src/lib.rspackages/web-platform/web-style-transformer/src/transformer/transform.rs
🧬 Code Graph Analysis (1)
packages/web-platform/web-style-transformer/standard.d.ts (1)
packages/web-platform/web-style-transformer/src/lib.rs (1)
transform_inline_style(14-20)
| if next_c.is_some() && is_digit!(next_c as u32) { | ||
| true | ||
| } else { | ||
| ($second == 0x002E) && is_digit!($third) // U+002E FULL STOP (.) | ||
| (next_c.is_some() && next_c as u32 == 0x002E) | ||
| && next_next_c.is_some() | ||
| && is_digit!(next_next_c as u32) // U+002E FULL STOP (.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix type conversion issues in is_number_start macro
The macro has incorrect type conversions - next_c as u32 attempts to cast an Option to u32 which won't compile.
- if next_c.is_some() && is_digit!(next_c as u32) {
+ if let Some(c) = next_c {
+ if is_digit!(c as u32) {
true
+ } else {
+ false
+ }
} else {
- (next_c.is_some() && next_c as u32 == 0x002E)
- && next_next_c.is_some()
- && is_digit!(next_next_c as u32) // U+002E FULL STOP (.)
+ if let (Some(c1), Some(c2)) = (next_c, next_next_c) {
+ (c1 as u32 == 0x002E) && is_digit!(c2 as u32)
+ } else {
+ false
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if next_c.is_some() && is_digit!(next_c as u32) { | |
| true | |
| } else { | |
| ($second == 0x002E) && is_digit!($third) // U+002E FULL STOP (.) | |
| (next_c.is_some() && next_c as u32 == 0x002E) | |
| && next_next_c.is_some() | |
| && is_digit!(next_next_c as u32) // U+002E FULL STOP (.) | |
| if let Some(c) = next_c { | |
| if is_digit!(c as u32) { | |
| true | |
| } else { | |
| false | |
| } | |
| } else { | |
| if let (Some(c1), Some(c2)) = (next_c, next_next_c) { | |
| (c1 as u32 == 0x002E) && is_digit!(c2 as u32) | |
| } else { | |
| false | |
| } | |
| } |
🤖 Prompt for AI Agents
In packages/web-platform/inline-style-parser/src/char_code_definitions.rs around
lines 215 to 220, the macro incorrectly casts an Option<char> directly to u32,
which is invalid. To fix this, first unwrap or pattern match the Option<char> to
get the char value, then convert that char to u32 before passing it to is_digit!
or comparing it to 0x002E. Adjust the code to safely handle the Option and
perform the conversion on the inner char only.
| if source.char_code_at(ii) == ('*' as u16 as f64) | ||
| && source.char_code_at(ii + 1) == ('/' as u16 as f64) | ||
| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix inconsistent character code comparison
The code uses u16 casting and f64 comparison which is inconsistent with the rest of the codebase that uses u32.
- if source.char_code_at(ii) == ('*' as u16 as f64)
- && source.char_code_at(ii + 1) == ('/' as u16 as f64)
+ if source.char_code_at(ii) as u32 == ('*' as u32)
+ && source.char_code_at(ii + 1) as u32 == ('/' as u32)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if source.char_code_at(ii) == ('*' as u16 as f64) | |
| && source.char_code_at(ii + 1) == ('/' as u16 as f64) | |
| { | |
| if source.char_code_at(ii) as u32 == ('*' as u32) | |
| && source.char_code_at(ii + 1) as u32 == ('/' as u32) | |
| { |
🤖 Prompt for AI Agents
In packages/web-platform/inline-style-parser/src/tokenize.rs around lines 309 to
311, the character code comparison uses inconsistent casting to u16 and f64,
while the rest of the codebase uses u32. Update the casts to use u32
consistently for both the character literals and the char_code_at method
comparisons to maintain uniformity and correctness.
| unsafe { | ||
| let slice = core::slice::from_raw_parts(ptr, len); | ||
| // Call the tokenize function with our data and callback | ||
| let (transformed_inline_style, _) = | ||
| transformer::transform::transform_inline_style_string(slice); | ||
| if !transformed_inline_style.is_empty() { | ||
| return Some(js_sys::JsString::from_char_code( | ||
| transformed_inline_style.as_slice(), | ||
| )); | ||
| } | ||
| let transformed_inline_style = transformer::transform::transform_inline_style_string(&source); | ||
| return Some(js_sys::JsString::from(transformed_inline_style)); | ||
| } | ||
| None | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unnecessary unsafe block and unreachable code
The function no longer performs any unsafe operations, so the unsafe block should be removed. Additionally, line 19 contains unreachable code after the return statement.
pub fn transform_inline_style(source: String) -> Option<JsString> {
- unsafe {
- let transformed_inline_style = transformer::transform::transform_inline_style_string(&source);
- return Some(js_sys::JsString::from(transformed_inline_style));
- }
- None
+ let transformed_inline_style = transformer::transform::transform_inline_style_string(&source);
+ Some(js_sys::JsString::from(transformed_inline_style))
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| unsafe { | |
| let slice = core::slice::from_raw_parts(ptr, len); | |
| // Call the tokenize function with our data and callback | |
| let (transformed_inline_style, _) = | |
| transformer::transform::transform_inline_style_string(slice); | |
| if !transformed_inline_style.is_empty() { | |
| return Some(js_sys::JsString::from_char_code( | |
| transformed_inline_style.as_slice(), | |
| )); | |
| } | |
| let transformed_inline_style = transformer::transform::transform_inline_style_string(&source); | |
| return Some(js_sys::JsString::from(transformed_inline_style)); | |
| } | |
| None | |
| } | |
| pub fn transform_inline_style(source: String) -> Option<JsString> { | |
| let transformed_inline_style = transformer::transform::transform_inline_style_string(&source); | |
| Some(js_sys::JsString::from(transformed_inline_style)) | |
| } |
🤖 Prompt for AI Agents
In packages/web-platform/web-style-transformer/src/lib.rs around lines 15 to 20,
remove the unnecessary unsafe block since no unsafe operations are performed.
Also, delete the unreachable code after the return statement on line 19 to clean
up the function.
| let k = js_sys::JsString::from(&name_source[name_start as usize..name_end as usize]); | ||
| let v = js_sys::JsString::from(&value_source[value_start as usize..value_end as usize]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Optimize string slicing to avoid unnecessary conversions
Converting JsString to String just for slicing is inefficient. Use JsString's slice method directly.
- let k = js_sys::JsString::from(&name_source[name_start as usize..name_end as usize]);
- let v = js_sys::JsString::from(&value_source[value_start as usize..value_end as usize]);
+ let k = name_source.slice(name_start, name_end);
+ let v = value_source.slice(value_start, value_end);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let k = js_sys::JsString::from(&name_source[name_start as usize..name_end as usize]); | |
| let v = js_sys::JsString::from(&value_source[value_start as usize..value_end as usize]); | |
| let k = name_source.slice(name_start, name_end); | |
| let v = value_source.slice(value_start, value_end); |
🤖 Prompt for AI Agents
In packages/web-platform/web-style-transformer/src/lib.rs at lines 27 to 28, the
code converts JsString to Rust String for slicing, which is inefficient.
Instead, use the slice method provided by JsString directly on name_source and
value_source to obtain the substrings without intermediate conversions. Replace
the current slicing approach with JsString's slice method calls using the
appropriate indices.
Summary by CodeRabbit
New Features
Refactor
JsString/string) instead of raw UTF-16 slices and pointers.Chores
Checklist