Skip to content

Commit bb7be6f

Browse files
justin808claude
andcommitted
refactor: clarify RSC migration parser helpers
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>
1 parent bcc6bae commit bb7be6f

1 file changed

Lines changed: 69 additions & 21 deletions

File tree

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

Lines changed: 69 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ def rsc_client_references_rewrite_needed?(config_path, content, is_server:)
138138
if rsc_plugin_references_any_scoped_client_references?(content, is_server: is_server)
139139
return false unless ensure_rsc_client_references_setup(config_path, content, is_server: is_server)
140140

141-
return rsc_plugin_without_client_references?(content, is_server: is_server)
141+
return any_rsc_plugin_missing_client_references?(content, is_server: is_server)
142142
end
143143

144144
rewritable_rsc_plugin?(config_path, content, is_server: is_server) &&
@@ -160,7 +160,7 @@ def rsc_plugin_sections_safe_to_rewrite?(config_path, content, is_server:)
160160
def rewritable_rsc_plugin?(config_path, content, is_server:)
161161
# Mixed same-target plugins are still rewritable: the later rewrite only updates plugins
162162
# missing clientReferences and leaves sibling custom clientReferences untouched.
163-
return true if rsc_plugin_without_client_references?(content, is_server: is_server)
163+
return true if any_rsc_plugin_missing_client_references?(content, is_server: is_server)
164164

