feat(react): fetchBundle-based lazy bundle loader#2584
Draft
upupming wants to merge 47 commits into
Draft
Conversation
…ode attr
Recognize `with: { mode: ... }` on dynamic import and rewrite as
`withLazyBundleMode(mode, () => import(...))`. The runtime symbol is
imported from `@lynx-js/react/internal` only when the wrapper is
actually emitted, gated via Lazy::get to avoid spurious imports.
Introduce `withLazyBundleMode(mode, factory)` to temporarily set the lazy bundle import mode via a Symbol on `lynx`, restoring the prior value after the factory returns. Wire `__dynamicImport` to read `mode` from `with` (matching what the SWC plugin forwards) and wrap component-type loads accordingly. Re-export from `react/internal` and the lazy proxy so consumers can resolve it.
Introduce a `__LAZY_BUNDLE_FETCHER__` define gated by the `REACT_LAZY_BUNDLE_FETCHER` env var (default `'FetchBundle'`, overridable to `'QueryComponent'` for the legacy path) and dispatch `loadLazyBundle` to one of two implementations at module init. The FetchBundle implementation uses `lynx.fetchBundle` + `lynx.loadScript`, with the sync/async behavior driven by the module-local `lazyBundleMode` set via `withLazyBundleMode`. Pin `@lynx-js/types` to a vendored 3.10.0 tarball — that release adds the `fetchBundle` / `loadScript` declarations the new code type-checks against. Existing tests stub the global to `'QueryComponent'` so they still exercise the legacy path.
…bundles
Restructure lazy-bundle binary output when the FetchBundle fetcher is
selected (default; fall back via REACT_LAZY_BUNDLE_FETCHER=QueryComponent):
- customSections['main-thread'] ← MTS payload, encoding: 'JsBytecode'
(compiled to QuickJS bytecode by the
lepus encoder; first-screen evaluates
bytecode directly)
- customSections['background'] ← first real BTS chunk (entry)
- customSections['CSS'] ← parsed style ruleList, encoding: 'CSS'
- manifest ← remaining BTS chunks (vendor / async),
served via lynx.requireModuleAsync
- lepusCode = undefined ← MTS moved to customSection
- css.cssMap = {} ← CSS moved to customSection
The legacy /app-service.js loader stub is dropped — under FetchBundle the
platform evaluates the entry chunk directly via loadScript('background'),
so the stub is dead weight.
React runtime side: lazy-bundle.ts MTS path now also calls
__LoadStyleSheet('CSS', url) + __AdoptStyleSheet + __FlushElementTree
after loadScript so the lazy bundle's styles are adopted at first-screen.
Misc:
- WebEncodePlugin: emit empty {} for missing lepusCode (the web encoder's
encodeStringMap calls Object.entries which throws on undefined).
- types.d.ts: declare __LoadStyleSheet / __AdoptStyleSheet (since 3.7).
- rstest.config.ts: force REACT_LAZY_BUNDLE_FETCHER=QueryComponent inside
the test process so existing fixtures (which assert legacy lepusCode +
manifest shape) keep passing. FetchBundle-specific tests should be
written separately and override this.
🦋 Changeset detectedLatest commit: 9ea2105 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 |
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR implements a FetchBundle-based lazy-bundle loader with sync/async mode control, replacing the legacy QueryComponent fetcher. It adds runtime loading implementations with first-screen blocking control, build-time output splitting into main-thread/background/CSS sections, a lifecycle handler for main-thread preloading, SWC transform support for mode annotations, comprehensive tests, and example applications demonstrating the feature. ChangesFetchBundle Lazy-Bundle Implementation
🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
…ce.yaml `pnpm.overrides` in `package.json` and the `overrides:` block in `pnpm-workspace.yaml` are mutually exclusive — when both exist, pnpm honors the `package.json` one and silently ignores the workspace one, which here pins the cataloged `@rspack/core` version. The clash showed up as `@rspack/core 1.7.9 vs 1.7.11` type-mismatch errors after `pnpm install`. Move the local `@lynx-js/types: file:./vendor/...tgz` override into `pnpm-workspace.yaml` so all overrides live in one place and the cataloged `@rspack/core` pin keeps working.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
… >= 3.8 Default __LAZY_BUNDLE_FETCHER__ is now derived from the host's engineVersion: >= 3.8 picks 'FetchBundle', otherwise 'QueryComponent'. The REACT_LAZY_BUNDLE_FETCHER env var still forces a specific fetcher, but forcing 'FetchBundle' on engineVersion < 3.8 now throws a build error (the platform doesn't expose lynx.fetchBundle on those hosts). Plumbs engineVersion through pluginReactLynx -> entry.ts -> ReactWebpackPlugin.
CI failures from the previous push: - code-style-check (api-extractor): the new 'engineVersion' field on ReactWebpackPluginOptions wasn't reflected in etc/*.api.md. - sherif (pnpm dedupe --check): @rspack/core had 2.0.0-beta.X variants mixed with the cataloged 1.7.9. The repl build failure was a knock-on effect of that resolution drift. Re-running api-extractor + pnpm dedupe fixes both.
CI's clippy runs with RUSTFLAGS=-D warnings, so the unused 'i' from 'attrs.iter().enumerate()' was rejected. The index isn't read inside the loop — drop the enumerate.
The local task config dropped the root's '^build' when overriding
'dependsOn' to add 'build:wasm'. As a result web-core's build can race
ahead of its workspace deps' builds — on a fresh CI checkout, when
@lynx-js/web-elements / @lynx-js/web-worker-rpc don't yet have a
'dist/index.js', web-core's rsbuild + rslib produces a partial dist
that downstream consumers (e.g. repl) then fail to bundle ('Module not
found: Can't resolve @lynx-js/web-worker-rpc' / missing
'dist/client/background/index.js').
Adding '^build' restores the workspace-dep ordering. The hash change
also busts the existing poisoned turbo cache.
3 tasks
Merging this PR will degrade performance by 26.77%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ | 002-hello-reactLynx-destroyBackground |
670.3 µs | 915.4 µs | -26.77% |
| ⚡ | 008-many-use-state-destroyBackground |
9.5 ms | 8 ms | +19.25% |
Comparing feat/lazy-bundle-with-fetchBundle (9ea2105) with main (705a3a3)
Footnotes
-
26 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
React External#1222 Bundle Size — 693.66KiB (+0.09%).9ea2105(current) vs 705a3a3 main#1216(baseline) Bundle metrics
|
| Current #1222 |
Baseline #1216 |
|
|---|---|---|
 Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
 Cache Invalidation |
