feat(react-webpack): route lepus chunks through standard pipeline for slardar sourcemaps#2310
feat(react-webpack): route lepus chunks through standard pipeline for slardar sourcemaps#2310Yradex wants to merge 4 commits intolynx-family:mainfrom
Conversation
… slardar sourcemaps
🦋 Changeset detectedLatest commit: c466718 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 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 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR refactors worklet runtime injection in the React webpack plugin by introducing a Lepus chunk pipeline infrastructure. It replaces single hardcoded worklet-runtime handling with dynamic per-chunk compilation and injection, adds worklet usage detection in loaders, and updates runtime wrapper patterns to accommodate multiple Lepus chunks. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Merging this PR will degrade performance by 7.61%
Performance Changes
Comparing 
Footnotes
|
Web Explorer#8046 Bundle Size — 384.5KiB (0%).c466718(current) vs 4daa4d9 main#8021(baseline) Bundle metrics
|
| Current #8046 |
Baseline #8021 |
|
|---|---|---|
 Initial JS |
155.59KiB |
155.59KiB |
| Initial CSS |
35.1KiB |
35.1KiB |
| Cache Invalidation |
0% |
0% |
| Chunks |
8 |
8 |
| Assets |
8 |
8 |
 Modules |
238(-0.42%) |
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 #8046 |
Baseline #8021 |
|
|---|---|---|
| JS |
253.55KiB |
253.55KiB |
| Other |
95.85KiB |
95.85KiB |
| CSS |
35.1KiB |
35.1KiB |
Bundle analysis report Branch Yradex:mtf-runtime-slardar Project dashboard
Generated by RelativeCI Documentation Report issue
Yradex
force-pushed
the
mtf-runtime-slardar
branch
from
0b7f81d to
b5249a5
Compare
![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: b5249a58fa
ℹ️ 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".
packages/webpack/react-webpack-plugin/src/lepusChunkPipeline.ts
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/rspeedy/plugin-react/test/config.test.ts (1)
2448-2450: Make this test follow the dynamic lepus chunk list.The implementation now excludes by lepus chunk names, but this test still only exercises the current
worklet-runtimesingleton. If another lepus chunk is added, the wrapper regex can regress without the test noticing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rspeedy/plugin-react/test/config.test.ts` around lines 2448 - 2450, The test currently hardcodes the 'worklet-runtime' chunk; modify it to iterate over the dynamic lepus chunk list used by the implementation (import the lepus chunk names constant or function used by the module under test) and for each chunk name assert that testRule.test(`static/js/${chunk}.js`) and testRule.test(`static/js/${chunk}.<hash>.js`) both return false; update the assertions referencing testRule.test and the existing string patterns so the test covers every lepus chunk rather than just 'worklet-runtime'.
🤖 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/react-webpack-plugin/src/lepusChunkPipeline.ts`:
- Around line 57-82: The current code builds queuedChunks from the ephemeral
requestedChunkNames populated in succeedModule, which misses unchanged modules
on incremental rebuilds; instead, inside compilation.hooks.finishModules (the
finishModules callback) iterate over the full module set provided there and for
each item in lepusChunkPipeline call shouldCompile(module) against those modules
to decide inclusion—populate queuedChunks/queuedChunkByName from that pass (or
compute a Set of present module identifiers and test each LepusChunkPipelineItem
against that Set) so worklet-runtime and other persistent dependencies are
preserved across incremental compilations.
In
`@packages/webpack/react-webpack-plugin/test/cases/worklet-runtime/standard-path/index.js`:
- Around line 44-47: The test's assertion is too permissive because the fallback
injected.includes('function __webpack_require__(') allows unwrapped bundles to
pass; update the assertion in the test so it requires the isolated IIFE wrapper
produced by createIsolatedLepusChunkSource() (check the injected string only for
an IIFE start, e.g. use a strict startsWith('(function(') or a regex like
/^\(function\s*\(/) and remove the fallback includes(...) branch so the test
fails if the IIFE wrapper is not present.
---
Nitpick comments:
In `@packages/rspeedy/plugin-react/test/config.test.ts`:
- Around line 2448-2450: The test currently hardcodes the 'worklet-runtime'
chunk; modify it to iterate over the dynamic lepus chunk list used by the
implementation (import the lepus chunk names constant or function used by the
module under test) and for each chunk name assert that
testRule.test(`static/js/${chunk}.js`) and
testRule.test(`static/js/${chunk}.<hash>.js`) both return false; update the
assertions referencing testRule.test and the existing string patterns so the
test covers every lepus chunk rather than just 'worklet-runtime'.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 09af0b72-7f23-479d-b73e-637bf344c743
📒 Files selected for processing (13)
.changeset/sweet-rings-kneel.mdpackages/rspeedy/plugin-react/src/entry.tspackages/rspeedy/plugin-react/test/config.test.tspackages/webpack/react-webpack-plugin/src/ReactWebpackPlugin.tspackages/webpack/react-webpack-plugin/src/lepusChunkPipeline.tspackages/webpack/react-webpack-plugin/src/loaders/background.tspackages/webpack/react-webpack-plugin/src/loaders/main-thread.tspackages/webpack/react-webpack-plugin/src/workletMetadata.tspackages/webpack/react-webpack-plugin/test/cases/worklet-runtime/not-using/index.jspackages/webpack/react-webpack-plugin/test/cases/worklet-runtime/standard-path/a.jsxpackages/webpack/react-webpack-plugin/test/cases/worklet-runtime/standard-path/index.jspackages/webpack/react-webpack-plugin/test/cases/worklet-runtime/standard-path/rspack.config.jspackages/webpack/react-webpack-plugin/test/cases/worklet-runtime/standard-path/test.config.cjs
packages/webpack/react-webpack-plugin/src/lepusChunkPipeline.ts
Outdated
Show resolved
Hide resolved
packages/webpack/react-webpack-plugin/test/cases/worklet-runtime/standard-path/index.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/webpack/react-webpack-plugin/src/lepusChunkPipeline.ts (1)
216-222: Function-based entry configurations are not currently used in the codebase.The
hasEntryfunction only checks the object form of webpack's entry configuration. While webpack does support function entries that return configurations dynamically (e.g.,entry: () => ({ 'worklet-runtime': './src/worklet.js' })), a search of the repository found no use of function-based entry configurations—all entries are object-based (e.g.,entry: { main: '...' }orentry: Object.fromEntries(...)).The theoretical risk of returning
falsefor function-based entries remains valid if someone adds such a configuration in the future, but this is not an issue affecting the current codebase.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webpack/react-webpack-plugin/src/lepusChunkPipeline.ts` around lines 216 - 222, The hasEntry helper currently only handles object entries and will miss function-based entries; update hasEntry(compiler: Compiler, name: string) to support function entries by detecting if compiler.options.entry is a function, invoking it (awaiting if it returns a Promise) to obtain the resolved entry object, then perform the same object-type check and "name in entry" lookup; this will require changing hasEntry to an async function and callers to await its result (or adapt call sites accordingly) so function-returning webpack entries are correctly recognized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/webpack/react-webpack-plugin/src/lepusChunkPipeline.ts`:
- Around line 216-222: The hasEntry helper currently only handles object entries
and will miss function-based entries; update hasEntry(compiler: Compiler, name:
string) to support function entries by detecting if compiler.options.entry is a
function, invoking it (awaiting if it returns a Promise) to obtain the resolved
entry object, then perform the same object-type check and "name in entry"
lookup; this will require changing hasEntry to an async function and callers to
await its result (or adapt call sites accordingly) so function-returning webpack
entries are correctly recognized.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 429996eb-ed93-43f9-8074-4ba628560b3a
📒 Files selected for processing (2)
packages/webpack/react-webpack-plugin/src/lepusChunkPipeline.tspackages/webpack/react-webpack-plugin/test/cases/worklet-runtime/standard-path/index.js
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/webpack/react-webpack-plugin/test/cases/worklet-runtime/standard-path/index.js
|
@colinaaa Take a look? |
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Checklist