feat: remove multi-thread preheating and update related tests#1597
feat: remove multi-thread preheating and update related tests#1597PupilTong merged 1 commit intolynx-family:mainfrom
Conversation
🦋 Changeset detectedLatest commit: 07a1a1f 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 |
📝 WalkthroughWalkthroughRemoves multi-thread preheating so non-UI boot always creates a fresh main worker, adds webpack bundler/load hints for worker and main-thread API imports, moves some dynamic imports, updates worker-runtime startup, deletes preheat tests and demo apps, and adds a patch changeset. (50 words) Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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/Issue comments)Type Other keywords and placeholders
Status, 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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/web-platform/web-core/src/uiThread/bootWorkers.ts (1)
103-109: Bug: groupId 0 is treated as “no group”, breaking worker sharingYou check truthiness (
if (lynxGroupId)) here but use!== undefinedelsewhere (Lines 35-41, 47-58). IflynxGroupIdcan be 0, it will fail to reuse/share the background worker and inflate worker counts.Apply this diff:
- if (lynxGroupId) { + if (lynxGroupId !== undefined) { backgroundThreadWorker = contextIdToBackgroundWorker[lynxGroupId] ?? createWebWorker('lynx-bg'); contextIdToBackgroundWorker[lynxGroupId] = backgroundThreadWorker; } else { backgroundThreadWorker = createWebWorker('lynx-bg'); }Follow-up: Consider normalizing the group key to a string and using a Map<number, Worker> to avoid sparse arrays and truthiness pitfalls.
🧹 Nitpick comments (6)
.changeset/good-onions-go.md (2)
5-7: Tighten wording and fix grammar in the changeset entryClarifies the intent and removes awkward phrasing (“mts heating”). Also aligns with the PR (remove multi-thread preheat because default is all-on-ui).
Apply this diff:
-feat: remove multi-thread mts heating - -The default rendering mode is "all-on-ui". Therefore the preheating for "multi-thread" will be removed. +feat(web-core): remove multi-thread preheating + +The default rendering mode is "all-on-ui"; therefore, preheating for the multi-thread path is removed.
1-3: Confirm patch impact levelYou’ve marked @lynx-js/web-core as a patch. Behavior changes (no more preheat + fresh main worker creation) can affect startup characteristics. If there’s no consumer-facing API change and tests cover it, patch is fine. Otherwise consider a minor with a short migration note.
I can draft a short “Performance notes” section in the changeset if helpful.
packages/web-platform/web-core/src/uiThread/bootWorkers.ts (1)
125-128: Webpack/Rspack magic comments placement: move inside the URL() for reliable parsingMagic comments are typically attached to the module specifier within
new URL(...). Placing them as separate arguments tonew Worker(...)may not be recognized by all bundlers.Apply this diff:
- return new Worker( - /* webpackFetchPriority: "high" */ - /* webpackChunkName: "web-core-worker-runtime" */ - /* webpackPreload: true */ - new URL('@lynx-js/web-worker-runtime', import.meta.url), + return new Worker( + new URL( + /* webpackFetchPriority: "high" */ + /* webpackChunkName: "web-core-worker-runtime" */ + /* webpackPreload: true */ + '@lynx-js/web-worker-runtime', + import.meta.url, + ), { type: 'module', name, }, );If you need to support both Webpack and Rspack, please confirm these exact directives are recognized in your versions. I can help add a tiny build check in CI to validate chunk names and preload hints across both bundlers.
packages/web-platform/web-tests/tests/react.spec.ts (3)
720-724: Make worker-count assertion resilient with pollingWorker termination is async; a hard equality immediately after DOM removal can be flaky. Prefer expect.poll for stability.
Apply this diff:
- expect(message).toContain('fin'); - expect(page.workers().length).toStrictEqual(0); + expect(message).toContain('fin'); + await expect + .poll(() => page.workers().length, { timeout: 2000 }) + .toBe(0);
908-913: Stabilize worker-count upper bound with pollingThe count can briefly exceed/lag around teardown/startup; polling reduces flakes across browsers.
Apply this diff:
- expect(page.workers().length).toBeLessThanOrEqual(3); + await expect + .poll(() => page.workers().length, { timeout: 1500 }) + .toBeLessThanOrEqual(3);
914-929: Use polling for sequential worker-count assertions during releaseThe teardown between removes is async; make all three checks poll-based.
Apply this diff:
- expect(page.workers().length).toBeLessThanOrEqual(3); + await expect + .poll(() => page.workers().length, { timeout: 1500 }) + .toBeLessThanOrEqual(3); @@ - expect(page.workers().length).toBeLessThanOrEqual(2); + await expect + .poll(() => page.workers().length, { timeout: 1500 }) + .toBeLessThanOrEqual(2); @@ - expect(page.workers().length).toBeLessThanOrEqual(0); + await expect + .poll(() => page.workers().length, { timeout: 2000 }) + .toBeLessThanOrEqual(0);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
.changeset/good-onions-go.md(1 hunks)packages/web-platform/web-core/src/uiThread/bootWorkers.ts(2 hunks)packages/web-platform/web-tests/tests/react.spec.ts(2 hunks)packages/web-platform/web-tests/tests/react/api-preheat-at-least-one/index.jsx(0 hunks)packages/web-platform/web-tests/tests/react/api-preheat/index.jsx(0 hunks)
💤 Files with no reviewable changes (2)
- packages/web-platform/web-tests/tests/react/api-preheat-at-least-one/index.jsx
- packages/web-platform/web-tests/tests/react/api-preheat/index.jsx
🧰 Additional context used
🪛 LanguageTool
.changeset/good-onions-go.md
[grammar] ~7-~7: There might be a mistake here.
Context: ...ting for "multi-thread" will be removed.
(QB_NEW_EN)
⏰ 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). (4)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: build / Build (Windows)
- GitHub Check: test-rust / Test (Ubuntu)
- GitHub Check: CodeQL Analyze (javascript-typescript)
🔇 Additional comments (1)
packages/web-platform/web-core/src/uiThread/bootWorkers.ts (1)
27-31: LGTM: always creating a fresh main worker when not all-on-uiRemoving the preheated global and instantiating the main worker per call simplifies lifecycle and avoids stale state across views.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
PupilTong
force-pushed
the
p/hw/no-automatic-mts-worker
branch
from
bed1f90 to
2c26a1e
Compare
React Example#4547 Bundle Size — 237.06KiB (0%).07a1a1f(current) vs 1cbccc1 main#4534(baseline) Bundle metrics
|
| Current #4547 |
Baseline #4534 |
|
|---|---|---|
 Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
158 |
158 |
| Duplicate Modules |
64 |
64 |
| Duplicate Code |
45.83% |
45.83% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #4547 |
Baseline #4534 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
91.3KiB |
91.3KiB |
Bundle analysis report Branch PupilTong:p/hw/no-automatic-mts-... Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#4540 Bundle Size — 368.19KiB (+0.41%).07a1a1f(current) vs 1cbccc1 main#4527(baseline) Bundle metrics
|
| Current #4540 |
Baseline #4527 |
|
|---|---|---|
 Initial JS |
144.25KiB(+0.53%) |
143.49KiB |
| Initial CSS |
31.84KiB |
31.84KiB |
 Cache Invalidation |
51.51% |
0% |
| Chunks |
8(+14.29%) |
7 |
| Assets |
8(+14.29%) |
7 |
| Modules |
220(+3.77%) |
212 |
| Duplicate Modules |
17 |
17 |
| Duplicate Code |
3.85%(-1.03%) |
3.89% |
| Packages |
4 |
4 |
| Duplicate Packages |
0 |
0 |
Bundle size by type 1 change 1 regression
| Current #4540 |
Baseline #4527 |
|
|---|---|---|
| JS |
236.18KiB (+0.64%) |
234.68KiB |
| Other |
100.16KiB |
100.16KiB |
| CSS |
31.84KiB |
31.84KiB |
Bundle analysis report Branch PupilTong:p/hw/no-automatic-mts-... Project dashboard
Generated by RelativeCI Documentation Report issue
CodSpeed Performance ReportMerging #1597 will not alter performanceComparing Summary
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/web-platform/web-core/src/uiThread/bootWorkers.ts (1)
103-109: Bug: falsy check breaks group 0; use explicit undefined checkIf a valid lynxGroupId can be 0, the current truthy check skips reuse and spawns a new background worker each time. Elsewhere (Line 47) you already compare to
undefined.Apply this diff:
- if (lynxGroupId) { + if (lynxGroupId !== undefined) { backgroundThreadWorker = contextIdToBackgroundWorker[lynxGroupId] ?? createWebWorker('lynx-bg'); contextIdToBackgroundWorker[lynxGroupId] = backgroundThreadWorker; } else { backgroundThreadWorker = createWebWorker('lynx-bg'); }
🧹 Nitpick comments (2)
packages/web-platform/web-core/src/uiThread/bootWorkers.ts (2)
14-16: Optional: use Map instead of parallel arrays for worker reuse and counts
number[]and(Worker|undefined)[]indexed bylynxGroupIdwork, but Maps remove accidental sparse-array pitfalls and make intent clearer. This also avoids large sparse indices if group IDs are not small, contiguous integers.A minimal refactor outline (outside this diff’s scope):
const backgroundWorkerContextCount = new Map<number, number>(); const contextIdToBackgroundWorker = new Map<number, Worker>(); // increment const n = backgroundWorkerContextCount.get(lynxGroupId) ?? 0; backgroundWorkerContextCount.set(lynxGroupId, n + 1); // reuse const existing = contextIdToBackgroundWorker.get(lynxGroupId) ?? createWebWorker('lynx-bg'); contextIdToBackgroundWorker.set(lynxGroupId, existing); // termination const count = backgroundWorkerContextCount.get(lynxGroupId) ?? 0; if (count <= 1) { existing.terminate(); backgroundWorkerContextCount.delete(lynxGroupId); contextIdToBackgroundWorker.delete(lynxGroupId); } else { backgroundWorkerContextCount.set(lynxGroupId, count - 1); }Also applies to: 36-41, 49-58
123-135: Optional: add defensive handling for worker creation failuresIf worker instantiation throws (e.g., CSP prohibits workers or module workers are unsupported),
bootWorkerscurrently fails hard. Consider catching and either rethrow with a clearer message (includingname) or falling back toallOnUI.Example pattern:
function createWebWorker(name: string): Worker { try { return new Worker( new URL(/* webpackChunkName: "web-core-worker-runtime" */ '@lynx-js/web-worker-runtime', import.meta.url), { type: 'module', name }, ); } catch (e) { // Option A: rethrow with context throw new Error(`Failed to create "${name}" worker: ${String(e)}`); // Option B: feature-flagged fallback to UI path (if acceptable) } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
.changeset/good-onions-go.md(1 hunks)packages/web-platform/web-core/src/uiThread/bootWorkers.ts(2 hunks)packages/web-platform/web-tests/tests/react.spec.ts(2 hunks)packages/web-platform/web-tests/tests/react/api-preheat-at-least-one/index.jsx(0 hunks)packages/web-platform/web-tests/tests/react/api-preheat/index.jsx(0 hunks)
💤 Files with no reviewable changes (2)
- packages/web-platform/web-tests/tests/react/api-preheat-at-least-one/index.jsx
- packages/web-platform/web-tests/tests/react/api-preheat/index.jsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/web-platform/web-tests/tests/react.spec.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-22T09:23:07.797Z
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.
Applied to files:
.changeset/good-onions-go.md
🪛 LanguageTool
.changeset/good-onions-go.md
[grammar] ~7-~7: There might be a mistake here.
Context: ...ting for "multi-thread" will be removed.
(QB_NEW_EN)
⏰ 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). (4)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: build / Build (Windows)
- GitHub Check: test-rust / Test (Ubuntu)
- GitHub Check: CodeQL Analyze (javascript-typescript)
🔇 Additional comments (3)
.changeset/good-onions-go.md (1)
2-3: Confirm the bump level is appropriate (patch vs. minor)Removing automatic preheating is user-visible (initial MTS boot latency and worker count), even though the API surface is unchanged. Please confirm patch is intended per your semver policy for behavior changes; minor might be more appropriate.
If patch is correct per team conventions, no change needed. Otherwise, update the changeset frontmatter accordingly.
packages/web-platform/web-core/src/uiThread/bootWorkers.ts (2)
29-29: LGTM: new main worker per non-UI boot aligns with removing preheatingThis matches the PR intent to drop reuse/reheat and simplifies lifecycle.
125-129: Magic comments onnew URL(…, import.meta.url)in web workersWebpack
- webpackChunkName: Supported for
new Worker(new URL(..., import.meta.url))in Webpack 5. You can write(native worker support was introduced in Webpack 5.0.0; magic-commented chunk names date back to Webpack 2.6.0) (docs4dev.com, webpack.js.org)new Worker( /* webpackChunkName: "foo-worker" */ new URL('./worker.js', import.meta.url) );- webpackPrefetch, webpackPreload, webpackFetchPriority: Not yet supported on worker URLs in any stable Webpack release. Issue #19373 (Mar 31, 2025) tracks this, and PR #19377 (Apr 2, 2025) adds support on
mainfor an upcoming release (github.com)Rspack
- webpackIgnore: Supported inside
new URL()(e.g.new URL(/* webpackIgnore: true */ './style.css', import.meta.url)) since v1.0.0 (rspack.rs, webpack.js.org)- webpackChunkName: Not in v0.5.7; support was merged in PR #6019 on
main(to ship in the next patch release) (github.com)- webpackPrefetch, webpackPreload, webpackFetchPriority: Rspack’s parser options (
dynamicImportPrefetch,dynamicImportPreload,dynamicImportFetchPriority) apply to dynamicimport()only (all added in v1.0.0), not to worker URLs (rspack.org)Recommendation
- Move magic comments inside the
new URL(…)call to ensure they’re picked up- Use only the comments currently supported (
webpackChunkNameon Webpack 5,webpackIgnoreon Rspack)- Drop
webpackPrefetch+webpackPreload(unsupported in stable) andwebpackFetchPriority(pending) or wait for the upcoming releases that include PR #19377 (Webpack) and PR #6019 (Rspack) before re-introducing them.
PupilTong
force-pushed
the
p/hw/no-automatic-mts-worker
branch
from
2c26a1e to
b218a22
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/web-platform/web-core/src/uiThread/bootWorkers.ts (1)
103-109: Bug: truthy check breaks groupId 0; use explicit undefined checkIf lynxGroupId is 0, the current condition treats it as falsy and skips reuse, defeating grouping. Use an explicit undefined check.
- if (lynxGroupId) { + if (lynxGroupId !== undefined) { backgroundThreadWorker = contextIdToBackgroundWorker[lynxGroupId] ?? createWebWorker('lynx-bg'); contextIdToBackgroundWorker[lynxGroupId] = backgroundThreadWorker; } else { backgroundThreadWorker = createWebWorker('lynx-bg'); }
♻️ Duplicate comments (1)
.changeset/good-onions-go.md (1)
5-7: Clarify release note, drop “feat:” prefix, and fix terminologyChangesets typically use a plain summary line (not commit-style “feat: …”). Also, “mts heating” reads like a typo; prefer “multi-thread preheating” (or expand the acronym once) and tighten the sentence on Line 7.
Apply this diff:
-feat: remove multi-thread mts heating +Remove automatic multi-thread preheating @@ -The default rendering mode is "all-on-ui". Therefore the preheating for "multi-thread" will be removed. +Since the default rendering mode is all-on-ui, preheating for the multi-thread path has been removed.
🧹 Nitpick comments (5)
.changeset/good-onions-go.md (1)
2-3: Confirm bump type and whether a changeset is needed at allPer prior repo guidance, only user-visible changes should carry a changeset. Removing preheating likely alters runtime behavior (worker lifecycle/latency) and may be user-visible; if so, “patch” might be fine, but please confirm it isn’t “minor.” If this is strictly internal with no externally observable effect, consider dropping the changeset.
Would you like me to scan the codebase for public API/docs mentions of preheating to determine end-user impact and recommend the correct bump?
packages/web-platform/web-worker-runtime/src/index.ts (2)
13-23: Guard the dynamic import with try/catch to surface startup failures to the UIIf the dynamic import or startup throws, the worker fails silently. Wrap the branch with try/catch and post a minimal error back to the UI thread for visibility.
-globalThis.onmessage = async (ev) => { - const { mode, toPeerThread, toUIThread } = ev - .data as WorkerStartMessage; - if (mode === 'main') { - const { startMainThreadWorker } = await import( - /* webpackChunkName: "web-worker-runtime-main-thread" */ - /* webpackMode: "lazy-once" */ - /* webpackPreload: true */ - './mainThread/startMainThread.js' - ); - startMainThreadWorker(toUIThread, toPeerThread); - } else { - startBackgroundThread(toUIThread, toPeerThread); - } -}; +globalThis.onmessage = async (ev) => { + const { mode, toPeerThread, toUIThread } = ev.data as WorkerStartMessage; + try { + if (mode === 'main') { + const { startMainThreadWorker } = await import( + /* webpackChunkName: "web-worker-runtime-main-thread" */ + /* webpackMode: "lazy-once" */ + /* webpackPreload: true */ + './mainThread/startMainThread.js' + ); + await startMainThreadWorker(toUIThread, toPeerThread); + } else { + startBackgroundThread(toUIThread, toPeerThread); + } + } catch (err) { + // Best-effort surfacing to UI; avoid crashing silently. + try { toUIThread.postMessage({ type: '__worker_start_error__', error: String(err) }); } catch {} + // eslint-disable-next-line no-console + console.error('[worker-runtime] Failed to start worker', err); + } +};
13-23: Optional: lazy-load the background thread too to minimize initial worker bundleFor symmetry and potentially smaller initial payload, consider dynamically importing the background path as well.
- } else { - startBackgroundThread(toUIThread, toPeerThread); - } + } else { + const { startBackgroundThread } = await import( + /* webpackChunkName: "web-worker-runtime-background" */ + /* webpackMode: "lazy-once" */ + './backgroundThread/index.js' + ); + startBackgroundThread(toUIThread, toPeerThread); + }And remove the static import at the top:
-import { startBackgroundThread } from './backgroundThread/index.js'; +// (moved to dynamic import in onmessage)packages/web-platform/web-worker-runtime/src/mainThread/startMainThread.ts (1)
89-91: Nit: optional chaining is unnecessaryuiThreadRpc is defined; optional chaining is redundant here.
- uiThreadRpc?.registerHandler(updateI18nResourcesEndpoint, data => { + uiThreadRpc.registerHandler(updateI18nResourcesEndpoint, data => { i18nResources.setData(data as InitI18nResources); });packages/web-platform/web-core/src/uiThread/bootWorkers.ts (1)
123-131: Align magic comments for Web Worker URL creationThe webpack/Rspack chunk-name comment must be placed inside the
new URL(…)call to affect the emitted worker filename, and thewebpackPrefetch,webpackPreload, andwebpackFetchPriorityhints are no-ops fornew WorkerURL creation and can be removed.• File:
packages/web-platform/web-core/src/uiThread/bootWorkers.ts
• Lines: 123–131Suggested diff:
-function createWebWorker(name: string): Worker { - return new Worker( - /* webpackFetchPriority: "high" */ - /* webpackChunkName: "web-core-worker-runtime" */ - /* webpackPrefetch: true */ - /* webpackPreload: true */ - new URL('@lynx-js/web-worker-runtime', import.meta.url), - { - type: 'module', - name, - }, - ); -} +function createWebWorker(name: string): Worker { + return new Worker( + new URL( + /* webpackChunkName: "web-core-worker-runtime" */ + '@lynx-js/web-worker-runtime', + import.meta.url, + ), + { + type: 'module', + name, + }, + ); +}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
.changeset/good-onions-go.md(1 hunks)packages/web-platform/web-core/src/uiThread/bootWorkers.ts(2 hunks)packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts(1 hunks)packages/web-platform/web-tests/tests/react.spec.ts(2 hunks)packages/web-platform/web-tests/tests/react/api-preheat-at-least-one/index.jsx(0 hunks)packages/web-platform/web-tests/tests/react/api-preheat/index.jsx(0 hunks)packages/web-platform/web-worker-runtime/src/index.ts(1 hunks)packages/web-platform/web-worker-runtime/src/mainThread/startMainThread.ts(1 hunks)
💤 Files with no reviewable changes (2)
- packages/web-platform/web-tests/tests/react/api-preheat/index.jsx
- packages/web-platform/web-tests/tests/react/api-preheat-at-least-one/index.jsx
✅ Files skipped from review due to trivial changes (1)
- packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/web-platform/web-tests/tests/react.spec.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-22T09:23:07.797Z
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.
Applied to files:
.changeset/good-onions-go.md
🪛 LanguageTool
.changeset/good-onions-go.md
[grammar] ~7-~7: There might be a mistake here.
Context: ...ting for "multi-thread" will be removed.
(QB_NEW_EN)
⏰ 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). (5)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: build / Build (Windows)
- GitHub Check: test-rust / Test (Ubuntu)
- GitHub Check: CodeQL Analyze (javascript-typescript)
- GitHub Check: CodeQL Analyze (actions)
🔇 Additional comments (3)
packages/web-platform/web-worker-runtime/src/index.ts (1)
17-22: Webpack magic comments apply in Worker contexts
Dynamic import() calls inside Web Workers are processed at build time just like in the main thread. Both webpack 5 and Rspack recognize and honor
webpackPreload,webpackPrefetch, andwebpackFetchPrioritymagic comments for imports within a Worker. These directives control how chunks are fetched regardless of execution context (webpack.js.org, rspack.dev).When creating a Worker via
new Worker(new URL(..., import.meta.url)), thewebpackChunkNamecomment must be placed immediately before thenew URL(...)expression passed to the Worker constructor. For example:new Worker( /* webpackChunkName: "foo-worker" */ new URL('./worker.js', import.meta.url) );This ensures the Worker’s chunk is named
"foo-worker"instead of an autogenerated ID (webpack.js.org).With these placements, your magic comments will be recognized and effective for both dynamic imports inside Workers and for chunk naming of Worker scripts.
packages/web-platform/web-worker-runtime/src/mainThread/startMainThread.ts (1)
26-33: Please verify consumer bundler & browser support for top-level await in Worker modulesThis package itself doesn’t ship a webpack/rspack config or explicit browserslist, so you’ll need to confirm downstream settings:
• Ensure your application’s bundler enables top-level await (e.g. in webpack:
experiments.toplevelAwait: true, or in Rspack/Esm output) and emits ESM modules.
• Verify your base tsconfig (extended by this package) uses"module": "esnext"(or equivalent) and a moderntargetsupporting async/await.
• Check your target browsers support top-level await inside Web Worker modules (Safari 14+, Chrome 89+, Firefox 89+).If you must support older environments, switch to on-demand loading inside the worker function. For example:
-const { prepareMainThreadAPIs } = await import( - /* webpackChunkName: "web-core-main-thread-apis" */ - /* webpackMode: "lazy-once" */ - /* webpackPreload: true */ - /* webpackPrefetch: true */ - /* webpackFetchPriority: "high" */ - '@lynx-js/web-mainthread-apis' -); +const loadMainThreadAPIs = () => + import( + /* webpackChunkName: "web-core-main-thread-apis" */ + /* webpackMode: "lazy-once" */ + /* webpackPreload: true */ + /* webpackPrefetch: true */ + /* webpackFetchPriority: "high" */ + '@lynx-js/web-mainthread-apis' + ); … - const { startMainThread } = prepareMainThreadAPIs( + const { prepareMainThreadAPIs } = await loadMainThreadAPIs(); + const { startMainThread } = prepareMainThreadAPIs(packages/web-platform/web-core/src/uiThread/bootWorkers.ts (1)
29-30: LGTM: fresh main worker aligns with preheat removalAlways creating a new main worker in non-UI mode matches the goal to drop preheating and avoids reuse-related edge cases.
PupilTong
force-pushed
the
p/hw/no-automatic-mts-worker
branch
from
b218a22 to
07a1a1f
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.changeset/good-onions-go.md (1)
8-8: Remove stray character that breaks changeset parsingThere’s an orphaned "8" at EOF. This will invalidate the Markdown/frontmatter parse and can block release tooling.
Apply this diff to remove it:
-8
♻️ Duplicate comments (2)
.changeset/good-onions-go.md (2)
5-5: Fix terminology: “MTS preheating” and drop unclear wordingHeadline reads awkwardly (“multi-thread mts heating”). Prefer clear, consistent terms and use “preheating”.
-feat: remove multi-thread mts heating +Remove automatic MTS worker preheating
7-7: Grammar/clarity: concise past-tense note without quotesTighten phrasing and remove unnecessary quotes around adjectives.
-The default rendering mode is "all-on-ui". Therefore the preheating for "multi-thread" will be removed. +Since the default rendering mode is all-on-ui, preheating for the multi-thread path has been removed.
🧹 Nitpick comments (2)
.changeset/good-onions-go.md (2)
2-2: Confirm bump level: feature-level behavior change likely warrants “minor”, not “patch”Removing worker preheating changes runtime behavior for consumers of @lynx-js/web-core. Per semver this is a feature (or at least observable change), so consider a minor bump.
-"@lynx-js/web-core": patch +"@lynx-js/web-core": minorI used the team learning that changesets are for user-visible changes; this seems user-visible.
5-7: Optional: Expand note with concrete impact and upstream refsA brief, concrete summary helps downstream readers scanning release notes.
-feat: remove multi-thread mts heating - -The default rendering mode is "all-on-ui". Therefore the preheating for "multi-thread" will be removed. +Remove automatic MTS worker preheating + +- Non-UI boot now always creates a fresh main worker (previous preheated worker reuse removed). +- Matches upstream direction (see webpack#19695 and rspack#11476). +- No public API changes expected; startup characteristics may differ.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
.changeset/good-onions-go.md(1 hunks)packages/web-platform/web-core/src/uiThread/bootWorkers.ts(2 hunks)packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts(1 hunks)packages/web-platform/web-tests/tests/react.spec.ts(2 hunks)packages/web-platform/web-tests/tests/react/api-preheat-at-least-one/index.jsx(0 hunks)packages/web-platform/web-tests/tests/react/api-preheat/index.jsx(0 hunks)packages/web-platform/web-worker-runtime/src/index.ts(1 hunks)packages/web-platform/web-worker-runtime/src/mainThread/startMainThread.ts(1 hunks)
💤 Files with no reviewable changes (2)
- packages/web-platform/web-tests/tests/react/api-preheat/index.jsx
- packages/web-platform/web-tests/tests/react/api-preheat-at-least-one/index.jsx
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/web-platform/web-worker-runtime/src/index.ts
- packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts
- packages/web-platform/web-tests/tests/react.spec.ts
- packages/web-platform/web-core/src/uiThread/bootWorkers.ts
- packages/web-platform/web-worker-runtime/src/mainThread/startMainThread.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-22T09:23:07.797Z
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.
Applied to files:
.changeset/good-onions-go.md
🪛 LanguageTool
.changeset/good-onions-go.md
[grammar] ~7-~7: There might be a mistake here.
Context: ...ting for "multi-thread" will be removed.
(QB_NEW_EN)
⏰ 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). (6)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: build / Build (Windows)
- GitHub Check: test-rust / Test (Ubuntu)
- GitHub Check: zizmor
- GitHub Check: CodeQL Analyze (actions)
- GitHub Check: CodeQL Analyze (javascript-typescript)
🔇 Additional comments (1)
.changeset/good-onions-go.md (1)
1-7: Add missing changeset entries for template packagesThe PR modifies these published packages but no changeset entries exist for them:
rspeedy-react-js
(packages/rspeedy/create-rspeedy/template-react-js & template-react-vitest-rltl-js)rspeedy-react-ts
(packages/rspeedy/create-rspeedy/template-react-ts & template-react-vitest-rltl-ts)All other changed published packages already have entries:
@lynx-js/react-alias-rsbuild-plugin@lynx-js/web-core@lynx-js/web-worker-runtimePlease add new
.changeset/*.mdfiles to bumprspeedy-react-jsandrspeedy-react-tsso they’ll be versioned and released.⛔ Skipped due to learnings
Learnt from: colinaaa PR: lynx-family/lynx-stack#1453 File: vitest.config.ts:49-61 Timestamp: 2025-08-06T13:28:57.182Z Learning: In the lynx-family/lynx-stack repository, the file `packages/rspeedy/create-rspeedy/template-react-vitest-rltl-js/vitest.config.js` is a template file for scaffolding new Rspeedy projects, not a test configuration that should be included in the main vitest projects array.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: colinaaa PR: lynx-family/lynx-stack#1558 File: .changeset/solid-squids-fall.md:2-2 Timestamp: 2025-08-19T11:25:36.127Z Learning: In the lynx-family/lynx-stack repository, changesets should use the exact package name from package.json#name, not generic or unscoped names. Each package has its own specific scoped name (e.g., "lynx-js/react-transform" for packages/react/transform).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.
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.0 ### Minor Changes - Deprecate `source.alias`, use `resolve.alias` instead. ([#1610](#1610)) Note that `resolve.alias` has **lower** priority than the deprecated `source.alias`. - Bump Rsbuild v1.5.0 with Rspack v1.5.0. ([#1591](#1591)) - **BREAKING CHANGE**: Remove the `./register` exports from `@lynx-js/rspeedy`. ([#1547](#1547)) This should not affect most users. ### Patch Changes - Support `resolve.alias`. ([#1610](#1610)) - Support `rspeedy build --watch` ([#1579](#1579)) - Updated dependencies \[[`d7d0b9b`](d7d0b9b), [`1952fc1`](1952fc1)]: - @lynx-js/cache-events-webpack-plugin@0.0.2 - @lynx-js/chunk-loading-webpack-plugin@0.3.2 ## @lynx-js/web-constants@0.16.0 ### Minor Changes - refactor: provide the mts a real globalThis ([#1589](#1589)) Before this change, We create a function wrapper and a fake globalThis for Javascript code. This caused some issues. After this change, we will create an iframe for createing an isolated Javascript context. This means the globalThis will be the real one. ### Patch Changes - refactor: add `:not([l-e-name])` at the end of selector for lazy component ([#1622](#1622)) - fix: the SystemInfo in bts should be assigned to the globalThis ([#1599](#1599)) - Updated dependencies \[[`c1f8715`](c1f8715)]: - @lynx-js/web-worker-rpc@0.16.0 ## @lynx-js/web-core@0.16.0 ### Minor Changes - refactor: provide the mts a real globalThis ([#1589](#1589)) Before this change, We create a function wrapper and a fake globalThis for Javascript code. This caused some issues. After this change, we will create an iframe for createing an isolated Javascript context. This means the globalThis will be the real one. ### Patch Changes - refactor: add `:not([l-e-name])` at the end of selector for lazy component ([#1622](#1622)) - feat: remove multi-thread mts heating ([#1597](#1597)) The default rendering mode is "all-on-ui". Therefore the preheating for "multi-thread" will be removed. - fix: the SystemInfo in bts should be assigned to the globalThis ([#1599](#1599)) - Updated dependencies \[[`1a32dd8`](1a32dd8), [`bb53d9a`](bb53d9a), [`1a32dd8`](1a32dd8), [`c1f8715`](c1f8715)]: - @lynx-js/web-mainthread-apis@0.16.0 - @lynx-js/web-constants@0.16.0 - @lynx-js/web-worker-runtime@0.16.0 - @lynx-js/offscreen-document@0.1.4 - @lynx-js/web-worker-rpc@0.16.0 ## @lynx-js/web-core-server@0.16.0 ### Minor Changes - refactor: provide the mts a real globalThis ([#1589](#1589)) Before this change, We create a function wrapper and a fake globalThis for Javascript code. This caused some issues. After this change, we will create an iframe for createing an isolated Javascript context. This means the globalThis will be the real one. ## @lynx-js/web-mainthread-apis@0.16.0 ### Minor Changes - refactor: provide the mts a real globalThis ([#1589](#1589)) Before this change, We create a function wrapper and a fake globalThis for Javascript code. This caused some issues. After this change, we will create an iframe for createing an isolated Javascript context. This means the globalThis will be the real one. ### Patch Changes - refactor: add `:not([l-e-name])` at the end of selector for lazy component ([#1622](#1622)) - fix: the SystemInfo in bts should be assigned to the globalThis ([#1599](#1599)) - Updated dependencies \[[`1a32dd8`](1a32dd8), [`bb53d9a`](bb53d9a), [`c1f8715`](c1f8715)]: - @lynx-js/web-constants@0.16.0 - @lynx-js/web-style-transformer@0.16.0 ## @lynx-js/web-worker-rpc@0.16.0 ### Minor Changes - refactor: provide the mts a real globalThis ([#1589](#1589)) Before this change, We create a function wrapper and a fake globalThis for Javascript code. This caused some issues. After this change, we will create an iframe for createing an isolated Javascript context. This means the globalThis will be the real one. ## @lynx-js/web-worker-runtime@0.16.0 ### Minor Changes - refactor: provide the mts a real globalThis ([#1589](#1589)) Before this change, We create a function wrapper and a fake globalThis for Javascript code. This caused some issues. After this change, we will create an iframe for createing an isolated Javascript context. This means the globalThis will be the real one. ### Patch Changes - fix: the SystemInfo in bts should be assigned to the globalThis ([#1599](#1599)) - Updated dependencies \[[`1a32dd8`](1a32dd8), [`bb53d9a`](bb53d9a), [`1a32dd8`](1a32dd8), [`c1f8715`](c1f8715)]: - @lynx-js/web-mainthread-apis@0.16.0 - @lynx-js/web-constants@0.16.0 - @lynx-js/offscreen-document@0.1.4 - @lynx-js/web-worker-rpc@0.16.0 ## @lynx-js/react@0.112.5 ### Patch Changes - Remove the "key is not on root element of snapshot" warning. ([#1558](#1558)) ## create-rspeedy@0.11.0 ### Patch Changes - Use TypeScript [Project References](https://www.typescriptlang.org/docs/handbook/project-references.html) in templates. ([#1612](#1612)) ## @lynx-js/offscreen-document@0.1.4 ### Patch Changes - remove innerHTML, replace it by textContent ([#1622](#1622)) ## @lynx-js/cache-events-webpack-plugin@0.0.2 ### Patch Changes - Fix that `__webpack_require__.lynx_ce` is incorrectly injected when lazy bundle is enabled. ([#1616](#1616)) ## @lynx-js/chunk-loading-webpack-plugin@0.3.2 ### Patch Changes - Should not load initial CSS chunks. ([#1601](#1601)) ## upgrade-rspeedy@0.11.0 ## @lynx-js/web-style-transformer@0.16.0 Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
2 participants
webpack/webpack#19695
web-infra-dev/rspack#11476
Summary by CodeRabbit
Checklist