fix: handle lynx.getJSModule error on web platform #1830
fix: handle lynx.getJSModule error on web platform #1830PupilTong merged 4 commits intolynx-family:mainfrom
lynx.getJSModule error on web platform #1830Conversation
🦋 Changeset detectedLatest commit: 167d169 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 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 a patch changeset and updates the dev plugin to split HMR ProvidePlugin entries: always register the dev-server client entry and register a Lynx-only WebSocket transport under a new plugin path when running in a Lynx environment; tests updated to reflect these behaviors and assertion changes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/rspeedy/core/test/plugins/dev.plugin.test.ts (1)
102-121: Fix casing in test title ("WebSocket").Prefer the canonical “WebSocket” spelling.
- test('no Websocket class injected for web', async () => { + test('no WebSocket class injected for web', async () => {packages/rspeedy/core/src/plugins/dev.plugin.ts (1)
153-177: Env‑gated ProvidePlugin config is correct; consider de‑duplicating the client entry.You can avoid duplicating webpack_dev_server_client by using a single object with a conditional spread.
- .use(ProvidePlugin, [ - isLynx(environment) ? { - WebSocket: [ - options?.client?.websocketTransport ?? require.resolve('@lynx-js/websocket'), - 'default', - ], - __webpack_dev_server_client__: [ - require.resolve( - './client/hmr/WebSocketClient.js', - { - paths: [rspeedyDir], - }, - ), - 'default' - ], - } : { - __webpack_dev_server_client__: [ - require.resolve( - './client/hmr/WebSocketClient.js', - { - paths: [rspeedyDir], - }, - ), - 'default' - ], - } - ]) + .use(ProvidePlugin, [ + { + ...(isLynx(environment) ? { + WebSocket: [ + options?.client?.websocketTransport ?? require.resolve('@lynx-js/websocket'), + 'default', + ], + } : {}), + __webpack_dev_server_client__: [ + require.resolve('./client/hmr/WebSocketClient.js', { + paths: [rspeedyDir], + }), + 'default', + ], + } + ])
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/crazy-jeans-hunt.md(1 hunks)packages/rspeedy/core/src/plugins/dev.plugin.ts(3 hunks)packages/rspeedy/core/test/plugins/dev.plugin.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.changeset/*.md
📄 CodeRabbit inference engine (AGENTS.md)
For contributions, always generate a changeset and commit the resulting markdown file(s)
Files:
.changeset/crazy-jeans-hunt.md
🧠 Learnings (7)
📓 Common learnings
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1454
File: pnpm-workspace.yaml:46-46
Timestamp: 2025-08-07T04:00:59.645Z
Learning: In the lynx-family/lynx-stack repository, the webpack patch (patches/webpack5.101.0.patch) was created to fix issues with webpack5.99.9 but only takes effect on webpack5.100.0 and later versions. The patchedDependencies entry should use "webpack@^5.100.0" to ensure the patch applies to the correct version range.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1612
File: packages/rspeedy/create-rspeedy/template-react-vitest-rltl-ts/src/tsconfig.json:3-13
Timestamp: 2025-08-27T08:10:09.932Z
Learning: In the lynx-family/lynx-stack repository, Rspeedy templates use `lynx-js/rspeedy/client` types via `rspeedy-env.d.ts` instead of `vite/client` types. Rspeedy provides its own client-side environment type definitions and doesn't require direct Vite type references.
Learnt from: upupming
PR: lynx-family/lynx-stack#1616
File: packages/webpack/cache-events-webpack-plugin/test/cases/not-cache-events/lazy-bundle/index.js:3-3
Timestamp: 2025-08-27T12:42:01.095Z
Learning: In webpack, properties like __webpack_require__.lynx_ce are injected during compilation/build time when webpack processes modules and generates bundles, not at runtime when dynamic imports execute. Tests for such properties don't need to wait for dynamic imports to complete.
📚 Learning: 2025-08-27T12:42:01.095Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1616
File: packages/webpack/cache-events-webpack-plugin/test/cases/not-cache-events/lazy-bundle/index.js:3-3
Timestamp: 2025-08-27T12:42:01.095Z
Learning: In webpack, properties like __webpack_require__.lynx_ce are injected during compilation/build time when webpack processes modules and generates bundles, not at runtime when dynamic imports execute. Tests for such properties don't need to wait for dynamic imports to complete.
Applied to files:
packages/rspeedy/core/test/plugins/dev.plugin.test.tspackages/rspeedy/core/src/plugins/dev.plugin.ts
📚 Learning: 2025-09-12T09:43:04.847Z
Learnt from: gaoachao
PR: lynx-family/lynx-stack#1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.847Z
Learning: In the lynx-family/lynx-stack repository, empty changeset files (containing only `---\n\n---`) are used for internal changes that modify src/** files but don't require meaningful release notes, such as private package changes or testing-only modifications. This satisfies CI requirements without generating user-facing release notes.
Applied to files:
.changeset/crazy-jeans-hunt.md
📚 Learning: 2025-09-12T09:43:04.847Z
Learnt from: gaoachao
PR: lynx-family/lynx-stack#1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.847Z
Learning: In the lynx-family/lynx-stack repository, private packages (marked with "private": true in package.json) like lynx-js/react-transform don't require meaningful changeset entries even when their public APIs change, since they are not published externally and only affect internal development.
Applied to files:
.changeset/crazy-jeans-hunt.md
📚 Learning: 2025-08-07T04:00:59.645Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1454
File: pnpm-workspace.yaml:46-46
Timestamp: 2025-08-07T04:00:59.645Z
Learning: In the lynx-family/lynx-stack repository, the webpack patch (patches/webpack5.101.0.patch) was created to fix issues with webpack5.99.9 but only takes effect on webpack5.100.0 and later versions. The patchedDependencies entry should use "webpack@^5.100.0" to ensure the patch applies to the correct version range.
Applied to files:
.changeset/crazy-jeans-hunt.md
📚 Learning: 2025-08-27T08:10:09.932Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1612
File: packages/rspeedy/create-rspeedy/template-react-vitest-rltl-ts/src/tsconfig.json:3-13
Timestamp: 2025-08-27T08:10:09.932Z
Learning: In the lynx-family/lynx-stack repository, Rspeedy templates use `lynx-js/rspeedy/client` types via `rspeedy-env.d.ts` instead of `vite/client` types. Rspeedy provides its own client-side environment type definitions and doesn't require direct Vite type references.
Applied to files:
.changeset/crazy-jeans-hunt.mdpackages/rspeedy/core/src/plugins/dev.plugin.ts
📚 Learning: 2025-08-21T08:46:54.494Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1370
File: packages/webpack/cache-events-webpack-plugin/src/LynxCacheEventsRuntimeModule.ts:23-27
Timestamp: 2025-08-21T08:46:54.494Z
Learning: In Lynx webpack runtime modules, the team prioritizes performance and simplicity over defensive runtime error handling. They prefer relying on compile-time type safety (TypeScript) rather than adding runtime checks like try-catch blocks or type validation, especially for performance-critical code like cache event setup/cleanup functions.
Applied to files:
.changeset/crazy-jeans-hunt.mdpackages/rspeedy/core/src/plugins/dev.plugin.ts
🧬 Code graph analysis (2)
packages/rspeedy/core/test/plugins/dev.plugin.test.ts (1)
packages/rspeedy/core/test/createStubRspeedy.ts (1)
createStubRspeedy(20-64)
packages/rspeedy/core/src/plugins/dev.plugin.ts (1)
packages/rspeedy/core/src/utils/is-lynx.ts (1)
isLynx(7-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build / Build (Windows)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: test-rust / Test (Ubuntu)
🔇 Additional comments (2)
.changeset/crazy-jeans-hunt.md (1)
5-5: Changeset LGTM; scope is correct.Patch entry for @lynx-js/rspeedy with concise note is appropriate for this fix.
packages/rspeedy/core/src/plugins/dev.plugin.ts (1)
15-15: LGTM: importing isLynx for env gating.This keeps the web build free of Lynx-only shims.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
React Example#5528 Bundle Size — 237.56KiB (0%).5eb736c(current) vs 909a842 main#5525(baseline) Bundle metrics
|
| Current #5528 |
Baseline #5525 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
4 |
4 |
|
166 |
166 |
|
68 |
68 |
|
46.81% |
46.81% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #5528 |
Baseline #5525 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
91.8KiB |
91.8KiB |
Bundle analysis report Branch PupilTong:p/hw/rspeedy-remove-we... Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#5523 Bundle Size — 365.44KiB (0%).5eb736c(current) vs 909a842 main#5520(baseline) Bundle metrics
|
| Current #5523 |
Baseline #5520 |
|
|---|---|---|
145.71KiB |
145.71KiB |
|
32KiB |
32KiB |
|
0% |
0% |
|
8 |
8 |
|
8 |
8 |
|
219 |
219 |
|
16 |
16 |
|
3.37% |
3.37% |
|
4 |
4 |
|
0 |
0 |
Bundle size by type no changes
| Current #5523 |
Baseline #5520 |
|
|---|---|---|
239.42KiB |
239.42KiB |
|
94.02KiB |
94.02KiB |
|
32KiB |
32KiB |
Bundle analysis report Branch PupilTong:p/hw/rspeedy-remove-we... Project dashboard
Generated by RelativeCI Documentation Report issue
CodSpeed Performance ReportMerging #1830 will degrade performances by 5.51%Comparing Summary
Benchmarks breakdown
Footnotes
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/rspeedy/core/src/plugins/dev.plugin.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1454
File: pnpm-workspace.yaml:46-46
Timestamp: 2025-08-07T04:00:59.645Z
Learning: In the lynx-family/lynx-stack repository, the webpack patch (patches/webpack5.101.0.patch) was created to fix issues with webpack5.99.9 but only takes effect on webpack5.100.0 and later versions. The patchedDependencies entry should use "webpack@^5.100.0" to ensure the patch applies to the correct version range.
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.
📚 Learning: 2025-08-27T08:10:09.932Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1612
File: packages/rspeedy/create-rspeedy/template-react-vitest-rltl-ts/src/tsconfig.json:3-13
Timestamp: 2025-08-27T08:10:09.932Z
Learning: In the lynx-family/lynx-stack repository, Rspeedy templates use `lynx-js/rspeedy/client` types via `rspeedy-env.d.ts` instead of `vite/client` types. Rspeedy provides its own client-side environment type definitions and doesn't require direct Vite type references.
Applied to files:
packages/rspeedy/core/src/plugins/dev.plugin.ts
📚 Learning: 2025-08-27T12:42:01.095Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1616
File: packages/webpack/cache-events-webpack-plugin/test/cases/not-cache-events/lazy-bundle/index.js:3-3
Timestamp: 2025-08-27T12:42:01.095Z
Learning: In webpack, properties like __webpack_require__.lynx_ce are injected during compilation/build time when webpack processes modules and generates bundles, not at runtime when dynamic imports execute. Tests for such properties don't need to wait for dynamic imports to complete.
Applied to files:
packages/rspeedy/core/src/plugins/dev.plugin.ts
🧬 Code graph analysis (1)
packages/rspeedy/core/src/plugins/dev.plugin.ts (1)
packages/rspeedy/core/src/utils/is-lynx.ts (1)
isLynx(7-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build / Build (Windows)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: test-rust / Test (Ubuntu)
Co-authored-by: Qingyu Wang <40660121+colinaaa@users.noreply.github.com> Signed-off-by: Haoyang Wang <12288479+PupilTong@users.noreply.github.com>
167d169 to
5eb736c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/crazy-jeans-hunt.md(1 hunks)packages/rspeedy/core/src/plugins/dev.plugin.ts(3 hunks)packages/rspeedy/core/test/plugins/dev.plugin.test.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .changeset/crazy-jeans-hunt.md
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1454
File: pnpm-workspace.yaml:46-46
Timestamp: 2025-08-07T04:00:59.645Z
Learning: In the lynx-family/lynx-stack repository, the webpack patch (patches/webpack5.101.0.patch) was created to fix issues with webpack5.99.9 but only takes effect on webpack5.100.0 and later versions. The patchedDependencies entry should use "webpack@^5.100.0" to ensure the patch applies to the correct version range.
📚 Learning: 2025-08-27T08:10:09.932Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1612
File: packages/rspeedy/create-rspeedy/template-react-vitest-rltl-ts/src/tsconfig.json:3-13
Timestamp: 2025-08-27T08:10:09.932Z
Learning: In the lynx-family/lynx-stack repository, Rspeedy templates use `lynx-js/rspeedy/client` types via `rspeedy-env.d.ts` instead of `vite/client` types. Rspeedy provides its own client-side environment type definitions and doesn't require direct Vite type references.
Applied to files:
packages/rspeedy/core/src/plugins/dev.plugin.ts
📚 Learning: 2025-09-12T09:43:04.847Z
Learnt from: gaoachao
PR: lynx-family/lynx-stack#1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.847Z
Learning: In the lynx-family/lynx-stack repository, private packages (marked with "private": true in package.json) like lynx-js/react-transform don't require meaningful changeset entries even when their public APIs change, since they are not published externally and only affect internal development.
Applied to files:
packages/rspeedy/core/src/plugins/dev.plugin.ts
📚 Learning: 2025-08-27T12:42:01.095Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1616
File: packages/webpack/cache-events-webpack-plugin/test/cases/not-cache-events/lazy-bundle/index.js:3-3
Timestamp: 2025-08-27T12:42:01.095Z
Learning: In webpack, properties like __webpack_require__.lynx_ce are injected during compilation/build time when webpack processes modules and generates bundles, not at runtime when dynamic imports execute. Tests for such properties don't need to wait for dynamic imports to complete.
Applied to files:
packages/rspeedy/core/src/plugins/dev.plugin.tspackages/rspeedy/core/test/plugins/dev.plugin.test.ts
📚 Learning: 2025-08-07T04:00:59.645Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1454
File: pnpm-workspace.yaml:46-46
Timestamp: 2025-08-07T04:00:59.645Z
Learning: In the lynx-family/lynx-stack repository, the webpack patch (patches/webpack5.101.0.patch) was created to fix issues with webpack5.99.9 but only takes effect on webpack5.100.0 and later versions. The patchedDependencies entry should use "webpack@^5.100.0" to ensure the patch applies to the correct version range.
Applied to files:
packages/rspeedy/core/src/plugins/dev.plugin.ts
🧬 Code graph analysis (2)
packages/rspeedy/core/src/plugins/dev.plugin.ts (1)
packages/rspeedy/core/src/utils/is-lynx.ts (1)
isLynx(7-11)
packages/rspeedy/core/test/plugins/dev.plugin.test.ts (1)
packages/rspeedy/core/test/createStubRspeedy.ts (1)
createStubRspeedy(20-64)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: build / Build (Windows)
- GitHub Check: test-rust / Test (Ubuntu)
🔇 Additional comments (2)
packages/rspeedy/core/test/plugins/dev.plugin.test.ts (1)
76-84:toHaveBeenCalledWithmatcher aligns with Vitest API.Thanks for migrating to the modern matcher; it keeps the assertion style consistent with Vitest/Jest conventions.
packages/rspeedy/core/src/plugins/dev.plugin.ts (1)
15-15: Don't drop the WebSocket polyfill for node-side bundles.Gating the
WebSocketProvidePlugin behindisLynx(environment)removes the polyfill for environments like Node (whereenvironment.name === 'node'), breaking HMR there. The intent was to skip the injection only for the web target that lackslynx.getJSModule, so please invert the predicate to “not web” instead of “is Lynx” and import the corresponding helper. (Same concern noted earlier in review.)-import { isLynx } from '../utils/is-lynx.js' +import { isWeb } from '../utils/is-web.js' @@ - if (isLynx(environment)) { + if (!isWeb(environment)) {Also applies to: 166-175
lynx-family#1806 <!-- Thank you for submitting a pull request! We appreciate the time and effort you have invested in making these changes. Please ensure that you provide enough information to allow others to review your pull request. Upon submission, your pull request will be automatically assigned with reviewers. If you want to learn more about contributing to this project, please visit: https://github.com/lynx-family/lynx-stack/blob/main/CONTRIBUTING.md. --> <!-- The AI summary below will be auto-generated - feel free to replace it with your own. --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - New Features - Environment-aware dev server client setup for smoother web development. - Bug Fixes - Resolved a web runtime error causing “getJSModule is not a function” during development. - Prevented unnecessary WebSocket injection on web to avoid startup issues. - Tests - Added coverage to verify correct client configuration and absence of WebSocket injection on web. - Chores - Published a patch release for @lynx-js/rspeedy. <!-- end of auto-generated comment: release notes by coderabbit.ai --> ## Checklist <!--- Check and mark with an "x" --> - [x] Tests updated (or not required). - [ ] Documentation updated (or not required). - [x] Changeset added, and when a BREAKING CHANGE occurs, it needs to be clearly marked (or not required). --------- Signed-off-by: Haoyang Wang <12288479+PupilTong@users.noreply.github.com> Co-authored-by: Qingyu Wang <40660121+colinaaa@users.noreply.github.com>
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/rspeedy@0.11.4 ### Patch Changes - Bump Rsbuild v1.5.12 with Rspack v1.5.7. ([#1708](#1708)) - Fix the "lynx.getJSModule is not a function" error on Web platform ([#1830](#1830)) - Support `server.compress` ([#1799](#1799)) - Support `server.cors` ([#1808](#1808)) ## @lynx-js/qrcode-rsbuild-plugin@0.4.2 ### Patch Changes - Bump @clack/prompts v1.0.0-alpha.5. ([#1809](#1809)) ## @lynx-js/web-constants@0.17.1 ### Patch Changes - Updated dependencies \[]: - @lynx-js/web-worker-rpc@0.17.1 ## @lynx-js/web-core@0.17.1 ### Patch Changes - Updated dependencies \[]: - @lynx-js/web-constants@0.17.1 - @lynx-js/web-mainthread-apis@0.17.1 - @lynx-js/web-worker-rpc@0.17.1 - @lynx-js/web-worker-runtime@0.17.1 ## @lynx-js/web-mainthread-apis@0.17.1 ### Patch Changes - Updated dependencies \[]: - @lynx-js/web-constants@0.17.1 - @lynx-js/web-style-transformer@0.17.1 ## @lynx-js/web-rsbuild-server-middleware@0.17.1 ### Patch Changes - chore: initial release with current version web platform ([#1807](#1807)) ## @lynx-js/web-worker-runtime@0.17.1 ### Patch Changes - Updated dependencies \[]: - @lynx-js/web-constants@0.17.1 - @lynx-js/web-mainthread-apis@0.17.1 - @lynx-js/web-worker-rpc@0.17.1 ## create-rspeedy@0.11.4 ## upgrade-rspeedy@0.11.4 ## @lynx-js/web-core-server@0.17.1 ## @lynx-js/web-style-transformer@0.17.1 ## @lynx-js/web-worker-rpc@0.17.1 Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
#1806
Summary by CodeRabbit
Checklist