Skip to content

Commit 37dd255

Browse files
authored
Clarify RSC client references rewrite flow (#3458)
## Summary - Split RSC client-reference setup preparation from the read-only rewrite-needed predicate. - Make `rsc_plugin_sections_safe_to_rewrite?` explicitly file-wide by removing the misleading `is_server:` keyword. - Add generator specs for both no-file-write predicate branches and the target-independent safety check. Fixes #3434. ## Test plan - `bundle exec rspec react_on_rails/spec/react_on_rails/generators/rsc_generator_spec.rb:2051 react_on_rails/spec/react_on_rails/generators/rsc_generator_spec.rb:2116 react_on_rails/spec/react_on_rails/generators/rsc_generator_spec.rb:2148` - `bundle exec rspec react_on_rails/spec/react_on_rails/generators/rsc_generator_spec.rb` - `(cd react_on_rails && bundle exec rubocop)` ## Notes - No changelog entry: this is an internal generator maintainability cleanup with behavior intended to stay unchanged. - Addressed all Claude optional review comments in follow-up commit `2c6074c6a`. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Refactored RSC setup logic to separate preparation/ensuring from the rewrite-decision step and simplified the safety-check behavior. * **Tests** * Added coverage verifying the new predicate does not modify files and validates both rewrite-needed outcomes. Note: Internal changes only; no user-facing impact. <!-- review_stack_entry_start --> [![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/shakacode/react_on_rails/pull/3458?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 -->
1 parent a2c74f6 commit 37dd255

2 files changed

Lines changed: 68 additions & 12 deletions

File tree

react_on_rails/lib/generators/react_on_rails/rsc_setup/client_references.rb

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -121,36 +121,41 @@ def js_line_ending(content)
121121
end
122122

123123
def update_existing_rsc_webpack_config(config_path, content, is_server:)
124-
return unless rsc_plugin_sections_safe_to_rewrite?(config_path, content, is_server: is_server)
124+
return unless rsc_plugin_sections_safe_to_rewrite?(config_path, content)
125125
return if rsc_plugin_uses_scoped_client_references?(content, is_server: is_server)
126-
return unless rsc_client_references_rewrite_needed?(config_path, content, is_server: is_server)
126+
127+
# May inject the scoped helper before the rewrite step re-reads the config from disk.
128+
return unless prepare_rsc_client_references_setup(config_path, content, is_server: is_server)
129+
return unless rsc_plugin_needs_client_references_rewrite?(content, is_server: is_server)
127130

128131
return if rewrite_rsc_plugin_client_references(config_path, is_server: is_server)
129132

130133
rollback_incomplete_rsc_client_references_setup(config_path, content)
131134
warn_missing_rsc_plugin_target(config_path, is_server: is_server)
132135
end
133136

134-
def rsc_client_references_rewrite_needed?(config_path, content, is_server:)
135-
# This predicate prepares the rewrite too: when it returns true, the scoped helper
136-
# may already have been injected on disk so `rewrite_rsc_plugin_client_references`
137-
# can re-read fresh content with valid offsets.
137+
def prepare_rsc_client_references_setup(config_path, content, is_server:)
138138
if rsc_plugin_references_any_scoped_client_references?(content, is_server: is_server)
139-
return false unless ensure_rsc_client_references_setup(config_path, content, is_server: is_server)
140-
141-
return rsc_plugin_without_client_references?(content, is_server: is_server)
139+
return ensure_rsc_client_references_setup(config_path, content, is_server: is_server)
142140
end
143141

144142
rewritable_rsc_plugin?(config_path, content, is_server: is_server) &&
145143
ensure_rsc_client_references_setup(config_path, content, is_server: is_server)
146144
end
147145

146+
def rsc_plugin_needs_client_references_rewrite?(content, is_server:)
147+
rsc_plugin_without_client_references?(content, is_server: is_server)
148+
end
149+
148150
# Detects RSCWebpackPlugin option blocks that the lightweight JS scanner could not parse
149151
# cleanly (most often a regex literal with an unmatched `{` / `}` that walks the depth
150152
# counter past the real closing brace). When found, we warn and refuse to rewrite anything
151153
# in the file so a sibling rewrite cannot accidentally splice into a wrong location.
152-
def rsc_plugin_sections_safe_to_rewrite?(config_path, content, is_server:)
153-
unparseable = rsc_plugin_option_sections_partition(content, is_server: is_server).fetch(:unparseable)
154+
def rsc_plugin_sections_safe_to_rewrite?(config_path, content)
155+
# The unparseable count is file-wide: the partition increments it for every invocation
156+
# that cannot be parsed, regardless of the `is_server` argument. The argument only
157+
# filters the target-specific safe bucket, which is ignored here.
158+
unparseable = rsc_plugin_option_sections_partition(content, is_server: true).fetch(:unparseable)
154159
return true if unparseable.zero?
155160

156161
warn_unparseable_rsc_plugin_sections(config_path, unparseable)

react_on_rails/spec/react_on_rails/generators/rsc_generator_spec.rb

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2069,7 +2069,7 @@
20692069
)
20702070
content = File.read(File.join(destination_root, config_path))
20712071

