Extract and own the webpack RSC plugin as TypeScript source#87
Conversation
|
Warning Review limit reached
More reviews will be available in 8 minutes and 43 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| dep.request, | ||
| ); | ||
| block.addDependency(dep); | ||
| module.addBlock!(block); |
There was a problem hiding this comment.
FlightModule.addBlock is typed as optional, so ! suppresses the TypeScript error but shifts the failure to a runtime TypeError with no diagnostic. All known webpack 5 builds will have addBlock here, but a guard would surface the problem more gracefully if that assumption ever breaks:
| module.addBlock!(block); | |
| if (module.addBlock) { | |
| module.addBlock(block); | |
| } |
Alternatively, emit a compilation.warnings.push(new webpack.WebpackError(...)) if the guard fails — consistent with how the plugin handles other unexpected-state cases.
Greptile SummaryThis PR replaces the vendored compiled JavaScript
Confidence Score: 4/5Safe to merge; the TypeScript port is line-faithful to the vendored plugin with only the documented, test-covered behavioral deviations. The only finding is a CHANGELOG link pointing to pull/75 instead of pull/87. The port is thorough and well-documented: all fork patches are present, the test suite is unchanged in expectations, and the integration tests run against the actual built artifact. The single issue is a mismatched PR number in the CHANGELOG footnote — no functional impact, but it should be corrected before the release entry is cut. CHANGELOG.md — the Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["src/WebpackPlugin.ts\n(public API wrapper)"] -->|"new RSCFlightWebpackPlugin()"| B["src/webpack/RSCWebpackPlugin.ts\n(TypeScript port — new)"]
B --> C["src/clientReferences.ts\nhasUseClientDirective()"]
B --> D["webpack public API\nModuleDependency · NullDependency\nTemplate · AsyncDependenciesBlock"]
subgraph "Before this PR"
E["src/WebpackPlugin.ts"] -->|"require()"| F["src/react-server-dom-webpack/cjs/\nreact-server-dom-webpack-plugin.js\n(vendored CJS — still shipped)"]
end
subgraph "Test wiring changes"
G["unit tests\n(client-ref-chunks, css-order,\nruntime-detection)"] -->|"import from TS source"| B
H["integration test\nrunWebpackWithPlugin.js"] -->|"require from dist/"| I["dist/webpack/RSCWebpackPlugin.js\n(built artifact)"]
end
Reviews (1): Last reviewed commit: "Point changelog entry at PR #87" | Re-trigger Greptile |
| dep.request, | ||
| ); | ||
| block.addDependency(dep); | ||
| module.addBlock!(block); |
There was a problem hiding this comment.
Potential runtime crash: non-null assertion on optional method
FlightModule.addDependency is typed as optional (addBlock?: (block: unknown) => void), so the ! operator silently bypasses the guard TypeScript set up. If a module type that lacks addBlock ever reaches this path (e.g. a DelegatedModule, external module, or a custom module type from a third-party plugin), this throws TypeError: module.addBlock is not a function with no diagnostic message.
| module.addBlock!(block); | |
| if (module.addBlock) module.addBlock(block); |
The parser hook only fires when isReactOnRailsRSCRuntimeResource returns true, so this path is narrow — but "narrow" doesn't mean "impossible" in a heterogeneous compilation, and a guarded no-op is far cheaper to debug than an unhandled TypeError.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| ## [Unreleased] | ||
|
|
||
| ### Changed | ||
| - Ported the webpack RSC plugin from the vendored built JavaScript artifact (`src/react-server-dom-webpack/cjs/react-server-dom-webpack-plugin.js`) to first-class TypeScript source at `src/webpack/RSCWebpackPlugin.ts`, preserving full behavior parity (server manifest emission, CSS/JS chunk scanning, runtime-chunk filtering, dependency-type chunk-group manifest construction with eager-import fallback, duplicate-package runtime detection, and hot-update CSS exclusion). The `./WebpackPlugin`, `./WebpackLoader`, and `./RSCReferenceDiscoveryPlugin` exports are unchanged; the vendored plugin file remains in the package but is no longer used by any export path. ([#87]) |
There was a problem hiding this comment.
#87 so the link points to the right pull request.
| - Ported the webpack RSC plugin from the vendored built JavaScript artifact (`src/react-server-dom-webpack/cjs/react-server-dom-webpack-plugin.js`) to first-class TypeScript source at `src/webpack/RSCWebpackPlugin.ts`, preserving full behavior parity (server manifest emission, CSS/JS chunk scanning, runtime-chunk filtering, dependency-type chunk-group manifest construction with eager-import fallback, duplicate-package runtime detection, and hot-update CSS exclusion). The `./WebpackPlugin`, `./WebpackLoader`, and `./RSCReferenceDiscoveryPlugin` exports are unchanged; the vendored plugin file remains in the package but is no longer used by any export path. ([#87]) | |
| - Ported the webpack RSC plugin from the vendored built JavaScript artifact (`src/react-server-dom-webpack/cjs/react-server-dom-webpack-plugin.js`) to first-class TypeScript source at `src/webpack/RSCWebpackPlugin.ts`, preserving full behavior parity (server manifest emission, CSS/JS chunk scanning, runtime-chunk filtering, dependency-type chunk-group manifest construction with eager-import fallback, duplicate-package runtime detection, and hot-update CSS exclusion). The `./WebpackPlugin`, `./WebpackLoader`, and `./RSCReferenceDiscoveryPlugin` exports are unchanged; the vendored plugin file remains in the package but is no longer used by any export path. ([#87]) |
| [19.0.5-rc.1]: https://github.com/shakacode/react_on_rails_rsc/releases/tag/19.0.5-rc.1 | ||
|
|
||
| [#52]: https://github.com/shakacode/react_on_rails_rsc/pull/52 | ||
| [#87]: https://github.com/shakacode/react_on_rails_rsc/pull/87 |
There was a problem hiding this comment.
| [#87]: https://github.com/shakacode/react_on_rails_rsc/pull/87 | |
| [#87]: https://github.com/shakacode/react_on_rails_rsc/pull/87 |
Code Review: Extract and own the webpack RSC plugin as TypeScript sourceOverviewA solid, well-documented port of the vendored CJS plugin to owned TypeScript. The parity checklist, zero fixture-expectation changes, and the codex/Claude/simplify gate history give high confidence the behavior is preserved. The shift to webpack's public API and the unified What's Working Well
Issues Found1. Non-null assertion on optional
|
| } | ||
| if (existing.css == null) existing.css = []; | ||
| for (const cssFile of cssFiles) { | ||
| if (existing.css.indexOf(cssFile) === -1) { |
There was a problem hiding this comment.
Linear scan in the merge path. The /simplify pass already converted the accumulation-side cssFiles to a Set; the same approach here would be O(1) per lookup:
| if (existing.css.indexOf(cssFile) === -1) { | |
| const existingCssSet = new Set(existing.css); | |
| for (const cssFile of cssFiles) { | |
| if (!existingCssSet.has(cssFile)) { | |
| existing.css.push(cssFile); | |
| } | |
| } |
Low priority for typical build sizes, but it aligns with the rest of the post-simplify code.
| normalModuleFactory.hooks.parser.for('javascript/auto').tap('HarmonyModulesPlugin', handler); | ||
| normalModuleFactory.hooks.parser.for('javascript/esm').tap('HarmonyModulesPlugin', handler); | ||
| normalModuleFactory.hooks.parser | ||
| .for('javascript/dynamic') | ||
| .tap('HarmonyModulesPlugin', handler); |
There was a problem hiding this comment.
Tap name 'HarmonyModulesPlugin' misleads diagnostics
These three taps are registered under a name that belongs to webpack's own HarmonyModulesPlugin. Webpack uses tap names in --profile output, hook tracing, and diagnostic messages — so any profiling or debug trace will attribute this plugin's parser work to the real HarmonyModulesPlugin, making performance and ordering issues harder to diagnose.
This is carried from the vendored CJS source (parity), but since we own this code now it's worth switching to PLUGIN_NAME:
| normalModuleFactory.hooks.parser.for('javascript/auto').tap('HarmonyModulesPlugin', handler); | |
| normalModuleFactory.hooks.parser.for('javascript/esm').tap('HarmonyModulesPlugin', handler); | |
| normalModuleFactory.hooks.parser | |
| .for('javascript/dynamic') | |
| .tap('HarmonyModulesPlugin', handler); | |
| normalModuleFactory.hooks.parser.for('javascript/auto').tap(PLUGIN_NAME, handler); | |
| normalModuleFactory.hooks.parser.for('javascript/esm').tap(PLUGIN_NAME, handler); | |
| normalModuleFactory.hooks.parser | |
| .for('javascript/dynamic') | |
| .tap(PLUGIN_NAME, handler); |
SyncHook tap order is insertion-order, not name-based, so this change is behaviorally safe.
| apply(compiler: webpack.Compiler): void { | ||
| const flightCompiler = compiler as unknown as FlightCompiler; | ||
| let resolvedClientReferences: ClientReferenceDependency[] | undefined; | ||
| let clientFileNameFound = false; |
There was a problem hiding this comment.
This flag is intentionally single-lifetime (set once, never reset between watch-mode compilations) — parity with the vendored plugin. Worth a short comment here so a future reader doesn't treat it as an overlooked bug:
| let clientFileNameFound = false; | |
| // Intentionally not reset between watch-mode compilations (parity with | |
| // the vendored plugin). Once the runtime module is seen in any build, | |
| // subsequent builds assume it is still present. | |
| let clientFileNameFound = false; |
| type FlightCompilation = { | ||
| dependencyFactories: Map<unknown, unknown>; | ||
| dependencyTemplates: Map<unknown, unknown>; | ||
| warnings: Error[]; |
There was a problem hiding this comment.
All call sites push webpack.WebpackError instances. Tightening the structural type from Error[] to webpack.WebpackError[] makes the contract more accurate at no cost:
| warnings: Error[]; | |
| warnings: webpack.WebpackError[]; |
| (err, resolvedDirectory) => { | ||
| if (err) return cb(err); | ||
| contextModuleFactory.resolveDependencies( | ||
| fs, | ||
| { | ||
| resource: resolvedDirectory as string, |
There was a problem hiding this comment.
Type-unsafe resolver cast: false can slip past the error guard
The resolver callback is (err: Error | null, result?: unknown) => void. Webpack resolvers can call back as (null, false) when the path doesn't resolve (non-error "not found"). The if (err) return cb(err) guard handles err !== null but lets resolvedDirectory === false fall through to the as string cast. Downstream this lands in contextModuleFactory.resolveDependencies as resource: false and later in path.join(false, dep.userRequest) (line 735), which produces "false/..." on some Node versions or throws on others.
| (err, resolvedDirectory) => { | |
| if (err) return cb(err); | |
| contextModuleFactory.resolveDependencies( | |
| fs, | |
| { | |
| resource: resolvedDirectory as string, | |
| (err, resolvedDirectory) => { | |
| if (err || typeof resolvedDirectory !== 'string') return cb(err ?? null); |
The same pattern covers undefined as well. The second occurrence on line 735 (path.join(resolvedDirectory as string, ...)) is inside the resolveDependencies callback that already ran, so fixing the outer guard is sufficient.
| const clientFileNameOnClient = require.resolve('../react-server-dom-webpack/client.browser.js'); | ||
| const clientFileNameOnServer = require.resolve('../react-server-dom-webpack/client.node.js'); | ||
|
|
||
| const runtimeResourceDetectionCache = new Map<string, boolean>(); |
There was a problem hiding this comment.
Module-level cache is never cleared: silent stale hits in watch mode
runtimeResourceDetectionCache lives at module scope, so it accumulates entries for the lifetime of the Node.js process. In watch mode a user could add or remove a duplicate react-on-rails-rsc install between rebuilds, and the cache would continue returning the stale result.
The PR body explicitly calls this out as a parity-preserving decision (the vendored plugin has the same module-level Map), which is a fair call for a porting PR. Flagging it here as a follow-up candidate: a per-compilation cache (set up in beforeCompile and cleared between runs) would be straightforward and eliminate the watch-mode blind spot without changing public behavior on single builds.
Code Review — PR #87: Extract webpack RSC plugin as TypeScript sourceOverviewThis is a well-executed porting PR. The parity checklist is thorough, the intentional behavioral deviations are documented with rationale, and the test wiring (unit tests against TS source, integration tests against the built dist artifact) gives solid confidence in correctness. The zero fixture-expectation changes are the strongest signal that behavior is preserved. FindingsI left four inline comments; summary below. Must-fix1. Non-null assertion on 2. Resolver Worth addressing3. Parser taps registered as Follow-up (not blocking)4. Module-level detection cache never cleared between watch-mode rebuilds (line 76) Notes on intentional deviationsThe two documented behavioral deltas look correct:
The SummaryTwo must-fix issues (non-null assertion crash risk, resolver |
Replace the vendored built ReactFlightWebpackPlugin (src/react-server-dom-webpack/cjs/react-server-dom-webpack-plugin.js) with a first-class TypeScript port at src/webpack/RSCWebpackPlugin.ts, mirroring the rspack plugin structure. The port preserves full behavior parity: server-build manifest emission, CSS-before-JS chunk scanning, runtime-chunk filtering, the #54 dependency-type chunk-group manifest algorithm with eager-import fallback and blocks-unavailable warning, the #52 runtime/hot-update CSS exclusions, #43 duplicate-package runtime detection, and #23 merge semantics (still required on the server path). Directive detection now reuses the shared hasUseClientDirective helper from clientReferences.ts. Public exports (./WebpackPlugin, ./WebpackLoader, ./RSCReferenceDiscoveryPlugin) are unchanged; the vendored plugin file stays in-tree and in dist but is no longer used by any export path. Test suites that exercised the vendored file now target the TS plugin (source for unit suites, dist for the child-process webpack runner), with zero fixture-expectation changes. Closes #56 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Remove the _this alias (all usages are in arrow callbacks), drop a redundant typeof-string guard before Array.isArray, use a Set for the cssFiles accumulator (same insertion-order dedup), and convert two index-unused C-style loops to for...of. Behavior-preserving; full suite re-verified. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
justin808
force-pushed
the
jg/56-extract-webpack-plugin-ts
branch
from
c9d7bf7 to
6d568aa
Compare
|
Rebased onto main after #85 (packed-tarball E2E suite) and #80 landed. Local validation on the rebased head 6d568aa: |
Code Review: Extract webpack RSC plugin as TypeScript sourceOverall assessment: Solid port with thorough documentation. The TypeScript source is well-typed, the JSDoc at the top traces every forked patch, and the parity checklist is unusually rigorous. A few specific issues and one potentially real bug are noted below — the rest is informational. Bug:
|
| dep.request, | ||
| ); | ||
| block.addDependency(dep); | ||
| module.addBlock!(block); |
There was a problem hiding this comment.
The non-null assertion ! is unsafe here. FlightModule.addBlock is typed as optional, so this crashes with TypeError: module.addBlock is not a function if any module that passes the isReactOnRailsRSCRuntimeResource check doesn't expose addBlock (e.g. a test mock or a future webpack version that renames the method).
| module.addBlock!(block); | |
| module.addBlock?.(block); |
Using optional chaining is a no-op when addBlock is present (the normal path) and safe when it is absent, which is all we need here.
| context: string, | ||
| contextResolver: FlightResolver, | ||
| normalResolver: FlightResolver, | ||
| fs: unknown, |
There was a problem hiding this comment.
The parameter name fs shadows the import * as fs from 'fs' at the top of the file. While it is not a bug (the module-level fs isn't referenced inside this method, and the parameter is cast to ReadFileFs before use), it is confusing and could mask accidental use of the wrong object.
| fs: unknown, | |
| inputFs: unknown, |
And update the cast on line 831 from (fs as ReadFileFs) to (inputFs as ReadFileFs). This matches the naming the rspack plugin uses for the equivalent parameter.
| apply(compiler: webpack.Compiler): void { | ||
| const flightCompiler = compiler as unknown as FlightCompiler; | ||
| let resolvedClientReferences: ClientReferenceDependency[] | undefined; | ||
| let clientFileNameFound = false; |
There was a problem hiding this comment.
clientFileNameFound is declared once in apply()'s closure and set to true the first time the Flight client runtime is parsed. In webpack watch mode it stays true for all subsequent compilations, so if the client runtime is removed or unreachable in a later watch run the "client runtime not found" warning is silently suppressed and the manifest is still emitted.
This is acknowledged as parity with the vendored plugin (PR description, finding #2). Since we now own the source, resetting the flag at the start of each compilation would be a cheap fix:
| let clientFileNameFound = false; | |
| let clientFileNameFound = false; | |
| flightCompiler.hooks.beforeCompile.tapAsync(PLUGIN_NAME, (_params, callback) => { | |
| clientFileNameFound = false; // reset per compilation for watch-mode correctness | |
| callback(); | |
| }); |
Leaving as-is is a reasonable call for an initial port, but worth tracking for the follow-up watch-mode hardening pass.
| } | ||
| if (existing.css == null) existing.css = []; | ||
| for (const cssFile of cssFiles) { | ||
| if (existing.css.indexOf(cssFile) === -1) { |
There was a problem hiding this comment.
Minor style nit: indexOf(...) === -1 can be written more clearly as includes.
| if (existing.css.indexOf(cssFile) === -1) { | |
| if (!existing.css.includes(cssFile)) { |
|
Review-thread triage (all inline threads are from advisory AI reviewers; none identify a confirmed blocker). Threads are left open for maintainer visibility; disposition for each:
|
…verification * origin/main: Harden dependency and Claude automation (#76) Make PR skill workflows discoverable (#88) Extract and own the webpack RSC plugin as TypeScript source (#87) Add packed-tarball E2E pipeline suite: webpack+rspack → Flight → SSR HTML → hydration (#85) Docs: Option 5 stock npm runtime go/no-go decision (#55 spike) (#80) # Conflicts: # yarn.lock
* origin/main: Add release artifact verification gate (#77) Apply React 19.0.7 RSC reply-decode DoS fixes (CVE-2026-23869, CVE-2026-23870) to vendored runtime (#86) Harden dependency and Claude automation (#76) Make PR skill workflows discoverable (#88) Extract and own the webpack RSC plugin as TypeScript source (#87) Add packed-tarball E2E pipeline suite: webpack+rspack → Flight → SSR HTML → hydration (#85) Docs: Option 5 stock npm runtime go/no-go decision (#55 spike) (#80)
…h-tests * origin/main: Add release artifact verification gate (#77) Apply React 19.0.7 RSC reply-decode DoS fixes (CVE-2026-23869, CVE-2026-23870) to vendored runtime (#86) Harden dependency and Claude automation (#76) Make PR skill workflows discoverable (#88) Extract and own the webpack RSC plugin as TypeScript source (#87) Add packed-tarball E2E pipeline suite: webpack+rspack → Flight → SSR HTML → hydration (#85) Docs: Option 5 stock npm runtime go/no-go decision (#55 spike) (#80)
* origin/main: Add compatibility matrix and canary signal (#83) Document runtime versioning policy and live backlog pointers (#79) Add Flight client error path coverage (#78) Document release dist-tag policy and artifact parity checks (#75) Allow artifact verifier to accept covering root peer ranges (#95) Add release artifact verification gate (#77) Apply React 19.0.7 RSC reply-decode DoS fixes (CVE-2026-23869, CVE-2026-23870) to vendored runtime (#86) Harden dependency and Claude automation (#76) Make PR skill workflows discoverable (#88) Extract and own the webpack RSC plugin as TypeScript source (#87) Add packed-tarball E2E pipeline suite: webpack+rspack → Flight → SSR HTML → hydration (#85) Docs: Option 5 stock npm runtime go/no-go decision (#55 spike) (#80) # Conflicts: # docs/releasing.md
…ase-motion * origin/main: Release from GitHub Actions with trusted publishing (#84) Add Claude agent setup and workflow skill stubs (#82) Add compatibility matrix and canary signal (#83) Document runtime versioning policy and live backlog pointers (#79) Add Flight client error path coverage (#78) Document release dist-tag policy and artifact parity checks (#75) Allow artifact verifier to accept covering root peer ranges (#95) Add release artifact verification gate (#77) Apply React 19.0.7 RSC reply-decode DoS fixes (CVE-2026-23869, CVE-2026-23870) to vendored runtime (#86) Harden dependency and Claude automation (#76) Make PR skill workflows discoverable (#88) Extract and own the webpack RSC plugin as TypeScript source (#87) Add packed-tarball E2E pipeline suite: webpack+rspack → Flight → SSR HTML → hydration (#85) Docs: Option 5 stock npm runtime go/no-go decision (#55 spike) (#80) # Conflicts: # CHANGELOG.md
1 participant
Closes #56
Summary
Ports the webpack React Server Components plugin from the vendored built JavaScript artifact (
src/react-server-dom-webpack/cjs/react-server-dom-webpack-plugin.js) to first-class TypeScript source atsrc/webpack/RSCWebpackPlugin.ts, mirroring the structure of the already-ownedsrc/react-server-dom-rspack/plugin.ts. The public wrapper insrc/WebpackPlugin.tsnow instantiates the TS plugin instead ofrequire-ing the vendored build."use client"directive detection reuses the sharedhasUseClientDirectivehelper fromsrc/clientReferences.ts.The vendored plugin file and all vendored RUNTIME files are untouched (runtime swap is #60). The vendored plugin still ships in
dist/(viaallowJscopy) but is no longer used by any export path — reverting this squash commit fully restores the old wiring.Parity checklist (every item ported and verified)
isServerbranch records every chunk group;react-server-client-manifest.jsondefault. Pinned bytests/react-flight-webpack-plugin-client-reference-chunks.test.ts("keeps generating server manifest metadata…") andtests/webpack-plugin/plugin-integration.test.ts("emits server manifest entries from the single merged server bundle").chunk.files(recordedJSflag). Pinned bytests/react-flight-webpack-plugin-css-order.test.ts(#44 regression tests).runtimeChunkFilesset fromentrypoint.getRuntimeChunk(); client JS/CSS filtered. Pinned by css-order + integration suites.ClientReferenceDependency(instanceof+type === 'client-reference'duplicate-copy fallback), unrecorded-files fallback loop with per-iteration pruning, warning texts byte-identical. Pinned bytests/webpack-plugin/plugin-integration.test.ts(eager-import, client-imports-client, multi-entry, split-shared fixtures) and the chunk-selection unit suite..hot-update.cssexclusion (client); server retains runtime CSSchunkResolvedClientFilesnarrowing + blocksIterable fallback + warning) is in the port;RSCReferenceDiscoveryPlugin/WebpackLoaderhalves were already TS and are unchanged.package.jsonwalk, memoized; static__internal_isReactOnRailsRSCRuntimeResourcekept. Pinned bytests/webpack-plugin-runtime-detection.test.ts(doppelganger-install cases).src/WebpackPlugin.tswrapper (DEFAULT_CLIENT_REFERENCES_INCLUDE/EXCLUDE), which is preserved verbatim; the inner plugin keeps the upstream default (`{directory: '.', include: /.(jsrecordModule(commit 8f09a6b) are still load-bearing: the server path records every chunk group, so a client reference appearing in several groups merges chunk lists (dedup by chunk id) and CSS (dedup by URL). Pinned by the existing unit test "dedupes CSS when merging a client reference recorded from several server chunk groups" and the client-path fallback loop. The merge code is therefore ported as-is, with a comment.Intentional, behavior-reviewed deviations
"use client"detection inresolveAllClientFilesnow uses the sharedhasUseClientDirective(acorn-loose,ecmaVersion: 'latest',allowHashBang: true, directive-terminator check) instead of the vendored inline copy (ecmaVersion: '2024', no hashbang support, no terminator check). The shared helper is the one already used byWebpackLoader/RSCReferenceDiscoveryPlugin/rspack plugin, so discovery is now consistent across all entry points. All fixtures pass unchanged.webpack.dependencies.ModuleDependency,webpack.dependencies.NullDependency,webpack.Template,webpack.AsyncDependenciesBlock) instead of deepwebpack/lib/*requires. Same classes, same runtime objects;tests/rspack-compat/static-analysis.test.tssanity check updated to document the new (still webpack-only) API usage.AsyncDependenciesBlockloc argument isundefinedinstead ofnull(both falsy; webpack stores it opaquely).Test wiring changes (no fixture-expectation changes)
tests/react-flight-webpack-plugin-client-reference-chunks.test.ts,tests/react-flight-webpack-plugin-css-order.test.ts,tests/webpack-plugin-runtime-detection.test.ts: import path switched from the vendored CJS file to../src/webpack/RSCWebpackPlugin(same assertions, same fixtures).tests/webpack-plugin/helpers/runWebpackWithPlugin.js: requiresdist/webpack/RSCWebpackPlugin.js(mirroringtests/rspack-plugin/helpers/runRspackWithPlugin.js, which already requires fromdist/), with the same "runyarn buildfirst" guard.tests/rspack-compat/static-analysis.test.ts: the "known-incompatible WebpackPlugin" sanity check now reads the TS source and asserts its webpack-only value-API usage.package.jsonbuild-if-needed: addsdist/webpack/RSCWebpackPlugin.jsto the artifact checks soprepare/prepackrebuild when it is missing.ZERO manifest/fixture expectation changes anywhere.
npm pack artifact diff (branch vs clean main clone, both
--dry-run --json --ignore-scriptsafter full build)dist/webpack/RSCWebpackPlugin.js/.js.map/.d.ts/.d.ts.mappackage.jsonexports,dependencies,peerDependencies: byte-identical (verified programmatically); onlyscripts.build-if-neededdiffers.Local validation
yarn build(tsc typecheck): clean.yarn test(both halves): 17 suites / 158 passed (non-RSC half) + all RSC suites passed, exit 0 — matches the green baseline on main.Review gates and Codex Decision Log
Codex review (
codex review --base origin/main, run from the worktree): completed with zero findings — "I did not identify any discrete, actionable correctness issues in the diff. The TypeScript port appears to preserve the vendored plugin behavior and the export path is updated consistently."Claude code-review pass (
claude -p '/code-review high'from the worktree, targeting the branch diff): 5 findings, all triaged, none confirmed as in-scope blockers:yarn build, integration tests will fail" — rejected (false).package.jsonpreparerunsbuild-if-neededonyarninstall (the CI install step), and this PR extendsbuild-if-neededto includedist/webpack/RSCWebpackPlugin.js. Empirically verified on a fresh clone:yarn install --frozen-lockfileemitted the build anddist/was fully populated. Precedent:tests/rspack-plugin/helpers/runRspackWithPlugin.jshas requireddist/on main with green CI since it landed.clientFileNameFoundnever reset between watch-mode compilations — rejected (pre-existing vendored behavior, parity-preserving by design). The vendored plugin has the identical single-assignment lifetime. Changing it would be a behavior change outside this port's scope.Map; the detection tests use uniquemkdtemppaths.publicPathcssPrefix gets no trailing slash — rejected (parity). IdenticalcssPrefix &&guard in the vendored code./simplify gate (
claude -p '/simplify origin/main' --model claude-opus-4-8 --max-budget-usd 20): ran to completion; 4 behavior-preserving cleanups accepted (removed_thisalias — all usages were inside arrow callbacks; dropped a redundanttypeof !== 'string'guard beforeArray.isArray;cssFilesarray →Setwith identical insertion-order dedup; two index-unused C-style loops →for...of). Speculative/out-of-scope suggestions (public-wrapper type dedup, include-regex unification) were rejected by the gate itself as behavior changes or public-API churn. Fullyarn build+yarn testre-run green after the edits (commit a83cc3f).Final-candidate codex re-review after the /simplify commit: skipped — the rerun was started but interrupted by a maintainer directive to land #56 ASAP (it blocks #60). The /simplify edits are mechanical and were manually verified hunk-by-hunk plus re-validated by the full suite; the substantive diff codex reviewed (zero findings) is unchanged.
Merge readiness evaluation
yarn buildclean;yarn testfully green both halves (17 suites / 158 tests non-RSC half + all RSC suites) with ZERO fixture-expectation changes; codex review zero findings; Claude review findings all triaged with evidence; /simplify applied and re-validated; npm pack diff shows no public API change.dist/artifact (exactly what ships).dist/, so a revert fully restores the previous wiring with no artifact gap.Note
Medium Risk
Large, behavior-sensitive change to webpack manifest generation on the critical RSC build path; risk is mitigated by a parity-focused port and existing unit/integration coverage, with small intentional deltas in directive detection and webpack public API usage.
Overview
Moves the webpack React Server Components plugin from the vendored CJS build (
react-server-dom-webpack-plugin.js) into owned TypeScript atsrc/webpack/RSCWebpackPlugin.ts, and wiressrc/WebpackPlugin.tsto instantiate that implementation instead ofrequire-ing the vendored file.Public surface is unchanged (
./WebpackPluginand related exports); the vendored plugin file remains in the package but is no longer on any export path. Build output addsdist/webpack/RSCWebpackPlugin.js, andbuild-if-needednow treats that artifact as required. Tests and the webpack integration helper load the new module (integration runs against the builtdist/file).The port is intended as behavior parity with the forked plugin (manifest emission, chunk/CSS scanning, runtime-chunk filtering, client-reference chunk groups, duplicate-install runtime detection).
"use client"discovery in the plugin now uses the sharedhasUseClientDirectivehelper instead of the vendored inline parser.Reviewed by Cursor Bugbot for commit 6d568aa. Bugbot is set up for automated code reviews on this repo. Configure here.