perf(externals-loading): merge multiple fetchBundle calls for the same URL

[upupming]

Summary

This PR optimizes the loading of external bundles by merging multiple fetchBundle calls for the same URL into a single request.

Previously, if multiple independent components or modules requested the same external bundle simultaneously, each would trigger a separate fetchBundle call. This optimization ensures that subsequent requests for an in-flight URL wait for the first request to complete and share the result.

Summary by CodeRabbit

• New Features

  • Performance optimization: multiple loads targeting the same URL are merged into a single request, reducing network overhead and improving startup time.

• Tests

  • Added test cases and fixtures to validate merged-request behavior and ensure clean test environment.

• Chores

  • Added release metadata for a patch release.

Checklist

• Tests updated (or not required).
• Documentation updated (or not required).
• Changeset added, and when a BREAKING CHANGE occurs, it needs to be clearly marked (or not required).

[changeset-bot]

🦋 Changeset detected

Latest commit: 82e7124

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@lynx-js/externals-loading-webpack-plugin	Patch
@lynx-js/external-bundle-rsbuild-plugin	Patch

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

[coderabbitai]

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (1)

• pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5d5c9fb5-4f83-48e0-b372-a006db9f9030

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Merges multiple fetchBundle calls targeting the same URL into a single request by changing runtime code generation to use a URL-keyed Map and Set; adds tests, test configs, and mock external bundles to verify the merge behavior.

Changes

