feat: support lazy message port assigning in web-worker-rpc#2040
feat: support lazy message port assigning in web-worker-rpc#2040PupilTong merged 1 commit intolynx-family:mainfrom
Conversation
🦋 Changeset detectedLatest commit: 2d4361c The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 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 PR adds lazy message port assignment support to the web-worker-rpc package. The Rpc constructor now accepts an optional port parameter, stores pending messages in a queue, and provides a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/web-platform/web-worker-rpc/test/rpc.test.ts (1)
125-139: Consider verifying that queued messages are actually sent.The test verifies that setting the port twice throws an error, but doesn't confirm that the message posted on line 129-131 (before the port was set) is actually flushed and sent when
setMessagePortis called on line 133. Consider adding an assertion or event listener to verify the queued message was delivered.Example enhancement to verify message delivery
test('set message port', async () => { const channel = new MessageChannel(); + const receivedMessages: any[] = []; + channel.port2.onmessage = (ev) => { + receivedMessages.push(ev.data); + }; + channel.port2.start(); + // buffer flush const rpc = new Rpc(undefined, 'test'); rpc.postMessage({ type: 'test', }); // flush rpc.setMessagePort(channel.port1); + await new Promise(resolve => setTimeout(resolve, 0)); // Let message propagate + expect(receivedMessages).toHaveLength(1); + expect(receivedMessages[0]).toEqual({ type: 'test' }); expect(() => { rpc.setMessagePort(channel.port1); }).toThrow('Rpc port already set'); channel.port1.close(); channel.port2.close(); });packages/web-platform/web-worker-rpc/src/Rpc.ts (1)
95-104: Consider adding type guards or JSDoc for the message parameter.The method accepts
unknownbut casts toRpcMessageData | RpcMessageDataSyncon line 100. While this works in practice since the method is only called internally with correct types, consider either:
- Narrowing the parameter type to
RpcMessageData | RpcMessageDataSync- Adding JSDoc to document the expected message structure
- Adding a runtime type guard
This would improve type safety and make the contract clearer.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.changeset/curvy-knives-wink.mdpackages/web-platform/web-worker-rpc/src/Rpc.tspackages/web-platform/web-worker-rpc/test/endpoints.jspackages/web-platform/web-worker-rpc/test/rpc.test.tspackages/web-platform/web-worker-rpc/test/worker.js
🧰 Additional context used
📓 Path-based instructions (1)
.changeset/*.md
📄 CodeRabbit inference engine (AGENTS.md)
For contributions, generate and commit a Changeset describing your changes
Files:
.changeset/curvy-knives-wink.md
🧠 Learnings (7)
📚 Learning: 2025-08-29T16:57:19.221Z
Learnt from: PupilTong
Repo: lynx-family/lynx-stack PR: 1235
File: packages/web-platform/web-mainthread-apis/src/crossThreadHandlers/createQueryComponent.ts:24-26
Timestamp: 2025-08-29T16:57:19.221Z
Learning: In the Lynx RPC system, when createRpcEndpoint has hasReturn=true (third parameter), even if the return type is void, it returns a Promise<void> that resolves when the work on the background thread is completed. This allows proper awaiting of cross-thread operations.
Applied to files:
packages/web-platform/web-worker-rpc/test/endpoints.jspackages/web-platform/web-worker-rpc/test/worker.jspackages/web-platform/web-worker-rpc/test/rpc.test.tspackages/web-platform/web-worker-rpc/src/Rpc.ts
📚 Learning: 2025-09-12T09:43:04.847Z
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.847Z
Learning: In the lynx-family/lynx-stack repository, 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/curvy-knives-wink.md
📚 Learning: 2025-09-12T09:43:04.847Z
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.847Z
Learning: In the lynx-family/lynx-stack repository, private packages (marked with "private": true in package.json) like lynx-js/react-transform don't require meaningful changeset entries even when their public APIs change, since they are not published externally and only affect internal development.
Applied to files:
.changeset/curvy-knives-wink.md
📚 Learning: 2025-07-22T09:23:07.797Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack 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.
Applied to files:
.changeset/curvy-knives-wink.md
📚 Learning: 2025-07-22T09:26:16.722Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack 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.
Applied to files:
.changeset/curvy-knives-wink.md
📚 Learning: 2025-08-07T04:00:59.645Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 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/curvy-knives-wink.md
📚 Learning: 2025-08-29T17:32:08.078Z
Learnt from: PupilTong
Repo: lynx-family/lynx-stack PR: 1235
File: packages/web-platform/web-constants/src/endpoints.ts:248-261
Timestamp: 2025-08-29T17:32:08.078Z
Learning: In the Lynx RPC system, Promise<void> with hasReturn=true is meaningful for cross-thread operations as it allows callers to await completion of background work, even when no data is returned. The Promise<void> serves as a completion signal rather than a data carrier.
Applied to files:
packages/web-platform/web-worker-rpc/src/Rpc.ts
🧬 Code graph analysis (3)
packages/web-platform/web-worker-rpc/test/worker.js (1)
packages/web-platform/web-worker-rpc/test/endpoints.js (2)
callbackifyEndpoint(57-62)callbackifyEndpoint(57-62)
packages/web-platform/web-worker-rpc/test/rpc.test.ts (2)
packages/web-platform/web-worker-rpc/test/endpoints.js (2)
callbackifyEndpoint(57-62)callbackifyEndpoint(57-62)packages/web-platform/web-worker-rpc/src/Rpc.ts (1)
Rpc(38-509)
packages/web-platform/web-worker-rpc/src/Rpc.ts (2)
packages/web-platform/web-worker-rpc/test/worker.js (1)
port(27-27)packages/web-platform/web-tests/rspack.config.js (1)
port(18-18)
🔇 Additional comments (10)
.changeset/curvy-knives-wink.md (1)
1-5: LGTM!The changeset correctly documents this feature addition with an appropriate patch-level bump for the
@lynx-js/web-worker-rpcpackage.packages/web-platform/web-worker-rpc/test/rpc.test.ts (2)
113-123: LGTM!The callbackify test correctly exercises the
createCallbackifymethod to wrap an async endpoint with a callback-style API.
141-145: LGTM!The test correctly verifies that handlers can be registered and removed.
packages/web-platform/web-worker-rpc/test/worker.js (1)
13-13: LGTM!The new
callbackifyEndpointhandler is correctly imported and registered, following the established pattern for other test endpoints.Also applies to: 54-56
packages/web-platform/web-worker-rpc/test/endpoints.js (1)
57-62: LGTM!The new endpoint definition follows the established pattern and correctly configures an async endpoint with return value.
packages/web-platform/web-worker-rpc/src/Rpc.ts (5)
41-44: LGTM!The message queue structure is well-defined and properly encapsulated using a private field.
76-80: LGTM!The constructor correctly supports optional port initialization, enabling the lazy assignment pattern. Existing callers that provide a port will continue to work as before.
82-93: LGTM!The
setMessagePortmethod correctly:
- Guards against re-initialization
- Flushes queued messages once the port is available
- Properly attaches the message handler
438-438: LGTM!Replacing direct
port.postMessagecalls withthis.postMessagecorrectly implements the message queueing behavior for lazy port initialization.Also applies to: 479-479, 487-487
460-466: Good compatibility improvement.Replacing
Promise.withResolvers()with explicit promise construction improves compatibility with environments that don't support the ES2023 feature. The pattern is correct and the variables are properly initialized before use.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
CodSpeed Performance ReportMerging #2040 will improve performance by 6.16%Comparing Summary
Benchmarks breakdown
Footnotes
|
React Example#6637 Bundle Size — 236.9KiB (0%).2d4361c(current) vs eaca723 main#6635(baseline) Bundle metrics
|
| Current #6637 |
Baseline #6635 |
|
|---|---|---|
 Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
162 |
162 |
| Duplicate Modules |
65 |
65 |
| Duplicate Code |
46.74% |
46.74% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #6637 |
Baseline #6635 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
91.14KiB |
91.14KiB |
Bundle analysis report Branch PupilTong:p/hw/rpc-add-message-q... Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#6797 Bundle Size — 376.4KiB (+0.13%).2d4361c(current) vs eaca723 main#6795(baseline) Bundle metrics
|
| Current #6797 |
Baseline #6795 |
|
|---|---|---|
 Initial JS |
149.87KiB(+0.17%) |
149.63KiB |
| Initial CSS |
32.4KiB |
32.4KiB |
 Cache Invalidation |
55.83% |
0% |
| Chunks |
8 |
8 |
| Assets |
8 |
8 |
| Modules |
232(+0.43%) |
231 |
| Duplicate Modules |
16 |
16 |
| Duplicate Code |
3.12%(+2.97%) |
3.03% |
| Packages |
4 |
4 |
| Duplicate Packages |
0 |
0 |
Bundle size by type 1 change 1 regression
| Current #6797 |
Baseline #6795 |
|
|---|---|---|
| JS |
247.01KiB (+0.2%) |
246.52KiB |
| Other |
96.98KiB |
96.98KiB |
| CSS |
32.4KiB |
32.4KiB |
Bundle analysis report Branch PupilTong:p/hw/rpc-add-message-q... 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/lynx-bundle-rslib-config@0.1.0 ### Minor Changes - Update external bundle minimum SDK version to 3.5. ([#2037](#2037)) ### Patch Changes - Fix `globDynamicComponentEntry is not defined` error when minify is enabled in external bundle consumer. ([#2058](#2058)) ## @lynx-js/web-elements@0.10.0 ### Minor Changes - chore: migrate all @lynx-js/web-elements-\* packages into one ([#2057](#2057)) ### Before ```js import "@lynx-js/web-elements-template"; import "@lynx-js/web-elements-compat/LinearContainer"; ``` ### After ```js import "@lynx-js/web-elements/html-templates"; import "@lynx-js/web-elements/compat/LinearContainer"; ``` ### Patch Changes - refactor: change code structure for improved readability and maintainability ([#2004](#2004)) - enable noUnusedLocals for web-elements - add source field for supporting @rsbuild/plugin-source-build This is a part of #1937 ## @lynx-js/react@0.115.2 ### Patch Changes - Fix `undefined factory (react:background)/./node_modules/.pnpm/@lynx-js+react...` error when loading a standalone lazy bundle after hydration. ([#2048](#2048)) - Partially fix "main-thread.js exception: TypeError: cannot read property '\_\_elements' of undefined" by recursively calling `snapshotDestroyList`. ([#2041](#2041)) - Fix a bug where React throws `CtxNotFound` error when lazy bundle resolves after unmount. ([#2003](#2003)) ## @lynx-js/rspeedy@0.12.4 ### Patch Changes - Updated dependencies \[]: - @lynx-js/web-rsbuild-server-middleware@0.19.3 ## @lynx-js/external-bundle-rsbuild-plugin@0.0.1 ### Patch Changes - Introduce `@lynx-js/external-bundle-rsbuild-plugin`. ([#2006](#2006)) ```ts // lynx.config.ts import { pluginExternalBundle } from "@lynx-js/external-bundle-rsbuild-plugin"; import { pluginReactLynx } from "@lynx-js/react-rsbuild-plugin"; export default { plugins: [ pluginReactLynx(), pluginExternalBundle({ externals: { lodash: { url: "http://lodash.lynx.bundle", background: { sectionPath: "background" }, mainThread: { sectionPath: "mainThread" }, }, }, }), ], }; ``` - Updated dependencies \[[`491c5ef`](491c5ef)]: - @lynx-js/externals-loading-webpack-plugin@0.0.2 ## @lynx-js/react-rsbuild-plugin@0.12.3 ### Patch Changes - expose LAYERS via `api.expose` for other rsbuild plugins. ([#2006](#2006)) - Updated dependencies \[[`cd89bf9`](cd89bf9)]: - @lynx-js/template-webpack-plugin@0.10.1 - @lynx-js/react-alias-rsbuild-plugin@0.12.3 - @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 ## @lynx-js/web-constants@0.19.3 ### Patch Changes - Updated dependencies \[[`986761d`](986761d)]: - @lynx-js/web-worker-rpc@0.19.3 ## @lynx-js/web-core@0.19.3 ### Patch Changes - Updated dependencies \[[`986761d`](986761d)]: - @lynx-js/web-worker-rpc@0.19.3 - @lynx-js/web-constants@0.19.3 - @lynx-js/web-worker-runtime@0.19.3 - @lynx-js/web-mainthread-apis@0.19.3 ## @lynx-js/web-mainthread-apis@0.19.3 ### Patch Changes - Updated dependencies \[]: - @lynx-js/web-constants@0.19.3 ## @lynx-js/web-worker-rpc@0.19.3 ### Patch Changes - feat: support lazy message port assigning in web-worker-rpc ([#2040](#2040)) ## @lynx-js/web-worker-runtime@0.19.3 ### Patch Changes - Updated dependencies \[[`986761d`](986761d)]: - @lynx-js/web-worker-rpc@0.19.3 - @lynx-js/web-constants@0.19.3 - @lynx-js/web-mainthread-apis@0.19.3 ## @lynx-js/externals-loading-webpack-plugin@0.0.2 ### Patch Changes - Export `ExternalValue` ts type. ([#2037](#2037)) ## @lynx-js/template-webpack-plugin@0.10.1 ### Patch Changes - fix: pass updated css from encodeData to resolvedEncodeOptions ([#2053](#2053)) Previously, the initial CSS was used in resolvedEncodeOptions instead of the potentially updated CSS from encodeData after the beforeEncode hook. This fix ensures resolvedEncodeOptions receives the latest CSS data. ## create-rspeedy@0.12.4 ## @lynx-js/react-alias-rsbuild-plugin@0.12.3 ## upgrade-rspeedy@0.12.4 ## @lynx-js/web-core-server@0.19.3 ## @lynx-js/web-rsbuild-server-middleware@0.19.3 Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
2 participants
This is a part of #1937
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.
Checklist