40.81% |
0% |
| Chunks |
0 |
0 |
| Assets |
3 |
3 |
| Modules |
17 |
17 |
| Duplicate Modules |
5 |
5 |
| Duplicate Code |
8.59% |
8.59% |
| Packages |
0 |
0 |
| Duplicate Packages |
0 |
0 |
Bundle size by type 1 change 
1 regression
| Current #1222 |
Baseline #1216 |
|
|---|---|---|
| Other |
693.66KiB (+0.09%) |
693.04KiB |
Bundle analysis report Branch feat/lazy-bundle-with-fetchBundl... Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#9682 Bundle Size — 901.38KiB (0%).9ea2105(current) vs 705a3a3 main#9676(baseline) Bundle metrics
|
| Current #9682 |
Baseline #9676 |
|
|---|---|---|
| Initial JS |
45.06KiB |
45.06KiB |
| Initial CSS |
2.22KiB |
2.22KiB |
| Cache Invalidation |
0% |
0% |
| Chunks |
9 |
9 |
| Assets |
11 |
11 |
| Modules |
229(+0.44%) |
228 |
| Duplicate Modules |
11 |
11 |
| Duplicate Code |
27.22% |
27.22% |
| Packages |
10 |
10 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #9682 |
Baseline #9676 |
|
|---|---|---|
| JS |
497.1KiB |
497.1KiB |
| Other |
402.06KiB |
402.06KiB |
| CSS |
2.22KiB |
2.22KiB |
Bundle analysis report Branch feat/lazy-bundle-with-fetchBundl... Project dashboard
Generated by RelativeCI Documentation Report issue
React MTF Example#1240 Bundle Size — 207.47KiB (~+0.01%).9ea2105(current) vs 705a3a3 main#1234(baseline) Bundle metrics
|
| Current #1240 |
Baseline #1234 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
46.38% |
0% |
| Chunks |
0 |
0 |
| Assets |
3 |
3 |
| Modules |
193(+0.52%) |
192 |
| Duplicate Modules |
77 |
77 |
| Duplicate Code |
44.31%(-0.16%) |
44.38% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type 1 change 1 regression
| Current #1240 |
Baseline #1234 |
|
|---|---|---|
| IMG |
111.23KiB |
111.23KiB |
| Other |
96.24KiB (+0.01%) |
96.23KiB |
Bundle analysis report Branch feat/lazy-bundle-with-fetchBundl... Project dashboard
Generated by RelativeCI Documentation Report issue
React Example with Element Template#375 Bundle Size — 197.79KiB (0%).9ea2105(current) vs 705a3a3 main#369(baseline) Bundle metrics
|
| Current #375 |
Baseline #369 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
79(-1.25%) |
80 |
| Duplicate Modules |
23 |
23 |
| Duplicate Code |
40.33%(+0.07%) |
40.3% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #375 |
Baseline #369 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
52.03KiB |
52.03KiB |
Bundle analysis report Branch feat/lazy-bundle-with-fetchBundl... Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#8108 Bundle Size — 236.52KiB (~+0.01%).9ea2105(current) vs 705a3a3 main#8102(baseline) Bundle metrics
|
| Current #8108 |
Baseline #8102 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
38.37% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
198(+0.51%) |
197 |
| Duplicate Modules |
80 |
80 |
| Duplicate Code |
44.8%(-0.16%) |
44.87% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type 1 change 1 regression
| Current #8108 |
Baseline #8102 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
90.76KiB (+0.01%) |
90.75KiB |
Bundle analysis report Branch feat/lazy-bundle-with-fetchBundl... Project dashboard
Generated by RelativeCI Documentation Report issue
PupilTong
reviewed
May 8, 2026
…t options
Picks up `loadScript<T>(sectionName, options?: { bundleName?: string })`
from template-assembler, matching how the FetchBundle path actually calls it.
upupming
added a commit
that referenced
this pull request
May 11, 2026
## Summary `packages/web-platform/web-core`'s turbo `build` task only depended on its local `build:wasm`, missing `^build`. This caused turbo to start `web-core`'s TypeScript build before its workspace deps (`@lynx-js/web-elements`, `@lynx-js/web-worker-rpc`, `@lynx-js/css-serializer`, ...) had finished building, producing partial `dist/` output and intermittent CI failures (e.g. `repl` failing to resolve modules from `web-core/dist`). Adding `^build` makes turbo wait for upstream workspace builds, matching the convention used by every other package in the repo. This was extracted from #2584 where the same fix unblocked Build (Ubuntu). ## Test plan - [x] Verified Build (Ubuntu) passes with this change applied on top of #2584 - [ ] Build (Ubuntu) passes on this PR - [ ] Build (Windows) passes on this PR <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated build task configuration to improve build process sequencing. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Hoist `resolveLazyBundleFetcher` into `pluginReactLynx` so the runtime and the build artifact agree on FetchBundle vs QueryComponent. Both ReactWebpackPlugin and LynxTemplatePlugin now take the resolved `lazyBundleFetcher` as a plugin option instead of each computing it (or defaulting to FetchBundle) on its own. Without this, a project on the default `targetSdkVersion` (3.2) would ship a customSections-based lazy bundle that the QueryComponent runtime path does not understand, and `react-lazy-bundle` crashed at load time.
…undle
The FetchBundle runtime evaluates `customSections['main-thread']` directly,
so the wrapper has to execute itself and bind `globDynamicComponentEntry`
locally. Switch to a self-invoked IIFE with the literal `__Card__` baked in
when `lazyBundleFetcher === 'FetchBundle'`. The QueryComponent path keeps
the existing `(function (globDynamicComponentEntry) { ... })` shape since
its caller still passes the entry name in.
…ples
Both lazy-bundle examples now have `dev:fetchbundle`, `build:fetchbundle`,
`preview:fetchbundle` scripts that flip the resolver to FetchBundle by
bumping engineVersion to 3.8 (gated on `LAZY_BUNDLE_FETCHBUNDLE=1` env).
Lazy imports are switched to `with: { mode: 'sync' }` so the demos
exercise the sync first-screen path. The runtime also now logs the raw
response on FetchBundle load failure to make field debugging easier.
upupming
force-pushed
the
feat/lazy-bundle-with-fetchBundle
branch
from
fd6bb6f to
2519670
Compare
Contributor
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
packages/webpack/template-webpack-plugin/src/WebEncodePlugin.ts (1)
98-104: 💤 Low valueNon-null assertion on
rootwhen type allowsundefined.
EncodeOptions.lepusCode.rootis declared asstring | undefined. Theroot!assertion will producelepusCode.root = undefinedat runtime whenlepusCodeis provided butrootis absent (e.g., an upstream hook that supplies onlylepusChunk), and the downstream encoder may not be resilient to that. Either guard with a conditional spread or treatlepusCodewith norootthe same as the missing-lepusCodecase.♻️ Suggested defensive shape
- lepusCode: encodeOptions.lepusCode - ? { - // flatten the lepusCode to a single object - ...encodeOptions.lepusCode.lepusChunk, - root: encodeOptions.lepusCode.root!, - } - : {}, + lepusCode: encodeOptions.lepusCode + ? { + ...encodeOptions.lepusCode.lepusChunk, + ...(encodeOptions.lepusCode.root !== undefined + ? { root: encodeOptions.lepusCode.root } + : {}), + } + : {},🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/webpack/template-webpack-plugin/src/WebEncodePlugin.ts` around lines 98 - 104, The code uses a non-null assertion on EncodeOptions.lepusCode.root (encodeOptions.lepusCode.root!) which can produce an undefined root at runtime; update the lepusCode construction in WebEncodePlugin (the block that spreads encodeOptions.lepusCode.lepusChunk into lepusCode) to avoid the `!` by only including root when it's defined (e.g., conditionally spread or add root: encodeOptions.lepusCode.root only if not undefined), or treat a missing lepusCode.root the same as the absent lepusCode case and return an empty object when root is absent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/react-lazy-bundle/lynx.config.js`:
- Around line 10-22: detectLanHost() is being called at module top-level which
causes errors during build; change the code to compute assetPrefix lazily and
allow an explicit override: read LYNX_LAZY_BUNDLE_HOST first and use it if set,
otherwise call detectLanHost() only when needed (e.g., inside dev/preview
startup logic) so builds that don't need LAN resolution won't run the scan; also
robustly parse and validate the port (use a helper like parseInt with a fallback
default 54173, reject empty strings/NaN and clamp to valid port range 1–65535)
and construct assetPrefix only after host and port are validated.
In `@packages/react/runtime/src/snapshot/lynx/lazy-bundle.ts`:
- Around line 165-194: The MT sync path currently calls
__LoadStyleSheet(SECTION_CSS, response.url) and __AdoptStyleSheet(styleSheet)
without protection, so any throw there escapes instead of returning the
never-resolving Promise like other failures; wrap the CSS load/adopt steps in a
try/catch around the calls to __LoadStyleSheet and __AdoptStyleSheet (after
lynx.loadScript/response handling) and on any error return new Promise(() => {})
to match the existing failure behavior used for lynx.fetchBundle and
lynx.loadScript in loadLazyBundle.
In `@packages/react/runtime/src/snapshot/lynx/prepareLazyBundleMTS.ts`:
- Around line 8-47: The current prepareLazyBundleMTS caches the URL before any
work completes, preventing retries on fetch/load failures; remove the early
cache.add(url) and instead call cache.add(url) only after the bundle has been
successfully loaded and applied (i.e., after lynx.loadScript succeeds and after
__AdoptStyleSheet has been called for the CSS), leaving all catch/early-return
paths without caching so failures remain retryable; update references in
prepareLazyBundleMTS around lynx.fetchBundle, lynx.loadScript,
processEvalResult, __LoadStyleSheet and __AdoptStyleSheet accordingly.
In `@packages/rspeedy/plugin-react/src/resolveLazyBundleFetcher.ts`:
- Around line 17-24: The error message constructed for the
REACT_LAZY_BUNDLE_FETCHER check references the wrong config key
("targetSdkVersion"); update the string to mention engineVersion instead. Locate
where FETCH_BUNDLE_MIN_ENGINE_VERSION and REACT_LAZY_BUNDLE_FETCHER are used and
change the phrases on lines that currently say "requires targetSdkVersion" and
"Either bump 'targetSdkVersion' to" so they refer to "engineVersion" (or
"'engineVersion'") and instruct users to bump/unset engineVersion accordingly;
keep the rest of the message (including engineVersion ? `'${engineVersion}'` :
'<unset>') unchanged.
In `@packages/webpack/react-refresh-webpack-plugin/package.json`:
- Line 52: The peer dependency entry "@lynx-js/react-webpack-plugin": "^0.3.0 ||
^0.4.0 || ^0.5.0 || ^0.6.0 || ^0.7.0 || ^0.8.0 || ^0.9.0 || ^0.10.0" references
a non-existent 0.10.0; remove the "|| ^0.10.0" segment from that dependency
string in package.json (or alternatively, if you intend to publish 0.10.0 now,
bump the peer package to include a real 0.10.0 release) so the peerDependencies
list only contains valid released versions.
In `@packages/webpack/react-webpack-plugin/test/lazy-bundle-fetcher.test.ts`:
- Around line 67-73: The Promise created around compiler.run may return early on
error paths without calling compiler.close(), leaking resources; update the
compiler.run callback handling in the test so compiler.close() is always invoked
before resolving or rejecting the Promise (for example call compiler.close(...)
in both error and success branches or move resolve/reject into a try/finally
that calls compiler.close()), referencing the compiler.run callback and
compiler.close to ensure close executes on every callback path.
In `@packages/webpack/template-webpack-plugin/src/LynxTemplatePlugin.ts`:
- Around line 1024-1062: In buildLazyBundleFetchBundleSections: don’t pick the
background entry by the first Object.entries(manifest) item—identify the real BG
entry using explicit metadata (e.g., asset/chunk flags or chunk-group markers
from assetsInfoByGroups/backgroundThread or fallback to the last asset) and use
that asset to set entryChunk and SECTION_BACKGROUND instead of the current
positional logic that sets entryChunk from manifest iteration; and replace the
single-first-css logic (cssAssets[0] + SECTION_CSS) with merging all cssAssets
through cssChunksToMap (or mapping each chunk and concatenating their ruleList
results) so every CSS chunk contributes to the SECTION_CSS ruleList rather than
dropping additional cssAssets.
---
Nitpick comments:
In `@packages/webpack/template-webpack-plugin/src/WebEncodePlugin.ts`:
- Around line 98-104: The code uses a non-null assertion on
EncodeOptions.lepusCode.root (encodeOptions.lepusCode.root!) which can produce
an undefined root at runtime; update the lepusCode construction in
WebEncodePlugin (the block that spreads encodeOptions.lepusCode.lepusChunk into
lepusCode) to avoid the `!` by only including root when it's defined (e.g.,
conditionally spread or add root: encodeOptions.lepusCode.root only if not
undefined), or treat a missing lepusCode.root the same as the absent lepusCode
case and return an empty object when root is absent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1392ece8-db3e-44bf-814f-63da26124e88
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (54)
.changeset/lazy-bundle-fetch-bundle.mdexamples/react-lazy-bundle-standalone/lynx.config.consumer.jsexamples/react-lazy-bundle-standalone/lynx.config.producer.jsexamples/react-lazy-bundle-standalone/package.jsonexamples/react-lazy-bundle-standalone/src/App.tsxexamples/react-lazy-bundle-standalone/src/LazyComponentAsync.cssexamples/react-lazy-bundle-standalone/src/LazyComponentAsync.tsxexamples/react-lazy-bundle-standalone/src/LazyComponentSync.cssexamples/react-lazy-bundle-standalone/src/LazyComponentSync.tsxexamples/react-lazy-bundle-standalone/src/entry-url.tsexamples/react-lazy-bundle/lynx.config.jsexamples/react-lazy-bundle/package.jsonexamples/react-lazy-bundle/src/App.tsxexamples/react-lazy-bundle/src/LazyComponentAsync.cssexamples/react-lazy-bundle/src/LazyComponentAsync.tsxexamples/react-lazy-bundle/src/LazyComponentSync.cssexamples/react-lazy-bundle/src/LazyComponentSync.tsxpackages/react/runtime/__test__/snapshot/lynx/lazy-bundle-fetchbundle.test.jspackages/react/runtime/__test__/snapshot/lynx/lazy-bundle.test.jspackages/react/runtime/__test__/snapshot/lynx/prepareLazyBundleMTS.test.jspackages/react/runtime/lazy/internal.jspackages/react/runtime/src/internal.tspackages/react/runtime/src/lynx.tspackages/react/runtime/src/snapshot/lifecycle/constant.tspackages/react/runtime/src/snapshot/lynx/dynamic-js.tspackages/react/runtime/src/snapshot/lynx/lazy-bundle.tspackages/react/runtime/src/snapshot/lynx/lazyBundleConstants.tspackages/react/runtime/src/snapshot/lynx/prepareLazyBundleMTS.tspackages/react/runtime/types/types.d.tspackages/react/transform/crates/swc_plugin_dynamic_import/lib.rspackages/react/transform/crates/swc_plugin_dynamic_import/tests/__swc_snapshots__/lib.rs/should_transform_import_call.jspackages/react/types/react.docs.d.tspackages/rspeedy/plugin-react/src/entry.tspackages/rspeedy/plugin-react/src/resolveLazyBundleFetcher.tspackages/rspeedy/plugin-react/test/config.test.tspackages/rspeedy/plugin-react/test/resolveLazyBundleFetcher.test.tspackages/webpack/css-extract-webpack-plugin/package.jsonpackages/webpack/react-refresh-webpack-plugin/package.jsonpackages/webpack/react-webpack-plugin/etc/react-webpack-plugin.api.mdpackages/webpack/react-webpack-plugin/package.jsonpackages/webpack/react-webpack-plugin/src/ReactWebpackPlugin.tspackages/webpack/react-webpack-plugin/test/fixtures/lazy-bundle-fetcher/index.jsxpackages/webpack/react-webpack-plugin/test/lazy-bundle-fetcher.test.tspackages/webpack/template-webpack-plugin/etc/template-webpack-plugin.api.mdpackages/webpack/template-webpack-plugin/rstest.config.tspackages/webpack/template-webpack-plugin/src/LynxTemplatePlugin.tspackages/webpack/template-webpack-plugin/src/WebEncodePlugin.tspackages/webpack/template-webpack-plugin/test/fixtures/lazy-bundle-fetcher/entry.jspackages/webpack/template-webpack-plugin/test/fixtures/lazy-bundle-fetcher/foo.bts.jspackages/webpack/template-webpack-plugin/test/fixtures/lazy-bundle-fetcher/foo.mts.jspackages/webpack/template-webpack-plugin/test/lazy-bundle-fetcher.test.tspackages/webpack/template-webpack-plugin/test/web-encode-plugin-fetchbundle.test.tspnpm-workspace.yamlvendor/lynx-js-types-3.10.0.tgz
- resolveLazyBundleFetcher: error message says \`engineVersion\` (the actual config knob), not \`targetSdkVersion\` - lazy-bundle.ts MT sync: wrap CSS load + adopt in try/catch so any stylesheet failure resolves into the never-resolving promise like every other failure mode in the same branch - react-webpack-plugin test: route compiler.close() through finally so it runs even if the run callback rejects
Same catch behavior, no need for two.
Example apps are demonstration projects without unit tests, so they should not count toward project / patch coverage. This brings the ignore list in line with packages/genui/** which is treated the same way.
`__swc_snapshots__` and `__snapshots__` directories hold generated test outputs (vitest/cargo snapshots), not source code that should contribute to coverage.
upupming
requested review from
HuJean,
Yradex,
colinaaa,
gaoachao,
hzy and
luhc228
as code owners
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…work The two pure shape tests (FetchBundle customSections / QueryComponent legacy lepusCode) belong with the rest of the package's case-based tests under \`test/cases/\` — they assert on the on-disk \`tasm.json\` shape and don't need env mutation. The 4 bytecode-gating tests stay standalone in \`lazy-bundle-fetcher.test.ts\` because they flip \`process.env.DEBUG\` per test, and \`cases.test.ts\` runs all cases in one process. Drive-by: standalone test now writes to a per-test \`mkdtemp\` output dir instead of the package's shared \`dist/\`, and the \`web-encode-plugin\` test uses bracket access on the parsed JSON to appease tsc's \`noPropertyAccessFromIndexSignature\`.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/webpack/template-webpack-plugin/test/lazy-bundle-fetcher.test.ts (2)
106-116: 💤 Low valueHandle errors from
compiler.close.Line 113 ignores potential errors from
compiler.close(). While rare, close errors could indicate resource-cleanup failures that might affect subsequent tests.🛡️ Proposed error handling
compiler.run((err, stats) => { if (err) return reject(err); if (!stats) return reject(new Error('webpack returned empty stats')); resolve(stats); - compiler.close(() => void 0); + compiler.close((closeErr) => { + if (closeErr) console.error('webpack close error:', closeErr); + }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/webpack/template-webpack-plugin/test/lazy-bundle-fetcher.test.ts` around lines 106 - 116, The test helper runWebpack currently ignores errors from compiler.close; update runWebpack (the compiler.run callback and its use of compiler.close) so that you only resolve the Promise after compiler.close's callback has run and check its error argument—if close yields an error, reject the Promise (or at minimum propagate/log the error) instead of silently swallowing it; this requires moving the resolve(stats) call into the compiler.close callback and handling close's (err) parameter in runWebpack.
48-54: ⚡ Quick winConsider cleaning up temporary directories.
Each
buildConfigcall creates a new temp directory viamkdtempSync, but the test suite never removes them. Frequent test runs could accumulate many directories intmpdir().♻️ Suggested cleanup approach
Add a cleanup mechanism in the test suite:
+const createdDirs: string[] = []; + function buildConfig( capturePlugin: (compiler: webpack.Compiler) => void, mode: 'development' | 'production', ): webpack.Configuration { const dist = mkdtempSync(join(tmpdir(), 'tmpl-fetchbundle-')); + createdDirs.push(dist); return {Then in the outer
describeblock:afterEach(() => { // ... existing DEBUG restore ... for (const dir of createdDirs) { rmSync(dir, { recursive: true, force: true }); } createdDirs.length = 0; });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/webpack/template-webpack-plugin/test/lazy-bundle-fetcher.test.ts` around lines 48 - 54, buildConfig is creating temp dirs via mkdtempSync that are never removed; track created dirs and remove them in test teardown. Modify buildConfig to push each created dist path into a shared array (e.g., createdDirs) and add an afterEach hook in the test file that iterates createdDirs and calls rmSync(dir, { recursive: true, force: true }) then clears the array; ensure you import/require rmSync and that DEBUG restore logic in afterEach remains intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/webpack/template-webpack-plugin/test/lazy-bundle-fetcher.test.ts`:
- Around line 106-116: The test helper runWebpack currently ignores errors from
compiler.close; update runWebpack (the compiler.run callback and its use of
compiler.close) so that you only resolve the Promise after compiler.close's
callback has run and check its error argument—if close yields an error, reject
the Promise (or at minimum propagate/log the error) instead of silently
swallowing it; this requires moving the resolve(stats) call into the
compiler.close callback and handling close's (err) parameter in runWebpack.
- Around line 48-54: buildConfig is creating temp dirs via mkdtempSync that are
never removed; track created dirs and remove them in test teardown. Modify
buildConfig to push each created dist path into a shared array (e.g.,
createdDirs) and add an afterEach hook in the test file that iterates
createdDirs and calls rmSync(dir, { recursive: true, force: true }) then clears
the array; ensure you import/require rmSync and that DEBUG restore logic in
afterEach remains intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7941f470-3bc8-47f5-ab06-7c2fb30f8bb8
📒 Files selected for processing (10)
packages/webpack/template-webpack-plugin/test/cases/lazy-bundle-fetcher/fetchbundle/foo.bts.jspackages/webpack/template-webpack-plugin/test/cases/lazy-bundle-fetcher/fetchbundle/foo.mts.jspackages/webpack/template-webpack-plugin/test/cases/lazy-bundle-fetcher/fetchbundle/index.jspackages/webpack/template-webpack-plugin/test/cases/lazy-bundle-fetcher/fetchbundle/rspack.config.jspackages/webpack/template-webpack-plugin/test/cases/lazy-bundle-fetcher/querycomponent/foo.bts.jspackages/webpack/template-webpack-plugin/test/cases/lazy-bundle-fetcher/querycomponent/foo.mts.jspackages/webpack/template-webpack-plugin/test/cases/lazy-bundle-fetcher/querycomponent/index.jspackages/webpack/template-webpack-plugin/test/cases/lazy-bundle-fetcher/querycomponent/rspack.config.jspackages/webpack/template-webpack-plugin/test/lazy-bundle-fetcher.test.tspackages/webpack/template-webpack-plugin/test/web-encode-plugin-fetchbundle.test.ts
✅ Files skipped from review due to trivial changes (4)
- packages/webpack/template-webpack-plugin/test/cases/lazy-bundle-fetcher/fetchbundle/index.js
- packages/webpack/template-webpack-plugin/test/cases/lazy-bundle-fetcher/fetchbundle/foo.mts.js
- packages/webpack/template-webpack-plugin/test/cases/lazy-bundle-fetcher/querycomponent/foo.bts.js
- packages/webpack/template-webpack-plugin/test/cases/lazy-bundle-fetcher/querycomponent/rspack.config.js
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/webpack/template-webpack-plugin/test/web-encode-plugin-fetchbundle.test.ts
Drop the vendored \`@lynx-js/types-3.10.0.tgz\` override now that \`@lynx-js/types@3.10.2-alpha.0\` ships the FetchBundle / loadScript declarations. Override pin moves to the alpha; \`vendor/\` removed.
Drop duplicate `@rsbuild/core@2.0.0-beta.3` indirect deps that sherif flagged after the main merge.
Drop the workspace override; the alpha is now referenced directly from each consumer's package.json so resolution behaves like any other dependency.
Collaborator
Author
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db3c74cf11
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
\`use once_cell::sync::Lazy\` resolved transitively through swc_core's deps, but cargo guarantees nothing about reaching transitive crates by their bare name. Declare \`once_cell\` explicitly to match the sibling swc plugins and lock the resolution. Spotted by Codex review.
Now that `once_cell` is declared in Cargo.toml, the indirect `swc_core::ecma::atoms::once_cell` re-export trick is no longer needed. Match `swc_plugin_snapshot`'s simpler import style.
The @lynx-js/types bump pulled some packages back to `@rsbuild/core@2.0.0-beta.3`; dedupe collapses them to the overridden 1.7.5 again.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add a
lynx.fetchBundle-based lazy bundle loader as an opt-in alternative tolynx.QueryComponent. Enables true async loading and per-import sync/async hints.Opt in by setting
engineVersion: '3.8'(or higher) inpluginReactLynx. Then:modeis FetchBundle-only — using it under QueryComponent throws in__DEV__.Output shape
Lazy bundle ships as
customSections:main-thread(bytecoded by default; skipped in dev /DEBUG=rspeedy) +background+CSS, plus a manifest of remaining BTS chunks.Cross-thread coordination
For async mode, BG calls
callLepusMethod('rLynxPrepareLazyBundleMTS')inside its ownfetchBundle.then. Because the bundle is already in native cache, MT's.thenfires synchronously, the C++Callreturns sync, and BG's cb only fires after MT snapshots are registered — so any patch BG sends next lands safely.Test plan
@lynx-js/reactruntime — 19/19 lazy-bundle tests@lynx-js/template-webpack-plugin— 344/344@lynx-js/react-rsbuild-plugin— 161/161@lynx-js/react-transform— 73 (vitest) + 22 (cargo)Summary by CodeRabbit
New Features
lynx.fetchBundleas an alternative to the existing approach, controlled viaengineVersion: '3.8'.import('./X', { with: { mode: 'sync' | 'async' } }).withLazyBundleModehelper for runtime mode control.Chores
@lynx-js/typesto3.10.2-alpha.0across packages.