fix: scope RSC client reference discovery#3219
Conversation
|
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:
WalkthroughGenerator and webpack/rspack templates now define a scoped ChangesRSC clientReferences scoping
Sequence DiagramsequenceDiagram
participant Generator
participant File as "webpack config file"
participant Shakapacker
participant RSCPlugin as "RSCWebpackPlugin"
Generator->>File: open/read file
alt existing RSCWebpackPlugin found without clientReferences
Generator->>File: rewrite plugin constructor to add clientReferences
else no RSCWebpackPlugin
Generator->>File: insert imports for Shakapacker & path
Generator->>Shakapacker: use config.source_path
Generator->>File: insert rscClientReferences + new RSCWebpackPlugin({..., clientReferences})
end
File->>RSCPlugin: RSCWebpackPlugin configured with scoped clientReferences
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Greptile SummaryThis PR scopes the Confidence Score: 5/5Safe to merge — changes are consistent across all code paths, no logic errors or regressions identified. All six changed files are internally consistent: both template (.tt) and existing-config transform paths inject the same scoped rscClientReferences object, config is never double-imported (server template already has it; client template adds it only in the RSC block), and specs validate the new content. No P0 or P1 findings. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Generator invoked with RSC] --> B{New install or existing config?}
B -- New install --> C[Render .js.tt templates]
B -- Existing config --> D[update_webpack_configs_for_rsc]
C --> E[clientWebpackConfig.js.tt\nInjects: config from shakapacker\n resolve from path\n RSCWebpackPlugin\n rscClientReferences scoped to config.source_path]
C --> F[serverWebpackConfig.js.tt\nInjects: resolve from path\n RSCWebpackPlugin\n rscClientReferences scoped to config.source_path]
D --> G[update_client_webpack_config_for_rsc\ngsub: add RSCWebpackPlugin import +\n shakapacker config + rscClientReferences]
D --> H[update_server_webpack_config_for_rsc\ngsub: add RSCWebpackPlugin import +\n resolve + rscClientReferences]
E --> I[RSCWebpackPlugin with clientReferences\ndirectory: resolve config.source_path\nrecursive: true\ninclude: js,ts,jsx,tsx]
F --> I
G --> I
H --> I
I --> J[Webpack scans only source_path\nSkips vendor/bundle in CI]
Reviews (1): Last reviewed commit: "Scope RSC client references to app sourc..." | Re-trigger Greptile |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
Caution Review failedAn error occurred during the review process. Please try again later. WalkthroughThe PR resolves a webpack compilation issue where RSCWebpackPlugin's default recursive project root scan could inadvertently include vendored gem templates in CI environments. Changes include updating documentation, modifying webpack config generators and templates to explicitly compute and pass Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Code Review — PR #3219: Scope RSC client reference discoverySummaryCorrect fix for a real CI reliability issue. Scoping Issues1.
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Code ReviewGood, targeted fix. The root cause (unscoped Summary
Issues1. Test gap — webpack server config (not in diff, so inline comment not possible)
Suggested addition to the test at line 99: it "adds RSCWebpackPlugin to serverWebpackConfig" do
assert_file "config/webpack/serverWebpackConfig.js" do |content|
expect(content).to include("RSCWebpackPlugin")
expect(content).to include("react-on-rails-rsc/WebpackPlugin")
expect(content).to include("clientReferences: rscClientReferences") # add
expect(content).to include("directory: resolve(config.source_path)") # add
end
endSimilarly 2. Possible duplicate
|
![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: d49943e2f3
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08e341ea21
ℹ️ 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".
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Server config missing duplicate
resolveimport guard- The server RSC webpack transform now skips injecting the path resolve import when it already exists and has regression coverage for that case.
Or push these changes by commenting:
@cursor push f73f903f50
Preview (f73f903f50)
diff --git a/react_on_rails/lib/generators/react_on_rails/rsc_setup.rb b/react_on_rails/lib/generators/react_on_rails/rsc_setup.rb
--- a/react_on_rails/lib/generators/react_on_rails/rsc_setup.rb
+++ b/react_on_rails/lib/generators/react_on_rails/rsc_setup.rb
@@ -385,11 +385,11 @@
# Add RSCWebpackPlugin import after bundler require
server_injected_imports = [
"\\1",
- "const { resolve } = require('path');",
+ ("const { resolve } = require('path');" unless path_resolve_imported?(content)),
"const { RSCWebpackPlugin } = require('react-on-rails-rsc/WebpackPlugin');",
"",
rsc_client_references_js
- ].join("\n")
+ ].compact.join("\n")
gsub_file(
config_path,
diff --git a/react_on_rails/spec/react_on_rails/generators/rsc_generator_spec.rb b/react_on_rails/spec/react_on_rails/generators/rsc_generator_spec.rb
--- a/react_on_rails/spec/react_on_rails/generators/rsc_generator_spec.rb
+++ b/react_on_rails/spec/react_on_rails/generators/rsc_generator_spec.rb
@@ -199,6 +199,38 @@
end
end
+ context "when the server webpack config already imports path resolve" do
+ before(:all) do
+ prepare_destination
+ simulate_existing_rails_files(package_json: true)
+ simulate_npm_files(package_json: true)
+ simulate_existing_file("config/initializers/react_on_rails_pro.rb", <<~RUBY)
+ ReactOnRailsPro.configure do |config|
+ config.server_renderer = "NodeRenderer"
+ end
+ RUBY
+ simulate_existing_file("Procfile.dev", "rails: bin/rails s\n")
+ simulate_pro_webpack_files
+ simulate_existing_file(
+ "config/webpack/serverWebpackConfig.js",
+ "const { resolve } = require('path');\n#{pro_server_webpack_content}"
+ )
+
+ Dir.chdir(destination_root) do
+ run_generator(["--force"])
+ end
+ end
+
+ it "does not inject a duplicate resolve import" do
+ assert_file "config/webpack/serverWebpackConfig.js" do |content|
+ expect(content.scan("const { resolve } = require('path');").length).to eq(1)
+ expect(content).to include("clientReferences: rscClientReferences")
+ expect(content).to include("directory: resolve(config.source_path)")
+ expect(content).to include("isServer: true")
+ end
+ end
+ end
+
context "when Pro is installed with a canonical legacy hello_world layout" do
before(:all) do
prepare_destinationYou can send follow-ups to the cloud agent here.
Code Review: PR #3219 — Scope RSC client reference discoveryOverviewThis PR correctly fixes the root cause of issue #3201: What's Good
Issues & SuggestionsBug: Server config injection lacks the
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 43a87cc3ce
ℹ️ 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".
Code ReviewOverviewThis PR fixes a real CI bug (#3201) where the RSCWebpackPlugin, when The approach is sound: generate the scoped references once at the module level, share them between server and client plugin calls, guard against duplicate imports when rewriting existing configs, and document the risk clearly. The new test matrix is comprehensive. Issues & SuggestionsBug risk:
|
Code ReviewOverviewThis PR scopes RSC webpack plugin's client-reference scanning to The core approach — computing Issues FoundExisting apps with old generator output are not auto-upgradedBoth return if content.include?("RSCWebpackPlugin")This means users who previously ran the generator (which produced Aliased Shakapacker import causes a duplicate
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 184460aa25
ℹ️ 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".
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Partial gsub can produce undefined variable reference
- Guarded the plugin rewrite on successful client-reference setup and added verification/tests so customized configs warn instead of receiving undefined references.
Or push these changes by commenting:
@cursor push 39dc680411
Preview (39dc680411)
diff --git a/react_on_rails/lib/generators/react_on_rails/rsc_setup.rb b/react_on_rails/lib/generators/react_on_rails/rsc_setup.rb
--- a/react_on_rails/lib/generators/react_on_rails/rsc_setup.rb
+++ b/react_on_rails/lib/generators/react_on_rails/rsc_setup.rb
@@ -489,6 +489,7 @@
missing = []
missing << "RSCWebpackPlugin in serverWebpackConfig.js" unless content.include?("RSCWebpackPlugin")
missing << "rscBundle parameter in serverWebpackConfig.js" unless content.include?("rscBundle")
+ missing.concat(check_rsc_client_references(content, "serverWebpackConfig.js"))
missing
end
@@ -497,7 +498,10 @@
return [] unless File.exist?(path)
content = File.read(path)
- content.include?("RSCWebpackPlugin") ? [] : ["RSCWebpackPlugin in clientWebpackConfig.js"]
+ missing = []
+ missing << "RSCWebpackPlugin in clientWebpackConfig.js" unless content.include?("RSCWebpackPlugin")
+ missing.concat(check_rsc_client_references(content, "clientWebpackConfig.js"))
+ missing
end
def check_rsc_scob_config
@@ -521,7 +525,7 @@
def update_existing_rsc_webpack_config(config_path, content, is_server:)
return if content.include?("clientReferences: rscClientReferences")
- add_rsc_client_references_setup(config_path, content, is_server: is_server)
+ return unless add_rsc_client_references_setup(config_path, content, is_server: is_server)
gsub_file(
config_path,
@@ -549,9 +553,25 @@
%r{(const commonWebpackConfig = require\('\./commonWebpackConfig'\);)}
end
+ return false unless content.match?(import_pattern)
+
gsub_file(config_path, import_pattern, injected_imports)
+ true
end
+ def check_rsc_client_references(content, filename)
+ return [] unless content.include?("RSCWebpackPlugin")
+
+ missing = []
+ unless content.include?("clientReferences: rscClientReferences")
+ missing << "clientReferences option in #{filename}"
+ end
+ if content.include?("clientReferences: rscClientReferences") && !content.include?("const rscClientReferences =")
+ missing << "rscClientReferences definition in #{filename}"
+ end
+ missing
+ end
+
def shakapacker_config_imported?(content)
commonjs_named_imported?(content, "shakapacker", "config")
end
diff --git a/react_on_rails/spec/react_on_rails/generators/rsc_generator_spec.rb b/react_on_rails/spec/react_on_rails/generators/rsc_generator_spec.rb
--- a/react_on_rails/spec/react_on_rails/generators/rsc_generator_spec.rb
+++ b/react_on_rails/spec/react_on_rails/generators/rsc_generator_spec.rb
@@ -303,6 +303,68 @@
end
end
+ context "when existing RSC webpack configs have customized import anchors" do
+ before(:all) do
+ prepare_destination
+ simulate_existing_rails_files(package_json: true)
+ simulate_npm_files(package_json: true)
+ simulate_existing_file("config/initializers/react_on_rails_pro.rb", <<~RUBY)
+ ReactOnRailsPro.configure do |config|
+ config.server_renderer = "NodeRenderer"
+ end
+ RUBY
+ simulate_existing_file("Procfile.dev", "rails: bin/rails s\n")
+ simulate_pro_webpack_files
+ simulate_existing_file("config/webpack/serverWebpackConfig.js", <<~JS)
+ const { RSCWebpackPlugin } = require('react-on-rails-rsc/WebpackPlugin');
+ const webpack = require('webpack');
+
+ const configureServer = (rscBundle = false) => {
+ const serverWebpackConfig = { plugins: [] };
+
+ if (!rscBundle) {
+ serverWebpackConfig.plugins.push(new RSCWebpackPlugin({ isServer: true }));
+ }
+
+ return serverWebpackConfig;
+ };
+
+ module.exports = { default: configureServer };
+ JS
+ simulate_existing_file("config/webpack/clientWebpackConfig.js", <<~JS)
+ const { RSCWebpackPlugin } = require('react-on-rails-rsc/WebpackPlugin');
+
+ const configureClient = () => {
+ const clientConfig = { plugins: [] };
+ clientConfig.plugins.push(new RSCWebpackPlugin({ isServer: false }));
+
+ return clientConfig;
+ };
+
+ module.exports = configureClient;
+ JS
+
+ Dir.chdir(destination_root) do
+ run_generator(["--force"])
+ end
+ end
+
+ it "does not add undefined rscClientReferences references" do
+ assert_file "config/webpack/serverWebpackConfig.js" do |content|
+ expect(content).not_to include("clientReferences: rscClientReferences")
+ expect(content).not_to include("const rscClientReferences =")
+ end
+ assert_file "config/webpack/clientWebpackConfig.js" do |content|
+ expect(content).not_to include("clientReferences: rscClientReferences")
+ expect(content).not_to include("const rscClientReferences =")
+ end
+
+ warning_text = GeneratorMessages.messages.join("\n")
+ expect(warning_text).to include("clientReferences option in serverWebpackConfig.js")
+ expect(warning_text).to include("clientReferences option in clientWebpackConfig.js")
+ end
+ end
+
context "when the client webpack config uses aliased imports" do
before(:all) do
prepare_destinationYou can send follow-ups to the cloud agent here.
Code Review — PR #3219: Scope RSC client reference discoveryOverviewThis PR fixes a real CI problem (#3201) cleanly: by scoping I found four issues worth addressing before merge — one is a silent failure path. Issues1. Silent no-op when user has a customised
|
Code Review - PR 3219: fix: scope RSC client reference discoveryOverviewThis PR fixes a real production issue: without a scoped The bulk of the change is a new ~1,300-line lightweight JavaScript parser ( Strengths
Issues Found1. Predicate with side effects in Named as a predicate but its own comment says the predicate also prepares the rewrite -- calling 2.
3. The name implies it operates only on a plugin options subsection, but 4. The 5. Minor: When no parseable SummaryThe core fix is solid and the edge-case coverage is excellent. Main actionable items: (1) rename or document the side-effecting |
|
View the full report in the job summary. |
|
@/var/folders/4n/m5j7lmks4mx8rt8x51pgw7t80000gn/T/tmp.gI5v401yKq |
Follow-up to PR #3219 cloud review. Internal cleanup, no behavior change. - Rename `rsc_plugin_without_client_references?` to `any_rsc_plugin_missing_client_references?` and add a docstring spelling out the any-section existential semantics — the prior name read as universal but the implementation is existential. - Consolidate the lightweight JS scanner's supported-surface notes onto `advance_js_scan_state` (the central dispatcher): which lexical constructs are tracked, which fall outside (regex literals, nested template literals), how downstream `rsc_plugin_option_sections_partition` detects the unsafe cases and warns, and what real-world expansion would look like. Replace the scattered shorter notes at `rsc_plugin_options_without_comments` and `matching_js_closing_brace` with pointers to the consolidated doc. - Document the defensive guards in `add_rsc_client_references_setup`: what bad outcome they prevent (a duplicate `const rscClientReferences` declaration → `Identifier already declared` SyntaxError at config load) and why two boolean checks are worth keeping in case a future caller bypasses `ensure_rsc_client_references_setup`. Fixes #3258 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Follow-up to PR #3219 cloud review. Internal cleanup, no behavior change. - Rename `rsc_plugin_without_client_references?` to `any_rsc_plugin_missing_client_references?` and add a docstring spelling out the any-section existential semantics — the prior name read as universal but the implementation is existential. - Consolidate the lightweight JS scanner's supported-surface notes onto `advance_js_scan_state` (the central dispatcher): which lexical constructs are tracked, which fall outside (regex literals, nested template literals), how downstream `rsc_plugin_option_sections_partition` detects the unsafe cases and warns, and what real-world expansion would look like. Replace the scattered shorter notes at `rsc_plugin_options_without_comments` and `matching_js_closing_brace` with pointers to the consolidated doc. - Document the defensive guards in `add_rsc_client_references_setup`: what bad outcome they prevent (a duplicate `const rscClientReferences` declaration → `Identifier already declared` SyntaxError at config load) and why two boolean checks are worth keeping in case a future caller bypasses `ensure_rsc_client_references_setup`. Fixes #3258 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Follow-up to PR #3219 cloud review. Internal cleanup, no behavior change. - Rename `rsc_plugin_without_client_references?` to `any_rsc_plugin_missing_client_references?` and add a docstring spelling out the any-section existential semantics — the prior name read as universal but the implementation is existential. - Consolidate the lightweight JS scanner's supported-surface notes onto `advance_js_scan_state` (the central dispatcher): which lexical constructs are tracked, which fall outside (regex literals, nested template literals), how downstream `rsc_plugin_option_sections_partition` detects the unsafe cases and warns, and what real-world expansion would look like. Replace the scattered shorter notes at `rsc_plugin_options_without_comments` and `matching_js_closing_brace` with pointers to the consolidated doc. - Document the defensive guards in `add_rsc_client_references_setup`: what bad outcome they prevent (a duplicate `const rscClientReferences` declaration → `Identifier already declared` SyntaxError at config load) and why two boolean checks are worth keeping in case a future caller bypasses `ensure_rsc_client_references_setup`. Fixes #3258 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Design spec for the three follow-up clarity items from PR #3219's cloud review: rename the per-section predicate, document the scanner's supported surface around regex-literal braces, and drop dead defensive guards in the helper-injection method. No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Design spec for the three follow-up clarity items from PR #3219's cloud review: rename the per-section predicate, document the scanner's supported surface around regex-literal braces, and drop dead defensive guards in the helper-injection method. No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Follow-up to PR #3219 cloud review. Internal cleanup, no behavior change. - Rename `rsc_plugin_without_client_references?` to `any_rsc_plugin_missing_client_references?` and add a docstring spelling out the any-section existential semantics — the prior name read as universal but the implementation is existential. - Consolidate the lightweight JS scanner's supported-surface notes onto `advance_js_scan_state` (the central dispatcher): which lexical constructs are tracked, which fall outside (regex literals, nested template literals), how downstream `rsc_plugin_option_sections_partition` detects the unsafe cases and warns, and what real-world expansion would look like. Replace the scattered shorter notes at `rsc_plugin_options_without_comments` and `matching_js_closing_brace` with pointers to the consolidated doc. - Document the defensive guards in `add_rsc_client_references_setup`: what bad outcome they prevent (a duplicate `const rscClientReferences` declaration → `Identifier already declared` SyntaxError at config load) and why two boolean checks are worth keeping in case a future caller bypasses `ensure_rsc_client_references_setup`. Fixes #3258 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Follow-up to PR #3219 cloud review. Internal cleanup, no behavior change. - Rename `rsc_plugin_without_client_references?` to `any_rsc_plugin_missing_client_references?` and add a docstring spelling out the any-section existential semantics — the prior name read as universal but the implementation is existential. - Consolidate the lightweight JS scanner's supported-surface notes onto `advance_js_scan_state` (the central dispatcher): which lexical constructs are tracked, which fall outside (regex literals, nested template literals), how downstream `rsc_plugin_option_sections_partition` detects the unsafe cases and warns, and what real-world expansion would look like. Replace the scattered shorter notes at `rsc_plugin_options_without_comments` and `matching_js_closing_brace` with pointers to the consolidated doc. - Document the defensive guards in `add_rsc_client_references_setup`: what bad outcome they prevent (a duplicate `const rscClientReferences` declaration → `Identifier already declared` SyntaxError at config load) and why two boolean checks are worth keeping in case a future caller bypasses `ensure_rsc_client_references_setup`. Fixes #3258 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Design spec for the three follow-up clarity items from PR #3219's cloud review: rename the per-section predicate, document the scanner's supported surface around regex-literal braces, and drop dead defensive guards in the helper-injection method. No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Summary Follow-up to #3219 / fixes #3258. This PR is now rebased onto current `origin/main`. ### RSC migration parser cleanup - Rename `rsc_plugin_without_client_references?` to `any_rsc_plugin_missing_client_references?` and document the existential semantics. - Consolidate lightweight JS scanner support/limitation notes onto `advance_js_scan_state`, including caller coverage and regex/template-literal caveats. - Clarify why `add_rsc_client_references_setup` keeps defensive duplicate-helper guards. ### Adjacent hardening - Strengthen Node renderer `SECURITY:` comments for CommonJS wrapping via both `additionalContext` and `supportModules`. - Make the package-json publish rewrite use a same-directory temp file plus atomic rename before marking the file as changed. - Add direct coverage for the package-json pin fallback warning formatter. - Keep the retry-sleep guard in release metadata lookup explicit and make the webpack helper test description path-agnostic. ## Review Follow-Ups - **Must-fix:** none. Thread-aware review scan currently shows 0 unresolved review threads. - **Optional fixed:** added the direct `supportModules` security anchor and the `package_json_pin_fallback_warning` formatter spec. - **Discuss / advice:** the prior `Generator tests / examples (3.2, minimum)` failure was caused by `npm install shakapacker@10.1.0` returning `ETARGET` before that npm version was available. The registry now resolves `shakapacker@10.1.0`, and the React 16 pinned example task passed locally. - **Discuss / non-code CI:** the current `claude-review` failure is the Claude Action hitting its weekly limit (`resets May 29, 3am UTC`), not a repository test failure. ## CI / Test Plan - [x] Rebased onto `origin/main` - [x] `npm view shakapacker@10.1.0 version` - [x] `(cd react_on_rails && bundle exec rake run_rspec:shakapacker_examples_react16)` - [x] `(cd react_on_rails && BUNDLE_FORCE_RUBY_PLATFORM=true bundle exec rspec spec/react_on_rails/shakapacker_examples_rake_spec.rb spec/react_on_rails/release_rake_helpers_spec.rb spec/react_on_rails/generators/js_dependency_manager_spec.rb)` - [x] Pre-push hook: branch Ruby RuboCop on changed Ruby files <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes RSC config auto-migration control flow and npm publish package.json handling; mistakes could corrupt webpack configs or leave bad publish manifests, though new specs cover the release path. > > **Overview** > This PR tightens **RSC webpack config migration** logic and documents **Node renderer sandboxing**, plus small **release** and **generator** hardening. > > **RSC clientReferences migration** renames `rsc_plugin_without_client_references?` to **`any_rsc_plugin_missing_client_references?`** and documents that it is an existential check (not the complement of “all plugins define clientReferences”). **`prepare_rsc_client_references_rewrite!`** replaces the old prepare helper so helper injection and rewrite decisions stay correct when some plugins already use scoped `clientReferences`. **`rsc_plugin_sections_safe_to_rewrite?`** now takes **`is_server:`** when counting unparseable sections. Lightweight JS scanner limits and caller contracts are **centralized on `advance_js_scan_state`**, with extra comments on defensive duplicate-helper guards. > > **Node renderer config** comments add explicit **SECURITY** notes that **`supportModules: true`** and a plain-object **`additionalContext`** both enable CommonJS wrapping and host **`require`**. > > **npm publish** rewrites `package.json` via a **same-directory temp file and atomic rename**, sets the restore **`changed`** flag only after rename succeeds, and adds a spec for failed rename cleanup. > > **JS dependency fallback** emits a clearer warning listing **`name@version`** pins written to `package.json` when install fails. The webpack helper test description is made path-agnostic. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 92f41ed. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * More robust npm publishing via atomic package.json replacement. * Improved React Server Components setup handling for multi-plugin configurations. * **Documentation** * Expanded security notes for supportModules and sandboxing behavior. * **Improvements** * Clearer, better-formatted package manager warning messages. * **Tests** * Updated/added tests and path-agnostic test descriptions for related warnings. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/shakacode/react_on_rails/pull/3343?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
](https://app.coderabbit.ai/change-stack/shakacode/react_on_rails/pull/3343?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
Design spec for the three follow-up clarity items from PR #3219's cloud review: rename the per-section predicate, document the scanner's supported surface around regex-literal braces, and drop dead defensive guards in the helper-injection method. No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 participant
Summary
Fixes #3201
Test plan
Note
Medium Risk
Touches RSC webpack generator migrations and CI build discovery; incorrect rewrites could break builds, but scope is limited to Pro RSC webpack setup, not security-critical runtime paths.
Overview
RSC webpack configs now limit client-reference scanning to app source instead of the project root. New and migrated setups inject
rscClientReferenceswithdirectory: resolve(config.source_path)and passclientReferences: rscClientReferencesintoRSCWebpackPluginon both client and server bundles, avoiding CI failures from scanning paths likevendor/bundle.The
react_on_rails:rscgenerator was refactored: webpack migration logic lives inrsc_setup/client_references.rb(JS-aware parsing, scoped rewrites, rollbacks, degraded paths when anchors/ESM block automation), and Hello Server layout selection moved torsc_setup/layouts.rb. Doctor/verify now requires scopedclientReferenceswhen the plugin is already present. Docs explain the CI risk, ESM/manual migration, and upgrade steps for apps that previously passed without scoping.Risk: Medium — changes only affect RSC webpack generator output and build-time file discovery, not runtime auth or data paths.
Reviewed by Cursor Bugbot for commit eff6ffa. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Documentation
Bug Fixes
Tests