Cohort / File(s)	Summary
Changelog ./.changeset/merge-fetchbundle-calls.md	Adds a changeset entry documenting the fetchBundle merge optimization.
Package manifest packages/webpack/externals-loading-webpack-plugin/package.json	Updates devDependencies to point to new test helper mock package paths (bar, baz, foo).
Core plugin runtime packages/webpack/externals-loading-webpack-plugin/src/index.ts	Replaces array-based codegen with a per-URL Map (url2fetchCode) and Set (loadCode); emits per-URL handler declarations and computes handler indices from map keys.
Tests bootstrap packages/webpack/externals-loading-webpack-plugin/test/cases.test.ts, packages/webpack/externals-loading-webpack-plugin/test/helpers/setup-env.js	Adds a beforeExecute hook to clear global lynx state and converts test env helpers to ES imports with expanded CustomSections mapping.
New test case & configs packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/merge-fetchBundle-calls/index.js, .../rspack.config.js, .../test.config.cjs	Adds rspack config, test config, and a test that asserts generated bundles contain a single fetchBundle( occurrence per output file.
Test helper mocks packages/webpack/externals-loading-webpack-plugin/test/helpers/external-bundle-mock/bar/*, .../baz/*, .../qux/*	Adds mock packages: bar (minus), baz (common, sub1.mul, sub2.div, index re-exports), and qux (exp) plus package.json files for each mock.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• feat(webpack-plugin): init externals-loading-webpack-plugin #1924: Modifies the plugin's runtime code generation and tests in the same area.
• feat: support externals sharing across bundles #2007: Overlaps on edits to src/index.ts for externals-loading runtime assembly.
• feat: allow globalObject config and reuse fetchBundle result #2123: Implements reuse of fetchBundle results and per-URL consolidation behavior.

Suggested reviewers

• colinaaa
• luhc228

Poem

🐰 I hopped through bytes and stitched each trail,
One URL, one fetch — no extra tale.
Map, Set, and tests in tidy rows,
Fewer calls where the river flows.
The rabbit smiles — efficiency grows 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main performance optimization: merging multiple fetchBundle calls for the same URL, which is the primary change evidenced by the changeset entry, core logic refactoring in index.ts using a per-URL Map, and the comprehensive test case demonstrating this exact behavior.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch perf/merge-fetchbundle-calls

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/merge-fetchBundle-calls/index.js (1)

11-28: Consider adding a clarifying comment for the assertion logic.

The split-based counting technique works correctly (split produces N+1 parts for N occurrences), but the magic number 2 may not be immediately obvious to future readers. A brief comment explaining the expected behavior would improve maintainability.

📝 Suggested improvement

   const backgroundFetchBundleCalls = background.split('fetchBundle' + '(');
   const mainThreadFetchBundleCalls = mainThread.split('fetchBundle' + '(');
+  // split produces N+1 parts for N occurrences; expect exactly 1 fetchBundle call per file
   expect(backgroundFetchBundleCalls.length).toBe(2);
   expect(mainThreadFetchBundleCalls.length).toBe(2);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/merge-fetchBundle-calls/index.js`
around lines 11 - 28, The assertions using background.split('fetchBundle' + '(')
and mainThread.split('fetchBundle' + '(') compare lengths to the magic number 2,
which represents one occurrence (split yields N+1 parts); add a short clarifying
comment above the two expect lines (near the test "should merge fetchBundle
calls" or next to variables backgroundFetchBundleCalls and
mainThreadFetchBundleCalls) that explains "split returns N+1 parts so length===2
means one fetchBundle call" to make the intent clear to future readers.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/webpack/externals-loading-webpack-plugin/src/index.ts`:
- Around line 418-422: The generated async load code references handler${i} (in
the loadCode.add call using mountVar and createLoadExternalAsync) but handlers
are emitted in URL-unique order, so handler indices don’t match external indices
and can be undefined; fix by resolving the correct handler variable for the
external’s URL before emitting the async string (e.g. compute or look up the
emitted handler index/variable for this external — call it handlerForUrl or
handlerIndex — or capture the handler into a const in the loop) and use that
resolved handler variable in createLoadExternalAsync; apply the same change
where handlers are emitted later (the handler emission block) so both places
reference the same handler variable mapping instead of handler${i}.

---

Nitpick comments:
In
`@packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/merge-fetchBundle-calls/index.js`:
- Around line 11-28: The assertions using background.split('fetchBundle' + '(')
and mainThread.split('fetchBundle' + '(') compare lengths to the magic number 2,
which represents one occurrence (split yields N+1 parts); add a short clarifying
comment above the two expect lines (near the test "should merge fetchBundle
calls" or next to variables backgroundFetchBundleCalls and
mainThreadFetchBundleCalls) that explains "split returns N+1 parts so length===2
means one fetchBundle call" to make the intent clear to future readers.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5a6fe5f8-0e72-4f44-b904-eacdbd06f616

📥 Commits

Reviewing files that changed from the base of the PR and between 5fc68ae and 7883c13.

⛔ Files ignored due to path filters (1)

• pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml

📒 Files selected for processing (17)

• .changeset/merge-fetchbundle-calls.md
• packages/webpack/externals-loading-webpack-plugin/package.json
• packages/webpack/externals-loading-webpack-plugin/src/index.ts
• packages/webpack/externals-loading-webpack-plugin/test/cases.test.ts
• packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/merge-fetchBundle-calls/index.js
• packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/merge-fetchBundle-calls/rspack.config.js
• packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/merge-fetchBundle-calls/test.config.cjs
• packages/webpack/externals-loading-webpack-plugin/test/helpers/external-bundle-mock/bar/index.js
• packages/webpack/externals-loading-webpack-plugin/test/helpers/external-bundle-mock/bar/package.json
• packages/webpack/externals-loading-webpack-plugin/test/helpers/external-bundle-mock/baz/common.js
• packages/webpack/externals-loading-webpack-plugin/test/helpers/external-bundle-mock/baz/index.js
• packages/webpack/externals-loading-webpack-plugin/test/helpers/external-bundle-mock/baz/package.json
• packages/webpack/externals-loading-webpack-plugin/test/helpers/external-bundle-mock/baz/sub1/index.js
• packages/webpack/externals-loading-webpack-plugin/test/helpers/external-bundle-mock/baz/sub2/index.js
• packages/webpack/externals-loading-webpack-plugin/test/helpers/external-bundle-mock/foo/index.js
• packages/webpack/externals-loading-webpack-plugin/test/helpers/external-bundle-mock/foo/package.json
• packages/webpack/externals-loading-webpack-plugin/test/helpers/setup-env.js

[coderabbitai]

♻️ Duplicate comments (1)

packages/webpack/externals-loading-webpack-plugin/src/index.ts (1)

418-422: ⚠️ Potential issue | 🔴 Critical

Critical: async path still uses external index instead of URL handler index.

At Line 419, handler${i} is keyed by external iteration index, but handlers are emitted by unique URL order at Line 438-Line 440. With shared URLs, this can reference an undefined handler and break loading at runtime.

🐛 Proposed fix

           url2fetchCode.set(
             url,
             `lynx.fetchBundle(${JSON.stringify(url)}, {});`,
           );
+          const handlerIndex = [...url2fetchCode.keys()].indexOf(url);

           const mountVar = `${
             getLynxExternalGlobal(
               externalsLoadingPluginOptions.globalObject,
             )
           }[${JSON.stringify(libraryNameStr)}]`;
           if (async) {
             loadCode.add(
-              `${mountVar} = ${mountVar} === undefined ? createLoadExternalAsync(handler${i}, ${
+              `${mountVar} = ${mountVar} === undefined ? createLoadExternalAsync(handler${handlerIndex}, ${
                 JSON.stringify(layerOptions.sectionPath)
               }) : ${mountVar};`,
             );
             continue;
           }

           loadCode.add(
             `${mountVar} = ${mountVar} === undefined ? createLoadExternalSync(handler${
-              [...url2fetchCode.keys()].indexOf(url)
+              handlerIndex
             }, ${
               JSON.stringify(layerOptions.sectionPath)
             }, ${timeout}) : ${mountVar};`,
           );

Also applies to: 438-440

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/webpack/externals-loading-webpack-plugin/src/index.ts` around lines
418 - 422, The async-load line uses handler${i} (external iteration index) but
the actual handlers are emitted in URL-unique order, so with duplicate URLs
handler${i} can be undefined; change the generated async path to look up the
handler by the URL's emitted index (e.g., use the same handlerIndex mapping used
when emitting the handlers array — reference the handlers emission logic that
produces the handlers list/ordering) and pass that handler (or its index) into
createLoadExternalAsync instead of handler${i}; update the code that pushes into
loadCode (the line assigning mountVar using createLoadExternalAsync) to use the
URL-derived handler lookup so async and sync paths reference the same handler.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@packages/webpack/externals-loading-webpack-plugin/src/index.ts`:
- Around line 418-422: The async-load line uses handler${i} (external iteration
index) but the actual handlers are emitted in URL-unique order, so with
duplicate URLs handler${i} can be undefined; change the generated async path to
look up the handler by the URL's emitted index (e.g., use the same handlerIndex
mapping used when emitting the handlers array — reference the handlers emission
logic that produces the handlers list/ordering) and pass that handler (or its
index) into createLoadExternalAsync instead of handler${i}; update the code that
pushes into loadCode (the line assigning mountVar using createLoadExternalAsync)
to use the URL-derived handler lookup so async and sync paths reference the same
handler.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: bd673249-6d3a-43c5-9a1d-aa7861c1b7c9

📥 Commits

Reviewing files that changed from the base of the PR and between 7883c13 and 3304a22.

⛔ Files ignored due to path filters (1)

• pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml

📒 Files selected for processing (17)

• .changeset/merge-fetchbundle-calls.md
• packages/webpack/externals-loading-webpack-plugin/package.json
• packages/webpack/externals-loading-webpack-plugin/src/index.ts
• packages/webpack/externals-loading-webpack-plugin/test/cases.test.ts
• packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/merge-fetchBundle-calls/index.js
• packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/merge-fetchBundle-calls/rspack.config.js
• packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/merge-fetchBundle-calls/test.config.cjs
• packages/webpack/externals-loading-webpack-plugin/test/helpers/external-bundle-mock/bar/index.js
• packages/webpack/externals-loading-webpack-plugin/test/helpers/external-bundle-mock/bar/package.json
• packages/webpack/externals-loading-webpack-plugin/test/helpers/external-bundle-mock/baz/common.js
• packages/webpack/externals-loading-webpack-plugin/test/helpers/external-bundle-mock/baz/index.js
• packages/webpack/externals-loading-webpack-plugin/test/helpers/external-bundle-mock/baz/package.json
• packages/webpack/externals-loading-webpack-plugin/test/helpers/external-bundle-mock/baz/sub1/index.js
• packages/webpack/externals-loading-webpack-plugin/test/helpers/external-bundle-mock/baz/sub2/index.js
• packages/webpack/externals-loading-webpack-plugin/test/helpers/external-bundle-mock/foo/index.js
• packages/webpack/externals-loading-webpack-plugin/test/helpers/external-bundle-mock/foo/package.json
• packages/webpack/externals-loading-webpack-plugin/test/helpers/setup-env.js

✅ Files skipped from review due to trivial changes (1)

• packages/webpack/externals-loading-webpack-plugin/test/helpers/external-bundle-mock/bar/package.json

🚧 Files skipped from review as they are similar to previous changes (9)

• packages/webpack/externals-loading-webpack-plugin/test/helpers/external-bundle-mock/baz/sub2/index.js
• .changeset/merge-fetchbundle-calls.md
• packages/webpack/externals-loading-webpack-plugin/test/helpers/external-bundle-mock/baz/index.js
• packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/merge-fetchBundle-calls/rspack.config.js
• packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/merge-fetchBundle-calls/test.config.cjs
• packages/webpack/externals-loading-webpack-plugin/test/helpers/external-bundle-mock/bar/index.js
• packages/webpack/externals-loading-webpack-plugin/test/cases.test.ts
• packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/merge-fetchBundle-calls/index.js
• packages/webpack/externals-loading-webpack-plugin/test/helpers/external-bundle-mock/baz/package.json

[codspeed-hq]

Merging this PR will degrade performance by 5.58%

❌ 1 regressed benchmark
✅ 71 untouched benchmarks
⏩ 3 skipped benchmarks¹

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Benchmark	BASE	HEAD	Efficiency
❌	basic-performance-text-200	18.3 ms	19.3 ms	-5.58%

---

_{Comparing perf/merge-fetchbundle-calls (82e7124) with main (f730d3c)}

Footnotes

1. 3 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. ↩

[relativeci]

Web Explorer

#8055 Bundle Size — 384.5KiB (0%).

82e7124(current) vs f730d3c main#8048(baseline)

Bundle metrics  no changes

	Current #8055	Baseline #8048
Initial JS	155.59KiB	155.59KiB
Initial CSS	35.1KiB	35.1KiB
Cache Invalidation	0%	0%
Chunks	8	8
Assets	8	8
Modules	239	239
Duplicate Modules	16	16
Duplicate Code	2.98%	2.98%
Packages	4	4
Duplicate Packages	0	0

Bundle size by type  no changes

	Current #8055	Baseline #8048
JS	253.55KiB	253.55KiB
Other	95.85KiB	95.85KiB
CSS	35.1KiB	35.1KiB

Bundle analysis report Branch perf/merge-fetchbundle-calls Project dashboard

---

^{Generated by RelativeCI Documentation Report issue}