fix: crash on chrome<96 #1361
fix: crash on chrome<96 #1361
Conversation
🦋 Changeset detectedLatest commit: 17b9073 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 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 |
📝 WalkthroughWalkthroughThis change introduces conditional loading of WebAssembly modules in the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ 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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
packages/web-platform/web-core-server/package.json (2)
14-16: Redundant"module"entry – preferexportsmap or drop itWith
"type": "module"already set,dist/index.jsis unambiguously ESM.
Keeping both"main": "dist/index.js", "module": "dist/index.js"doesn’t add value and occasionally confuses bundlers that treat
"module"as “browser-optimised ESM”. Either:
- Remove the redundant
"module"key, or- Publish dual builds (CJS + ESM) and point
"main"to*.cjs,"module"to*.js, plus an explicit"exports"map for forward-compatibility.
25-28:build/watchscripts look good but publishing may miss themNice to see dedicated build commands wired to
rslib. Two quick checks:
rslibis not declared in this package’s deps; ensure it’s provided by the root workspace so these scripts resolve in all environments (CI, local,npm pack).- If this package is published to npm, add at least one of
"prepare": "npm run build"or
"prepublishOnly": "npm run build"so consumers always receive the compiled
distartefacts even when the tarball is built outside the monorepo pipeline..changeset/tall-experts-take.md (2)
8-8: Format the GitHub issue reference as a proper Markdown link.The bare URL should be formatted as a proper Markdown link for better readability and standards compliance.
-https://github.com/wasm-bindgen/wasm-bindgen/issues/4211#issuecomment-2505965903 +[wasm-bindgen issue #4211](https://github.com/wasm-bindgen/wasm-bindgen/issues/4211#issuecomment-2505965903)
10-10: Format the GitHub issue reference as a proper Markdown link.The bare URL should be formatted as a proper Markdown link for consistency and better readability.
-https://github.com/WebAssembly/binaryen/issues/7358 +[WebAssembly binaryen issue #7358](https://github.com/WebAssembly/binaryen/issues/7358)packages/web-platform/web-style-transformer/index.js (1)
2-2: Consider making the wasm variable read-only after initialization.The exported
letvariable allows external reassignment which could lead to unexpected behavior. Consider using a getter function or making it immutable after initialization.-export let wasm; +let _wasm; +export const getWasm = () => _wasm;Then update the assignment in
initWasm:- wasm = await import( + _wasm = await import(packages/web-platform/web-style-transformer/standard.d.ts (1)
1-40: Consider using more specific types for the parsed result array.The type declarations are well-documented with appropriate safety warnings. However,
Array<any>on line 30 could be more specific to improve type safety.Based on the Rust implementation that pushes style tuples, consider defining a more specific type:
+export type StyleTuple = [string, string]; +export type ParsedStyleResult = [StyleTuple[], StyleTuple[]?]; + export function transform_raw_u16_inline_style_ptr_parsed( name_ptr: number, name_len: number, value_ptr: number, value_len: number, -): Array<any> | undefined; +): ParsedStyleResult | undefined;This would provide better type safety for consumers of this API.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
.changeset/mighty-readers-clean.md(1 hunks).changeset/tall-experts-take.md(1 hunks)packages/web-platform/web-core-server/package.json(2 hunks)packages/web-platform/web-core-server/rslib.config.ts(1 hunks)packages/web-platform/web-core-server/src/utils/loadTemplate.ts(1 hunks)packages/web-platform/web-core-server/tsconfig.json(1 hunks)packages/web-platform/web-core-server/turbo.json(1 hunks)packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts(2 hunks)packages/web-platform/web-mainthread-apis/src/utils/tokenizer.ts(1 hunks)packages/web-platform/web-style-transformer/index.d.ts(1 hunks)packages/web-platform/web-style-transformer/index.js(1 hunks)packages/web-platform/web-style-transformer/legacy.js(1 hunks)packages/web-platform/web-style-transformer/package.json(1 hunks)packages/web-platform/web-style-transformer/standard.d.ts(1 hunks)packages/web-platform/web-style-transformer/standard.js(1 hunks)packages/web-platform/web-tests/rspack.config.js(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
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.
packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts (1)
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.
.changeset/mighty-readers-clean.md (2)
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.
packages/web-platform/web-style-transformer/package.json (3)
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: colinaaa
PR: #1238
File: packages/react/runtime/src/debug/component-stack.ts:70-90
Timestamp: 2025-07-18T04:27:18.291Z
Learning: The component-stack.ts file in packages/react/runtime/src/debug/component-stack.ts is a direct fork from https://github.com/preactjs/preact/blob/main/debug/src/component-stack.js. The team prefers to keep it aligned with the upstream Preact version and may contribute improvements back to Preact in the future.
.changeset/tall-experts-take.md (3)
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: #1292
File: packages/web-platform/web-core-server/src/createLynxView.ts:144-151
Timestamp: 2025-07-15T10:00:56.154Z
Learning: In the lynx-stack codebase, PupilTong prefers the "let it crash" approach over defensive null safety checks when the condition should never occur in normal operation. This applies to cases like the element.getAttribute(lynxUniqueIdAttribute)! call in SSR event capture where the attribute is expected to always be present.
packages/web-platform/web-core-server/package.json (1)
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.
🧬 Code Graph Analysis (3)
packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts (1)
packages/web-platform/web-style-transformer/index.js (1)
initWasm(3-23)
packages/web-platform/web-core-server/rslib.config.ts (2)
packages/web-platform/web-tests/rspack.config.js (1)
config(19-272)packages/rspeedy/core/src/index.ts (1)
defineConfig(33-33)
packages/web-platform/web-style-transformer/standard.d.ts (1)
packages/web-platform/web-style-transformer/src/lib.rs (2)
transform_raw_u16_inline_style_ptr(13-31)transform_raw_u16_inline_style_ptr_parsed(55-87)
🪛 markdownlint-cli2 (0.17.2)
.changeset/tall-experts-take.md
8-8: Bare URL used
(MD034, no-bare-urls)
10-10: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (20)
packages/web-platform/web-tests/rspack.config.js (1)
270-270: Approved:futureDefaultsenables required WebAssembly supportThe
futureDefaults: trueflag not only activates native CSS but also enablesasyncWebAssembly—the WebAssembly experiment needed to support asynchronous modules and reference types per the updated spec (critical for Chrome <96). No further changes are needed.
- File:
packages/web-platform/web-tests/rspack.config.js- Line: 270
packages/web-platform/web-core-server/src/utils/loadTemplate.ts (1)
8-15: Potential race condition in lazy initialization.The lazy initialization pattern is good for performance, but there's a race condition if multiple concurrent calls to
createJsModuleUrloccur beforetmpDiris set. The second call could start executing before the first call finishes awaitingtmpDirPromise.Consider this safer approach:
-let tmpDir: string | undefined; -const tmpDirPromise = fs.mkdtemp(path.join(os.tmpdir(), 'lynx')); +let tmpDirPromise: Promise<string> | undefined; async function createJsModuleUrl(content: string, filename: string) { - if (!tmpDir) { - tmpDir = await tmpDirPromise; - } - const fileUrl = path.join(tmpDir, filename); + if (!tmpDirPromise) { + tmpDirPromise = fs.mkdtemp(path.join(os.tmpdir(), 'lynx')); + } + const tmpDir = await tmpDirPromise; + const fileUrl = path.join(tmpDir, filename);Likely an incorrect or invalid review comment.
.changeset/mighty-readers-clean.md (1)
1-6: LGTM! Changeset properly documents the refactoring work.The changeset appropriately documents the refactoring and bundling improvements to the web-core-server package. This satisfies the CI requirement for changes to src/** files while clearly indicating the internal nature of the changes.
packages/web-platform/web-core-server/tsconfig.json (1)
9-10: LGTM! TypeScript configuration aligns with new build system.The addition of
noEmit: trueandisolatedModules: trueis appropriate for the new rslib-based build system. These options ensure TypeScript focuses on type checking while rslib handles compilation, andisolatedModulesensures better compatibility with bundlers.packages/web-platform/web-core-server/rslib.config.ts (1)
4-18: LGTM! Configuration properly supports WASM compatibility objectives.The rslib configuration is well-structured and directly supports the PR's objective of fixing WASM compatibility:
- ESM format with TypeScript declarations enables modern module consumption
wasmLoading: 'async-node'aligns with the conditional WASM loading strategy to handle browser compatibility- Source maps support debugging
packages/web-platform/web-core-server/turbo.json (1)
1-18: LGTM! Turbo configuration follows best practices.The build task configuration properly integrates the web-core-server package into the monorepo's build orchestration:
- Correct dependency chain with root and dependency builds
- Appropriate input/output specifications for caching
- Standard Turbo configuration structure
packages/web-platform/web-style-transformer/standard.js (1)
1-2: LGTM! Clean proxy module pattern.The re-export syntax and explicit memory export from the WASM binary follow best practices for WebAssembly module interfaces. This proxy pattern provides a clean abstraction layer for the conditional loading mechanism.
packages/web-platform/web-style-transformer/legacy.js (1)
1-2: LGTM! Consistent proxy module pattern.The legacy module follows the same clean pattern as the standard module, maintaining consistency in the conditional loading architecture. The re-export and memory export structure is correct.
packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts (2)
34-35: LGTM! Correct integration with new WASM initialization.The import and initialization logic have been properly updated to use the centralized
initWasmfunction from@lynx-js/web-style-transformer. The naming change frominitTokenizertoinitWasmbetter reflects the broader WASM initialization scope.
88-88: LGTM! Proper async initialization sequencing.The await call correctly ensures WASM initialization completes before proceeding with the main thread setup, maintaining the proper initialization order.
.changeset/tall-experts-take.md (1)
6-20: LGTM! Clear and accurate problem description.The changeset provides a clear explanation of the Chrome <96 compatibility issue with WebAssembly reference types and the implemented workaround. The technical details accurately describe the conditional loading solution.
packages/web-platform/web-style-transformer/index.js (2)
1-1: LGTM! Appropriate feature detection dependency.The
wasm-feature-detectpackage is the right choice for detecting WebAssembly reference types support, providing a reliable way to determine browser capabilities.
4-22: LGTM! Excellent conditional loading strategy.The webpack magic comments are well-chosen - eager loading with high priority for modern browsers that support reference types, and lazy loading for older browsers. This optimization strategy aligns perfectly with the performance characteristics needed for each browser tier.
packages/web-platform/web-mainthread-apis/src/utils/tokenizer.ts (3)
15-26: LGTM! Proper error handling and memory management.The function correctly handles WASM memory allocation/deallocation with proper cleanup in both success and error cases.
28-67: LGTM! Robust implementation with proper fallback handling.The function correctly manages memory for both property and value allocations, with appropriate error handling and fallback to original values when transformation fails.
1-1: WASM Initialization Order Verified – No Changes RequiredWe’ve confirmed that:
initWasm()is invoked immediately whenprepareMainThreadAPIs.tsis imported:import { initWasm } from '@lynx-js/web-style-transformer'; const initWasmPromise = initWasm();- Inside
startMainThread, weawait initWasmPromiseat the very beginning (decode_start) before any user‐provided entry code or DOM‐style transformations run.- All calls to
transformInlineStyleString/transformParsedStylesoccur afterawait initWasmPromise, ensuringwasmis initialized.The direct import of
wasmintokenizer.tsis safe under this flow—no runtime errors are introduced by accessingwasmin these utility functions.packages/web-platform/web-style-transformer/package.json (3)
13-13: LGTM! Package structure updated for conditional WASM loading.The main entry point and file patterns correctly reflect the new structure where root-level files handle conditional loading of WASM modules.
Also applies to: 18-19
32-34: LGTM! Required dependency for browser feature detection.The
wasm-feature-detectpackage is essential for the conditional loading mechanism that prevents crashes on Chrome < 96.
26-28: Dual WASM build & imports verifiedIndex.js correctly feature-detects and conditionally imports
./standard.jsand./legacy.js. Both module wrappers export the built artifacts fromdist/standard.jsanddist/legacy.jsas expected, and the legacy build’sRUSTFLAGS=-Cstrip=symbolsoptimization is in place. No further changes required.packages/web-platform/web-style-transformer/index.d.ts (1)
1-3: Conditional WASM loader interface verified for both modulesBoth
legacy.jsandstandard.jsre-export the same API surface (wildcard exports from their respectivedist/*.jsfiles plus thememoryexport), so derivingWASMModulefromimport('./standard.js')safely covers both implementations. No changes needed.
PupilTong
force-pushed
the
p/hw/wasm-feature-detect
branch
from
53ffd37 to
597a4ec
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.changeset/tall-experts-take.md (2)
8-11: Replace bare URLs with Markdown links to satisfymarkdownlintMD034
markdownlintflagged the two bare URLs. Wrapping them in Markdown links keeps CI happy and reads better.-https://github.com/wasm-bindgen/wasm-bindgen/issues/4211#issuecomment-2505965903 +[wasm-bindgen issue #4211 (comment)](https://github.com/wasm-bindgen/wasm-bindgen/issues/4211#issuecomment-2505965903) -https://github.com/WebAssembly/binaryen/issues/7358 +[binaryen issue #7358](https://github.com/WebAssembly/binaryen/issues/7358)
14-20: Tighten wording & casing for clarityA few small edits improve readability and consistency:
-However this feature is not supported by chromium lower than version 96 +However, this feature is not supported by Chromium versions earlier than 96. -Therefore we found a workaround for it. +Therefore, we implemented a workaround. -In this implementation we detect if browser supports `reference types` first. +At runtime, we first detect whether the browser supports `reference types`. -If user's browser supported it, we load the wasm file with `reference types` on, otherwise we load the wasm file with `reference types` off. +If the browser supports the feature, we load the WASM module with `reference types` enabled; otherwise, we load a legacy module without it.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
.changeset/mighty-readers-clean.md(1 hunks).changeset/tall-experts-take.md(1 hunks)packages/web-platform/web-core-server/package.json(2 hunks)packages/web-platform/web-core-server/rslib.config.ts(1 hunks)packages/web-platform/web-core-server/src/utils/loadTemplate.ts(1 hunks)packages/web-platform/web-core-server/tsconfig.json(1 hunks)packages/web-platform/web-core-server/turbo.json(1 hunks)packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts(2 hunks)packages/web-platform/web-mainthread-apis/src/utils/tokenizer.ts(1 hunks)packages/web-platform/web-style-transformer/index.d.ts(1 hunks)packages/web-platform/web-style-transformer/index.js(1 hunks)packages/web-platform/web-style-transformer/legacy.js(1 hunks)packages/web-platform/web-style-transformer/package.json(1 hunks)packages/web-platform/web-style-transformer/standard.d.ts(1 hunks)packages/web-platform/web-style-transformer/standard.js(1 hunks)packages/web-platform/web-tests/rspack.config.js(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- packages/web-platform/web-core-server/rslib.config.ts
- packages/web-platform/web-core-server/tsconfig.json
- packages/web-platform/web-style-transformer/legacy.js
- packages/web-platform/web-style-transformer/standard.d.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- .changeset/mighty-readers-clean.md
- packages/web-platform/web-style-transformer/standard.js
- packages/web-platform/web-tests/rspack.config.js
- packages/web-platform/web-core-server/package.json
- packages/web-platform/web-core-server/src/utils/loadTemplate.ts
- packages/web-platform/web-core-server/turbo.json
- packages/web-platform/web-mainthread-apis/src/utils/tokenizer.ts
- packages/web-platform/web-style-transformer/package.json
- packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts
- packages/web-platform/web-style-transformer/index.js
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1238
File: packages/react/runtime/src/debug/component-stack.ts:70-90
Timestamp: 2025-07-18T04:27:18.291Z
Learning: The component-stack.ts file in packages/react/runtime/src/debug/component-stack.ts is a direct fork from https://github.com/preactjs/preact/blob/main/debug/src/component-stack.js. The team prefers to keep it aligned with the upstream Preact version and may contribute improvements back to Preact in the future.
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.
.changeset/tall-experts-take.md (3)
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: #1292
File: packages/web-platform/web-core-server/src/createLynxView.ts:144-151
Timestamp: 2025-07-15T10:00:56.154Z
Learning: In the lynx-stack codebase, PupilTong prefers the "let it crash" approach over defensive null safety checks when the condition should never occur in normal operation. This applies to cases like the element.getAttribute(lynxUniqueIdAttribute)! call in SSR event capture where the attribute is expected to always be present.
🧬 Code Graph Analysis (1)
packages/web-platform/web-style-transformer/index.d.ts (1)
packages/web-platform/web-style-transformer/index.js (3)
wasm(2-2)wasm(2-2)initWasm(3-23)
🪛 markdownlint-cli2 (0.17.2)
.changeset/tall-experts-take.md
8-8: Bare URL used
(MD034, no-bare-urls)
10-10: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (3)
packages/web-platform/web-style-transformer/index.d.ts (3)
2-2: LGTM!The variable declaration correctly uses
letto allow conditional assignment and is properly typed with theWASMModuleinterface.
3-3: LGTM!The function declaration correctly represents the async initialization function with appropriate return type
Promise<void>.
1-1: Ensure legacy.js exports match the standard module’s APIThe
WASMModuletype is derived fromstandard.js, but at runtime you may loadlegacy.jsinstead. Please confirm that the legacy build re-exports the same public API surface:• memory
• transform_raw_u16_inline_style_ptr
• transform_raw_u16_inline_style_ptr_parsed
• malloc
• freeIf any of these are missing or differ in signature, consider adjusting the declarations or introducing a shared interface.
wasm-bindgen/wasm-bindgen#4211 (comment) WebAssembly/binaryen#7358 The rust toolchain enables WASM feature `reference types` by default. However this feature is not supported by chromium lower than version 96 Therefore we found a workaround for it. In this implementation we detect if browser supports `reference types` first. If user's browser supported it, we load the wasm file with `reference types` on, otherwise we load the wasm file with `reference types` off.
PupilTong
force-pushed
the
p/hw/wasm-feature-detect
branch
from
597a4ec to
9ffef20
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.changeset/tall-experts-take.md (1)
6-10: Polish summary line & wrap bare URLs
- Use a descriptive sentence instead of a “commit style” prefix for the first line—this becomes the changelog entry:
-fix: crash on chrome<96 +Prevent crash on Chrome < 96 by loading a legacy WASM build when reference-types are unsupported
- Wrap raw links in angle brackets to satisfy
markdownlintand keep release notes tidy:-https://github.com/wasm-bindgen/wasm-bindgen/issues/4211#issuecomment-2505965903 - -https://github.com/WebAssembly/binaryen/issues/7358 +<https://github.com/wasm-bindgen/wasm-bindgen/issues/4211#issuecomment-2505965903> + +<https://github.com/WebAssembly/binaryen/issues/7358>
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
.changeset/tall-experts-take.md(1 hunks)packages/web-platform/web-style-transformer/index.js(1 hunks)packages/web-platform/web-style-transformer/legacy.js(1 hunks)packages/web-platform/web-style-transformer/package.json(1 hunks)packages/web-platform/web-style-transformer/standard.js(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/web-platform/web-style-transformer/legacy.js
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/web-platform/web-style-transformer/index.js
- packages/web-platform/web-style-transformer/standard.js
- packages/web-platform/web-style-transformer/package.json
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
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.
.changeset/tall-experts-take.md (3)
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: #1292
File: packages/web-platform/web-core-server/src/createLynxView.ts:144-151
Timestamp: 2025-07-15T10:00:56.154Z
Learning: In the lynx-stack codebase, PupilTong prefers the "let it crash" approach over defensive null safety checks when the condition should never occur in normal operation. This applies to cases like the element.getAttribute(lynxUniqueIdAttribute)! call in SSR event capture where the attribute is expected to always be present.
🪛 markdownlint-cli2 (0.17.2)
.changeset/tall-experts-take.md
8-8: Bare URL used
(MD034, no-bare-urls)
10-10: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (1)
.changeset/tall-experts-take.md (1)
1-4: Verify affected package list in front-matter
web-style-transformerappears to be the module that now ships the dual WASM bundles, yet it’s missing from the changeset header. Please double-check that every user-facing package touched by this PR is listed here; otherwise the release version bump will be skipped for that package.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/web-platform/web-style-transformer/package.json(1 hunks)packages/web-platform/web-style-transformer/scripts/build.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/web-platform/web-style-transformer/package.json
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
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.
🔇 Additional comments (1)
packages/web-platform/web-style-transformer/scripts/build.js (1)
1-7: LGTM - Clean ES module setupThe imports and directory setup are properly configured for an ES module build script.
PupilTong
force-pushed
the
p/hw/wasm-feature-detect
branch
from
6be9cc7 to
17b9073
Compare
CodSpeed Performance ReportMerging #1361 will not alter performanceComparing Summary
|
React Example#3413 Bundle Size — 235.18KiB (0%).17b9073(current) vs a2c8e11 main#3408(baseline) Bundle metrics
|
| Current #3413 |
Baseline #3408 |
|
|---|---|---|
 Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
156 |
156 |
| Duplicate Modules |
63 |
63 |
| Duplicate Code |
45.94% |
45.94% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #3413 |
Baseline #3408 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
89.42KiB |
89.42KiB |
Bundle analysis report Branch PupilTong:p/hw/wasm-feature-dete... Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#3403 Bundle Size — 352.53KiB (+14.91%).17b9073(current) vs a2c8e11 main#3398(baseline) Warning Bundle introduced one new package: wasm-feature-detect – View changed packages Bundle metrics
|
| Current #3403 |
Baseline #3398 |
|
|---|---|---|
 Initial JS |
142.12KiB(+0.02%) |
142.09KiB |
| Initial CSS |
31.83KiB |
31.83KiB |
 Cache Invalidation |
86.38% |
86.25% |
| Chunks |
7(+16.67%) |
6 |
| Assets |
7(+16.67%) |
6 |
| Modules |
215(+2.87%) |
209 |
| Duplicate Modules |
17 |
17 |
| Duplicate Code |
3.94%(-6.86%) |
4.23% |
| Packages |
4(+33.33%) |
3 |
| Duplicate Packages |
0 |
0 |
Bundle size by type 2 changes 2 regressions
| Current #3403 |
Baseline #3398 |
|
|---|---|---|
| JS |
226.4KiB (+1.03%) |
224.09KiB |
| Other |
94.29KiB (+85.35%) |
50.87KiB |
| CSS |
31.83KiB |
31.83KiB |
Bundle analysis report Branch PupilTong:p/hw/wasm-feature-dete... Project dashboard
Generated by RelativeCI Documentation Report issue
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @lynx-js/[email protected] ### Minor Changes - Add support for Lynx UI plugin system with configurable options. ([#1363](#1363)) - Introduced `lynxUIPlugins` option in `createLynxPreset`, allowing userland opt-in to Lynx UI specific plugins. - Implemented `uiVariants` plugin as the first UI plugin, supporting `ui-*` variant prefixes (e.g. `ui-checked`, `ui-open`) with customizable mappings. ## @lynx-js/[email protected] ### Patch Changes - Fix crash caused by not removing event listeners during destroy. ([#1379](#1379)) - Fix missing "type" in "update-list-info" in hydrate ([#1392](#1392)) ## @lynx-js/[email protected] ### Patch Changes - Bump Rsbuild v1.4.12 with Rspack v1.4.11. ([#1326](#1326)) ## @lynx-js/[email protected] ### Patch Changes - Updated dependencies \[[`e9edca0`](e9edca0), [`6f37db2`](6f37db2)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - feat: support `__ElementFromBinary` ([#1391](#1391)) - Updated dependencies \[]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - feat: support `__ElementFromBinary` ([#1391](#1391)) - fix: crash on chrome<96 ([#1361](#1361)) <wasm-bindgen/wasm-bindgen#4211 (comment)> <WebAssembly/binaryen#7358> The rust toolchain enables WASM feature `reference types` by default. However this feature is not supported by chromium lower than version 96 Therefore we found a workaround for it. In this implementation we detect if browser supports `reference types` first. If user's browser supported it, we load the wasm file with `reference types` on, otherwise we load the wasm file with `reference types` off. - Updated dependencies \[[`22ca433`](22ca433), [`8645d12`](8645d12), [`143e481`](143e481)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - refactor: bundle web-core-server ([#819](#819)) ## @lynx-js/[email protected] ### Patch Changes - feat: support color style for x-textarea ([#1382](#1382)) - Updated dependencies \[]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - feat: support `__ElementFromBinary` ([#1391](#1391)) - fix: crash on chrome<96 ([#1361](#1361)) <wasm-bindgen/wasm-bindgen#4211 (comment)> <WebAssembly/binaryen#7358> The rust toolchain enables WASM feature `reference types` by default. However this feature is not supported by chromium lower than version 96 Therefore we found a workaround for it. In this implementation we detect if browser supports `reference types` first. If user's browser supported it, we load the wasm file with `reference types` on, otherwise we load the wasm file with `reference types` off. - Updated dependencies \[[`22ca433`](22ca433)]: - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - optimize IO for all-on-ui: make startMainThreadWorker async and defer import ([#1385](#1385)) - Updated dependencies \[[`22ca433`](22ca433), [`143e481`](143e481)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Support Rspack v1.4.9. ([#1351](#1351)) ## @lynx-js/[email protected] ### Patch Changes - feat: support elementTemplate for web ([#1374](#1374)) ## [email protected] ## @lynx-js/[email protected] ## [email protected] ## @lynx-js/[email protected] ## @lynx-js/[email protected] ## @lynx-js/[email protected] Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
2 participants
wasm-bindgen/wasm-bindgen#4211 (comment)
WebAssembly/binaryen#7358
The rust toolchain enables WASM feature
reference typesby default.However this feature is not supported by chromium lower than version 96
Therefore we found a workaround for it.
In this implementation we detect if browser supports
reference typesfirst.If user's browser supported it, we load the wasm file with
reference typeson, otherwise we load the wasm file withreference typesoff.relies on #819
Summary by CodeRabbit