2072-
expect(generator.send(:rsc_plugin_sections_safe_to_rewrite?, config_path, content, is_server: true))
2072+
expect(generator.send(:rsc_plugin_sections_safe_to_rewrite?, config_path, content))
20732073
.to be(false)
20742074

20752075
messages = GeneratorMessages.messages.join("\n")
@@ -2113,6 +2113,57 @@
21132113
expect(false_partition.fetch(:safe).length).to eq(1)
21142114
end
21152115

2116+
it "keeps the client references rewrite predicate free of file writes" do
2117+
config_path = "config/webpack/clientWebpackConfig.js"
2118+
simulate_existing_file(
2119+
config_path,
2120+
<<~JS
2121+
const commonWebpackConfig = require('./commonWebpackConfig');
2122+
const { RSCWebpackPlugin } = require('react-on-rails-rsc/WebpackPlugin');
2123+
2124+
const configureClient = () => {
2125+
const clientConfig = commonWebpackConfig();
2126+
clientConfig.plugins.push(
2127+
new RSCWebpackPlugin({
2128+
isServer: false,
2129+
clientReferences: rscClientReferences,
2130+
}),
2131+
);
2132+
2133+
return clientConfig;
2134+
};
2135+
2136+
module.exports = configureClient;
2137+
JS
2138+
)
2139+
2140+
full_path = File.join(destination_root, config_path)
2141+
content = File.read(full_path)
2142+
2143+
expect(generator.send(:rsc_plugin_needs_client_references_rewrite?, content, is_server: false))
2144+
.to be(false)
2145+
expect(File.read(full_path)).to eq(content)
2146+
end
2147+
2148+
it "returns true from the client references rewrite predicate without file writes" do
2149+
config_path = "config/webpack/clientWebpackConfig.js"
2150+
simulate_existing_file(
2151+
config_path,
2152+
<<~JS
2153+
const { RSCWebpackPlugin } = require('react-on-rails-rsc/WebpackPlugin');
2154+
2155+
clientConfig.plugins.push(new RSCWebpackPlugin({ isServer: false }));
2156+
JS
2157+
)
2158+
2159+
full_path = File.join(destination_root, config_path)
2160+
content = File.read(full_path)
2161+
2162+
expect(generator.send(:rsc_plugin_needs_client_references_rewrite?, content, is_server: false))
2163+
.to be(true)
2164+
expect(File.read(full_path)).to eq(content)
2165+
end
2166+
21162167
it "does not inject duplicate imports for legacy let or var CommonJS destructuring" do
21172168
config_path = "config/webpack/clientWebpackConfig.js"
21182169
simulate_existing_file(

0 commit comments

Comments
 (0)