165165
if rsc_plugin_defines_client_references?(content, is_server: is_server)
166166
GeneratorMessages.add_warning(
@@ -258,7 +258,12 @@ def rsc_plugin_defines_client_references?(content, is_server:)
258258
end
259259
end
260260

261-
def rsc_plugin_without_client_references?(content, is_server:)
261+
# Existential check: returns true when at least one matching plugin section is missing a
262+
# top-level `clientReferences:` key. Pairs with `rsc_plugin_defines_client_references?`,
263+
# which uses the same any-section semantics for the opposite condition. The two are not
264+
# complements when multiple plugin sections exist — a file with one configured plugin and
265+
# one unconfigured plugin returns true from both.
266+
def any_rsc_plugin_missing_client_references?(content, is_server:)
262267
rsc_plugin_option_sections(content, is_server: is_server).any? do |section|
263268
!rsc_plugin_body_has_top_level_key?(section.fetch(:body), "clientReferences")
264269
end
@@ -268,8 +273,8 @@ def rsc_plugin_without_client_references?(content, is_server:)
268273
# so `clientReferences:` / `isServer:` substrings inside strings are not mis-detected.
269274
# Shares the `advance_js_scan_state` family used by `js_top_level_position?` and
270275
# `matching_js_closing_brace` so all JS-aware passes follow the same comment/string rules.
271-
# Regex literals (e.g. `/a{2}/`) are still outside this scanner's supported surface
272-
# because brace quantifiers can confuse `matching_js_closing_brace`'s depth counter.
276+
# See `advance_js_scan_state` for the scanner's supported surface (including the regex-
277+
# literal and nested-template-literal limits that callers must be aware of).
273278
def rsc_plugin_options_without_comments(options)
274279
result = String.new(capacity: options.length)
275280
state = nil
@@ -429,15 +434,12 @@ def first_significant_js_index(content, start_index)
429434
end
430435

431436
# Expects `content[open_index] == "{"`; callers pass the options-object opening brace.
432-
# This lightweight scanner treats template literals as opaque strings (backtick to backtick).
433-
# Simple `${...}` expressions are handled correctly: while in the backtick state every
434-
# character — including `{` and `}` inside the expression — is consumed as string content
435-
# and never reaches the depth counter. The real unsupported case is *nested* template
436-
# literals (e.g. `` `outer ${`inner`}` ``) where the inner backtick falsely closes the outer
437-
# string state, exposing later braces to the depth counter. Callers detect that via
438-
# `rsc_plugin_options_followed_by_close_paren?` and mark the section unparseable rather
439-
# than producing a corrupt rewrite. Regex literals are outside this scanner's supported
440-
# surface for the same reason.
437+
# See `advance_js_scan_state` for the scanner's supported surface — in short, simple
438+
# `${...}` interpolations inside template literals stay inside the string state, while
439+
# nested template literals and regex literals fall outside the scanner. When the depth
440+
# counter is confused by either, the section is caught downstream via
441+
# `rsc_plugin_options_followed_by_close_paren?` and marked unparseable so the migration
442+
# warns the user instead of corrupting the rewrite.
441443
def matching_js_closing_brace(content, open_index)
442444
depth = 0
443445
index = open_index
@@ -470,8 +472,47 @@ def matching_js_closing_brace(content, open_index)
470472
nil
471473
end
472474

473-
# Return index is the last consumed character. Line comments leave the newline
474-
# for the caller's normal index increment; block comments consume the closing slash.
475+
# Central dispatcher for the lightweight JS scanner shared by every JS-aware pass in this
476+
# generator (`matching_js_closing_brace`, `js_top_level_position?`, `js_code_position?`,
477+
# `rsc_plugin_options_without_comments`, `first_significant_js_index`,
478+
# `rsc_plugin_options_followed_by_close_paren?`, `last_js_code_char_index`). Return index
479+
# is the last consumed character. Line comments leave the newline for the caller's normal
480+
# index increment; block comments consume the closing slash.
481+
#
482+
# Supported lexical constructs:
483+
# - Line comments (`// ...\n`) and block comments (`/* ... */`).
484+
# - Single-quoted (`'...'`), double-quoted (`"..."`), and template-literal (`` `...` ``)
485+
# strings, including escape sequences and the simple `${expr}` interpolation form
486+
# (interpolation braces stay inside the string state and never reach the depth counter).
487+
#
488+
# Outside the supported surface — the scanner cannot distinguish these from the syntax
489+
# they shadow, so `{`/`}` characters they contain can confuse the depth counter:
490+
# - Regex literals (e.g. `/a{2}/`, `/\{/`, `/[{]/`): not recognized as a distinct state,
491+
# so brace-containing patterns walk the depth counter past the real options close. The
492+
# user-facing warning text in `warn_unparseable_rsc_plugin_sections` calls these out
493+
# explicitly.
494+
# - Nested template literals (`` `outer ${`inner`}` ``): the inner backtick falsely closes
495+
# the outer string state, exposing later braces to the depth counter.
496+
#
497+
# The downstream `rsc_plugin_option_sections_partition` catches both failure modes by
498+
# requiring the matched closing `}` to be followed by `)`. When it isn't, the section is
499+
# marked unparseable and `warn_unparseable_rsc_plugin_sections` asks the user to add
500+
# `clientReferences:` manually — the migration declines to rewrite rather than risk
501+
# corrupting the config.
502+
#
503+
# Future expansion (only worth doing if a real-world RSC plugin options block needs it):
504+
# 1. Add a `:regex_literal` state alongside the string and comment states. Track regex
505+
# contexts by detecting `/` after a token that legally precedes a regex literal
506+
# (`=`, `(`, `,`, `:`, `;`, `?`, `!`, `&&`, `||`, `return`, `typeof`, etc.) and consume
507+
# until the unescaped closing `/` plus any flags. The token-context check is necessary
508+
# because the same `/` character means division in expression position.
509+
# 2. Add a stack-based template-literal state so nested `` `...${`inner`}...` `` pairs
510+
# track depth instead of toggling a single boolean state.
511+
# Regex literals require expanding `advance_js_default_scan_state`; nested template
512+
# literals would also require replacing `advance_js_string_state` with stack-aware
513+
# handling. Both changes need a new state-machine branch; the current callers were
514+
# specifically designed around the simpler scanner and would need re-validation against
515+
# the expanded state set.
475516
#
476517
# IMPORTANT CALLER CONTRACT — block-comment exit:
477518
# When this returns from a `*/` exit, `state` is cleared, but `char` is still the `*` and
@@ -910,11 +951,18 @@ def rsc_client_references_setup_anchor?(content, is_server:)
910951
# this helper deliberately omits the `RSCWebpackPlugin` import that `inject_rsc_*_imports`
911952
# adds on the from-scratch path — adding it here would produce a duplicate import.
912953
def add_rsc_client_references_setup(config_path, content, existing_imports_content, is_server:)
913-
# Belt-and-suspenders: the only caller, `ensure_rsc_client_references_setup`, already
914-
# checks both `scoped_rsc_client_references_defined?` and `rsc_client_references_defined?`
915-
# before delegating here. The guards are kept so the helper is safe to call directly.
916-
return false if scoped_rsc_client_references_defined?(content)
917-
return false if rsc_client_references_defined?(content)
954+
# The only caller, `ensure_rsc_client_references_setup`, already runs these same checks
955+
# before delegating here, so in normal flow both conditions evaluate to `false` and no
956+
# early return is triggered. They are kept (rather than deleted) so a future second
957+
# caller — or a refactor that bypasses `ensure_rsc_client_references_setup` — cannot
958+
# accidentally splice a second `const rscClientReferences = { ... }` into a file that
959+
# already declares one. JavaScript would reject that with an
960+
# `Identifier 'rscClientReferences' has already been declared` SyntaxError at config
961+
# load, and the cost of the duplicate check is two boolean ops on the already-loaded
962+
# file body. Leaving the method defensive is cheaper than re-deriving the precondition
963+
# at each new call site.
964+
return if scoped_rsc_client_references_defined?(content)
965+
return if rsc_client_references_defined?(content)
918966

919967
replace_rsc_client_references_setup_anchor(config_path, content, is_server: is_server) do |anchor|
920968
join_rsc_client_references_setup(

0 commit comments

Comments
 (0)