feat(external-loading-plugin): remove backgroundChunks and mainThreadChunks options#2002
Merged
luhc228 merged 1 commit intolynx-family:mainfrom Dec 18, 2025
Merged
Conversation
🦋 Changeset detectedLatest commit: 466a4f3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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
📝 WalkthroughWalkthroughRemoved Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (11)
💤 Files with no reviewable changes (9)
🧰 Additional context used📓 Path-based instructions (1).changeset/*.md📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (9)📚 Learning: 2025-09-12T09:43:04.847ZApplied to files:
📚 Learning: 2025-07-22T09:26:16.722ZApplied to files:
📚 Learning: 2025-09-29T06:43:40.182ZApplied to files:
📚 Learning: 2025-07-22T09:23:07.797ZApplied to files:
📚 Learning: 2025-09-23T08:53:56.927ZApplied to files:
📚 Learning: 2025-08-21T08:46:54.494ZApplied to files:
📚 Learning: 2025-08-21T07:21:51.621ZApplied to files:
📚 Learning: 2025-07-15T10:00:56.154ZApplied to files:
📚 Learning: 2025-08-27T12:42:01.095ZApplied to files:
🔇 Additional comments (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/webpack/externals-loading-webpack-plugin/src/index.ts (1)
262-264: Remove unnecessary chunk name check.The condition
!this.chunk?.nameappears to be leftover from the previous implementation. Sincechunk.nameis no longer used in the method body (the logic now relies onthis.options.layer), this check is unnecessary.🔎 Simplify the early return condition:
- if (!this.chunk?.name || !externalsLoadingPluginOptions.externals) { + if (!externalsLoadingPluginOptions.externals) { return ''; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.changeset/chilly-ravens-smile.md(0 hunks).changeset/forty-groups-drum.md(1 hunks)examples/react-externals/lynx.config.js(0 hunks)packages/webpack/externals-loading-webpack-plugin/src/index.ts(3 hunks)packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/async/rspack.config.js(0 hunks)packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/filter-duplicate-externals/rspack.config.js(0 hunks)packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/not-overrides-existed-externals/rspack.config.js(0 hunks)packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/only-background-externals/rspack.config.js(0 hunks)packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/only-main-thread-externals/rspack.config.js(0 hunks)packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/sync/rspack.config.js(0 hunks)
💤 Files with no reviewable changes (8)
- packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/async/rspack.config.js
- .changeset/chilly-ravens-smile.md
- packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/only-main-thread-externals/rspack.config.js
- examples/react-externals/lynx.config.js
- packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/sync/rspack.config.js
- packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/filter-duplicate-externals/rspack.config.js
- packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/not-overrides-existed-externals/rspack.config.js
- packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/only-background-externals/rspack.config.js
🧰 Additional context used
📓 Path-based instructions (1)
.changeset/*.md
📄 CodeRabbit inference engine (AGENTS.md)
For contributions, generate and commit a Changeset describing your changes
Files:
.changeset/forty-groups-drum.md
🧠 Learnings (7)
📚 Learning: 2025-09-12T09:43:04.847Z
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.847Z
Learning: In the lynx-family/lynx-stack repository, empty changeset files (containing only `---\n\n---`) are used for internal changes that modify src/** files but don't require meaningful release notes, such as private package changes or testing-only modifications. This satisfies CI requirements without generating user-facing release notes.
Applied to files:
.changeset/forty-groups-drum.md
📚 Learning: 2025-09-29T06:43:40.182Z
Learnt from: CR
Repo: lynx-family/lynx-stack PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-29T06:43:40.182Z
Learning: Applies to .changeset/*.md : For contributions, generate and commit a Changeset describing your changes
Applied to files:
.changeset/forty-groups-drum.md
📚 Learning: 2025-07-22T09:26:16.722Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:26:16.722Z
Learning: In the lynx-family/lynx-stack repository, CI checks require changesets when files matching the pattern "src/**" are modified (as configured in .changeset/config.json). For internal changes that don't need meaningful changesets, an empty changeset file is used to satisfy the CI requirement while not generating any release notes.
Applied to files:
.changeset/forty-groups-drum.md
📚 Learning: 2025-07-22T09:23:07.797Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.
Applied to files:
.changeset/forty-groups-drum.md
📚 Learning: 2025-08-27T12:42:01.095Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1616
File: packages/webpack/cache-events-webpack-plugin/test/cases/not-cache-events/lazy-bundle/index.js:3-3
Timestamp: 2025-08-27T12:42:01.095Z
Learning: In webpack, properties like __webpack_require__.lynx_ce are injected during compilation/build time when webpack processes modules and generates bundles, not at runtime when dynamic imports execute. Tests for such properties don't need to wait for dynamic imports to complete.
Applied to files:
packages/webpack/externals-loading-webpack-plugin/src/index.ts
📚 Learning: 2025-08-21T08:46:54.494Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1370
File: packages/webpack/cache-events-webpack-plugin/src/LynxCacheEventsRuntimeModule.ts:23-27
Timestamp: 2025-08-21T08:46:54.494Z
Learning: In Lynx webpack runtime modules, the team prioritizes performance and simplicity over defensive runtime error handling. They prefer relying on compile-time type safety (TypeScript) rather than adding runtime checks like try-catch blocks or type validation, especially for performance-critical code like cache event setup/cleanup functions.
Applied to files:
packages/webpack/externals-loading-webpack-plugin/src/index.ts
📚 Learning: 2025-08-19T12:49:05.875Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1370
File: packages/webpack/template-webpack-plugin/src/LynxCacheEventsSetupListRuntimeModule.ts:21-25
Timestamp: 2025-08-19T12:49:05.875Z
Learning: In the Lynx cache events system, there are two separate runtime modules with distinct responsibilities: `LynxCacheEventsSetupListRuntimeModule` is only responsible for initializing the setup list with the setup functions, while `LynxCacheEventsRuntimeModule` guarantees the initialization of `loaded` and `cachedActions` properties. The modules have a dependency relationship where `lynxCacheEventsSetupList` is required by `lynxCacheEvents`.
Applied to files:
packages/webpack/externals-loading-webpack-plugin/src/index.ts
🔇 Additional comments (2)
packages/webpack/externals-loading-webpack-plugin/src/index.ts (2)
257-259: LGTM! Clean constructor design.The refactored constructor properly receives the layer context at instantiation time, which simplifies the runtime module's logic.
268-288: LGTM! Layer mapping logic is sound.The method correctly determines the layer by comparing
chunkLayeragainst configured layer names and gracefully handles unknown layers by returning an empty string.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
luhc228
force-pushed
the
remove-chunks-option
branch
from
f44f0f9 to
466a4f3
Compare
Contributor
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/webpack/externals-loading-webpack-plugin/src/index.ts (1)
227-228: Update documentation example to remove stale options.The JSDoc example still references
mainThreadChunksandbackgroundChunks, which are being removed in this PR. This will confuse users consulting the API documentation.🔎 Apply this diff to update the example:
* export default { * plugins: [ * new ExternalsLoadingPlugin({ * mainThreadLayer: 'main-thread', * backgroundLayer: 'background', -* mainThreadChunks: ['index__main-thread'], -* backgroundChunks: ['index'], * externals: {
♻️ Duplicate comments (1)
.changeset/forty-groups-drum.md (1)
1-3: Add a proper changeset for this breaking change.This PR removes
mainThreadChunksandbackgroundChunksfrom the publicExternalsLoadingPluginOptionsinterface. An empty changeset is insufficient for breaking API changes. The changeset should document:
- The removed options
- Rationale (runtime modules inject only into entry chunks)
- Mark as breaking change
Based on retrieved learnings, empty changesets are only appropriate for internal changes that don't require meaningful release notes.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.changeset/chilly-ravens-smile.md(0 hunks).changeset/forty-groups-drum.md(1 hunks)examples/react-externals/lynx.config.js(0 hunks)packages/webpack/externals-loading-webpack-plugin/etc/externals-loading-webpack-plugin.api.md(0 hunks)packages/webpack/externals-loading-webpack-plugin/src/index.ts(3 hunks)packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/async/rspack.config.js(0 hunks)packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/filter-duplicate-externals/rspack.config.js(0 hunks)packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/not-overrides-existed-externals/rspack.config.js(0 hunks)packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/only-background-externals/rspack.config.js(0 hunks)packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/only-main-thread-externals/rspack.config.js(0 hunks)packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/sync/rspack.config.js(0 hunks)
💤 Files with no reviewable changes (9)
- examples/react-externals/lynx.config.js
- packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/only-main-thread-externals/rspack.config.js
- packages/webpack/externals-loading-webpack-plugin/etc/externals-loading-webpack-plugin.api.md
- packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/only-background-externals/rspack.config.js
- .changeset/chilly-ravens-smile.md
- packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/async/rspack.config.js
- packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/not-overrides-existed-externals/rspack.config.js
- packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/sync/rspack.config.js
- packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/filter-duplicate-externals/rspack.config.js
🧰 Additional context used
📓 Path-based instructions (1)
.changeset/*.md
📄 CodeRabbit inference engine (AGENTS.md)
For contributions, generate and commit a Changeset describing your changes
Files:
.changeset/forty-groups-drum.md
🧠 Learnings (9)
📚 Learning: 2025-09-12T09:43:04.847Z
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.847Z
Learning: In the lynx-family/lynx-stack repository, empty changeset files (containing only `---\n\n---`) are used for internal changes that modify src/** files but don't require meaningful release notes, such as private package changes or testing-only modifications. This satisfies CI requirements without generating user-facing release notes.
Applied to files:
.changeset/forty-groups-drum.md
📚 Learning: 2025-07-22T09:26:16.722Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:26:16.722Z
Learning: In the lynx-family/lynx-stack repository, CI checks require changesets when files matching the pattern "src/**" are modified (as configured in .changeset/config.json). For internal changes that don't need meaningful changesets, an empty changeset file is used to satisfy the CI requirement while not generating any release notes.
Applied to files:
.changeset/forty-groups-drum.md
📚 Learning: 2025-09-29T06:43:40.182Z
Learnt from: CR
Repo: lynx-family/lynx-stack PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-29T06:43:40.182Z
Learning: Applies to .changeset/*.md : For contributions, generate and commit a Changeset describing your changes
Applied to files:
.changeset/forty-groups-drum.md
📚 Learning: 2025-07-22T09:23:07.797Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.
Applied to files:
.changeset/forty-groups-drum.md
📚 Learning: 2025-09-23T08:53:56.927Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1670
File: packages/webpack/css-extract-webpack-plugin/src/loader.ts:244-251
Timestamp: 2025-09-23T08:53:56.927Z
Learning: In webpack CSS extraction plugins, when storing per-module dependencies in a compiler-scoped map like cssModuleId2Deps, the map should not be reset at compilation start because in incremental compilation (watch mode/HMR), only changed files pass through the loader. Unchanged modules need their dependency information to persist between compilations so the plugin can access all modules' dependencies when generating CSS output.
Applied to files:
packages/webpack/externals-loading-webpack-plugin/src/index.ts
📚 Learning: 2025-08-21T08:46:54.494Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1370
File: packages/webpack/cache-events-webpack-plugin/src/LynxCacheEventsRuntimeModule.ts:23-27
Timestamp: 2025-08-21T08:46:54.494Z
Learning: In Lynx webpack runtime modules, the team prioritizes performance and simplicity over defensive runtime error handling. They prefer relying on compile-time type safety (TypeScript) rather than adding runtime checks like try-catch blocks or type validation, especially for performance-critical code like cache event setup/cleanup functions.
Applied to files:
packages/webpack/externals-loading-webpack-plugin/src/index.ts
📚 Learning: 2025-08-21T07:21:51.621Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1562
File: packages/react/transform/src/swc_plugin_snapshot/jsx_helpers.rs:261-283
Timestamp: 2025-08-21T07:21:51.621Z
Learning: In packages/react/transform/src/swc_plugin_snapshot/jsx_helpers.rs, the team prefers to keep the original unreachable! logic for JSXSpreadChild in jsx_is_children_full_dynamic function rather than implementing defensive error handling.
Applied to files:
packages/webpack/externals-loading-webpack-plugin/src/index.ts
📚 Learning: 2025-07-15T10:00:56.154Z
Learnt from: PupilTong
Repo: lynx-family/lynx-stack PR: 1292
File: packages/web-platform/web-core-server/src/createLynxView.ts:144-151
Timestamp: 2025-07-15T10:00:56.154Z
Learning: In the lynx-stack codebase, PupilTong prefers the "let it crash" approach over defensive null safety checks when the condition should never occur in normal operation. This applies to cases like the `element.getAttribute(lynxUniqueIdAttribute)!` call in SSR event capture where the attribute is expected to always be present.
Applied to files:
packages/webpack/externals-loading-webpack-plugin/src/index.ts
📚 Learning: 2025-08-27T12:42:01.095Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1616
File: packages/webpack/cache-events-webpack-plugin/test/cases/not-cache-events/lazy-bundle/index.js:3-3
Timestamp: 2025-08-27T12:42:01.095Z
Learning: In webpack, properties like __webpack_require__.lynx_ce are injected during compilation/build time when webpack processes modules and generates bundles, not at runtime when dynamic imports execute. Tests for such properties don't need to wait for dynamic imports to complete.
Applied to files:
packages/webpack/externals-loading-webpack-plugin/src/index.ts
🔇 Additional comments (3)
packages/webpack/externals-loading-webpack-plugin/src/index.ts (3)
257-265: LGTM!Clean refactor - the constructor now accepts the layer via an options object, and
generate()properly guards against missing chunk name or externals configuration before delegating to the layer-specific code generation.
268-288: LGTM!The layer mapping logic is well-structured - it maps the chunk's layer name to the concrete
'background'or'mainThread'value, and gracefully returns an empty string for chunks that don't match either configured layer.
413-426: LGTM - null safety concern addressed.The implementation now properly iterates without converting to an array, breaks early on finding a layer, and returns early if no layer is found. This addresses the previous review concern about potential runtime errors from non-null assertions.
upupming
approved these changes
Dec 18, 2025
CodSpeed Performance ReportMerging #2002 will not alter performanceComparing Summary
Footnotes
|
React Example#6497 Bundle Size — 236.9KiB (0%).466a4f3(current) vs 3a9a730 main#6495(baseline) Bundle metrics
|
| Current #6497 |
Baseline #6495 |
|
|---|---|---|
 Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
162 |
162 |
| Duplicate Modules |
65 |
65 |
| Duplicate Code |
46.74% |
46.74% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #6497 |
Baseline #6495 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
91.14KiB |
91.14KiB |
Bundle analysis report Branch luhc228:remove-chunks-option Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#6657 Bundle Size — 372.63KiB (0%).466a4f3(current) vs 3a9a730 main#6655(baseline) Bundle metrics
|
| Current #6657 |
Baseline #6655 |
|
|---|---|---|
| Initial JS |
146.36KiB |
146.36KiB |
| Initial CSS |
32.4KiB |
32.4KiB |
| Cache Invalidation |
0% |
0% |
| Chunks |
8 |
8 |
| Assets |
8 |
8 |
 Modules |
229(-0.87%) |
231 |
| Duplicate Modules |
16 |
16 |
| Duplicate Code |
2.97% |
2.97% |
| Packages |
4 |
4 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #6657 |
Baseline #6655 |
|
|---|---|---|
| JS |
243.25KiB |
243.25KiB |
| Other |
96.98KiB |
96.98KiB |
| CSS |
32.4KiB |
32.4KiB |
Bundle analysis report Branch luhc228:remove-chunks-option Project dashboard
Generated by RelativeCI Documentation Report issue
f0rdream
pushed a commit
to f0rdream/lynx-stack
that referenced
this pull request
Dec 18, 2025
…Chunks options (lynx-family#2002) `compilation.addRuntimeModule()` will only inject the runtime code into entry chunk. It's unnecessary to use `backgroundChunks` and `mainThreadChunks` to distinguish chunks. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Removed the plugin options that allowed explicit per-chunk mappings for main-thread and background loading. * **Chore** * Added an inert metadata changeset file (placeholder entry). <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai --> ## Checklist <!--- Check and mark with an "x" --> - [x] Tests updated (or not required). - x] Documentation updated (or not required). - [ ] Changeset added, and when a BREAKING CHANGE occurs, it needs to be clearly marked (or not required).
This was referenced Dec 18, 2025
3 tasks
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.
compilation.addRuntimeModule()will only inject the runtime code into entry chunk. It's unnecessary to usebackgroundChunksandmainThreadChunksto distinguish chunks.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.
Checklist