fix: avoid crash on user's CPU does not support simd#2133
fix: avoid crash on user's CPU does not support simd#2133PupilTong merged 2 commits intolynx-family:mainfrom
Conversation
🦋 Changeset detectedLatest commit: fef070c The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 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 |
📝 WalkthroughWalkthroughAdds SIMD detection alongside reference-types for WASM feature checks, gating WASM bundle loading on both features; updates tests to mock SIMD and adds a changelog entry documenting a fix for SIMD-related crashes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.changeset/bitter-mails-boil.md:
- Line 5: The release note line "fix: avoid crash on user's CPU does not support
simd" is grammatically awkward; update the text to a clear, concise phrasing
such as "fix: avoid crash when user's CPU does not support SIMD" (or "fix: avoid
crash on CPUs without SIMD support") to improve clarity and tone—edit the string
in .changeset/bitter-mails-boil.md accordingly.
🧹 Nitpick comments (1)
packages/web-platform/web-core-wasm/ts/client/wasm.ts (1)
6-38: Refactor to async/await for clarity in feature-detection flow.The feature detection resolves to boolean values (
true/false) rather than rejecting, so the current code is safe. However, the async/await pattern aligns better with the project's TypeScript/JavaScript guidelines and makes the conditional logic more readable.♻️ Suggested refactor
-const wasmLoaded = Promise.all([referenceTypes(), simd()]).then( - ([supportsReferenceTypes, supportsSimd]) => { - if (supportsReferenceTypes && supportsSimd) { - return Promise.all([ - import( - /* webpackMode: "eager" */ - /* webpackFetchPriority: "high" */ - /* webpackPrefetch: true */ - /* webpackPreload: true */ - '../../binary/client/client.js' - ), - isWorker ? undefined : WebAssembly.compileStreaming( - fetch( - new URL( - /* webpackChunkName: "standard-wasm" */ - /* webpackMode: "eager" */ - /* webpackFetchPriority: "high" */ - /* webpackPrefetch: true */ - /* webpackPreload: true */ - '../../binary/client/client_bg.wasm', - import.meta.url, - ), - ), - ), - ]); - } else { - throw new Error('WASM not supported'); - } - }, -); +const wasmLoaded = (async () => { + const [supportsReferenceTypes, supportsSimd] = await Promise.all([ + referenceTypes(), + simd(), + ]); + if (!supportsReferenceTypes || !supportsSimd) { + throw new Error('WASM not supported'); + } + return Promise.all([ + import( + /* webpackMode: "eager" */ + /* webpackFetchPriority: "high" */ + /* webpackPrefetch: true */ + /* webpackPreload: true */ + '../../binary/client/client.js' + ), + isWorker ? undefined : WebAssembly.compileStreaming( + fetch( + new URL( + /* webpackChunkName: "standard-wasm" */ + /* webpackMode: "eager" */ + /* webpackFetchPriority: "high" */ + /* webpackPrefetch: true */ + /* webpackPreload: true */ + '../../binary/client/client_bg.wasm', + import.meta.url, + ), + ), + ), + ]); +})();
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Merging this PR will degrade performance by 8.67%
Performance Changes
Comparing Footnotes
|
Web Explorer#7315 Bundle Size — 384.14KiB (+0.05%).fef070c(current) vs f7133c1 main#7309(baseline) Bundle metrics
Bundle size by type
Bundle analysis report Branch PupilTong:p/hw/fix-simd128-issue Project dashboard Generated by RelativeCI Documentation Report issue |
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/react@0.116.0 ### Minor Changes - **BREAKING CHANGE**: Bump Preact from [10.24.0](preactjs/preact@1807173) to [10.28.0](preactjs/preact@f7693b7), see diffs at [hzy/preact#6](hzy/preact#6). ([#2042](#2042)) ### Patch Changes - Add safety checks for compilation macros to prevent runtime errors when they are undefined. ([#2110](#2110)) Replaces direct usage of `__PROFILE__`, `__MAIN_THREAD__`, `__BACKGROUND__` with `typeof` checks. This improves robustness by checking variable existence before access, preventing runtime errors in environments where compilation macros are not defined. ## @lynx-js/lynx-bundle-rslib-config@0.2.0 ### Minor Changes - Use `LAYERS` exposed by DSL plugins ([#2114](#2114)) ## @lynx-js/gesture-runtime@2.1.2 ### Patch Changes - Fix an issue that `NativeGesture` does not get correct types in callback ([#2130](#2130)) ## @lynx-js/rspeedy@0.13.1 ### Patch Changes - Updated dependencies \[]: - @lynx-js/web-rsbuild-server-middleware@0.19.6 ## @lynx-js/react-rsbuild-plugin@0.12.6 ### Patch Changes - Support using `pluginReactLynx` in Rslib. ([#2114](#2114)) - Updated dependencies \[[`4cd7182`](4cd7182)]: - @lynx-js/template-webpack-plugin@0.10.2 - @lynx-js/react-alias-rsbuild-plugin@0.12.6 - @lynx-js/use-sync-external-store@1.5.0 - @lynx-js/react-refresh-webpack-plugin@0.3.4 - @lynx-js/react-webpack-plugin@0.7.3 - @lynx-js/css-extract-webpack-plugin@0.7.0 ## upgrade-rspeedy@0.13.1 ### Patch Changes - Fix the issue `rslib-runtime.js` was not published in dist folder. ([#2122](#2122)) ## @lynx-js/testing-environment@0.1.10 ### Patch Changes - Fix the error "lynxTestingEnv is not defined" ([#2118](#2118)) ## @lynx-js/web-constants@0.19.6 ### Patch Changes - feat: add main-thread API: \_\_QuerySelector ([#2115](#2115)) - fix: when a list-item is deleted from list, the deleted list-item is still showed incorrectly. ([#1092](#1092)) This is because the `enqueueComponent` method does not delete the node from the Element Tree. It is only to maintain the display node on RL, and lynx web needs to delete the dom additionally. - feat: support main thread invoke ui method ([#2104](#2104)) - feat: support lynx.reload() ([#2127](#2127)) - Updated dependencies \[[`f7133c1`](f7133c1)]: - @lynx-js/web-worker-rpc@0.19.6 ## @lynx-js/web-core@0.19.6 ### Patch Changes - fix: avoid crash on CPUs that do not support SIMD ([#2133](#2133)) - feat: support lynx.reload() ([#2127](#2127)) - Updated dependencies \[[`179f984`](179f984), [`f7133c1`](f7133c1), [`6c2b51a`](6c2b51a), [`556fe9f`](556fe9f), [`5b589ab`](5b589ab)]: - @lynx-js/web-constants@0.19.6 - @lynx-js/web-mainthread-apis@0.19.6 - @lynx-js/web-worker-rpc@0.19.6 - @lynx-js/web-worker-runtime@0.19.6 ## @lynx-js/web-core-wasm@0.0.1 ### Patch Changes - Updated dependencies \[[`f7133c1`](f7133c1)]: - @lynx-js/web-worker-rpc@0.19.6 ## @lynx-js/web-mainthread-apis@0.19.6 ### Patch Changes - feat: add main-thread API: \_\_QuerySelector ([#2115](#2115)) - fix: when a list-item is deleted from list, the deleted list-item is still showed incorrectly. ([#1092](#1092)) This is because the `enqueueComponent` method does not delete the node from the Element Tree. It is only to maintain the display node on RL, and lynx web needs to delete the dom additionally. - feat: support main thread invoke ui method ([#2104](#2104)) - fix: mts && bts events can be binded both ([#2121](#2121)) - Updated dependencies \[[`179f984`](179f984), [`f7133c1`](f7133c1), [`6c2b51a`](6c2b51a), [`5b589ab`](5b589ab)]: - @lynx-js/web-constants@0.19.6 ## @lynx-js/web-worker-rpc@0.19.6 ### Patch Changes - fix: when a list-item is deleted from list, the deleted list-item is still showed incorrectly. ([#1092](#1092)) This is because the `enqueueComponent` method does not delete the node from the Element Tree. It is only to maintain the display node on RL, and lynx web needs to delete the dom additionally. ## @lynx-js/web-worker-runtime@0.19.6 ### Patch Changes - feat: support lynx.reload() ([#2127](#2127)) - Updated dependencies \[[`179f984`](179f984), [`f7133c1`](f7133c1), [`6c2b51a`](6c2b51a), [`556fe9f`](556fe9f), [`5b589ab`](5b589ab)]: - @lynx-js/web-constants@0.19.6 - @lynx-js/web-mainthread-apis@0.19.6 - @lynx-js/web-worker-rpc@0.19.6 ## @lynx-js/template-webpack-plugin@0.10.2 ### Patch Changes - Polyfill `lynx.requireModuleAsync` to allow cache same parallel requests. ([#2108](#2108)) ## create-rspeedy@0.13.1 ## @lynx-js/react-alias-rsbuild-plugin@0.12.6 ## @lynx-js/web-core-server@0.19.6 ## @lynx-js/web-rsbuild-server-middleware@0.19.6 Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary by CodeRabbit
Bug Fixes
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.
Checklist