Skip to content

Commit f952cb4

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 c66609c commit f952cb4

1 file changed

Lines changed: 66 additions & 18 deletions

File tree

react_on_rails/lib/generators/react_on_rails/rsc_setup.rb

Lines changed: 66 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -573,7 +573,7 @@ def rsc_plugin_sections_safe_to_rewrite?(config_path, content, is_server:)
573573
def rewritable_rsc_plugin?(config_path, content, is_server:)
574574
# Mixed same-target plugins are still rewritable: the later rewrite only updates plugins
575575
# missing clientReferences and leaves sibling custom clientReferences untouched.
576-
return true if rsc_plugin_without_client_references?(content, is_server: is_server)
576+
return true if any_rsc_plugin_missing_client_references?(content, is_server: is_server)
577577

578578
if rsc_plugin_defines_client_references?(content, is_server: is_server)
579579
GeneratorMessages.add_warning(
@@ -655,7 +655,12 @@ def rsc_plugin_defines_client_references?(content, is_server:)
655655
end
656656
end
657657

658-
def rsc_plugin_without_client_references?(content, is_server:)
658+
# Existential check: returns true when at least one matching plugin section is missing a
659+
# top-level `clientReferences:` key. Pairs with `rsc_plugin_defines_client_references?`,
660+
# which uses the same any-section semantics for the opposite condition. The two are not
661+
# complements when multiple plugin sections exist — a file with one configured plugin and
662+
# one unconfigured plugin returns true from both.
663+
def any_rsc_plugin_missing_client_references?(content, is_server:)
659664
rsc_plugin_option_sections(content, is_server: is_server).any? do |section|
660665
!rsc_plugin_body_has_top_level_key?(section.fetch(:body), "clientReferences")
661666
end
@@ -665,8 +670,8 @@ def rsc_plugin_without_client_references?(content, is_server:)
665670
# so `clientReferences:` / `isServer:` substrings inside strings are not mis-detected.
666671
# Shares the `advance_js_scan_state` family used by `js_top_level_position?` and
667672
# `matching_js_closing_brace` so all JS-aware passes follow the same comment/string rules.
668-
# Regex literals (e.g. `/a{2}/`) are still outside this scanner's supported surface
669-
# because brace quantifiers can confuse `matching_js_closing_brace`'s depth counter.
673+
# See `advance_js_scan_state` for the scanner's supported surface (including the regex-
674+
# literal and nested-template-literal limits that callers must be aware of).
670675
def rsc_plugin_options_without_comments(options)
671676
result = String.new(capacity: options.length)
672677
state = nil
@@ -816,15 +821,12 @@ def first_significant_js_index(content, start_index)
816821
end
817822

818823
# Expects `content[open_index] == "{"`; callers pass the options-object opening brace.
819-
# This lightweight scanner treats template literals as opaque strings (backtick to backtick).
820-
# Simple `${...}` expressions are handled correctly: while in the backtick state every
821-
# character — including `{` and `}` inside the expression — is consumed as string content
822-
# and never reaches the depth counter. The real unsupported case is *nested* template
823-
# literals (e.g. `` `outer ${`inner`}` ``) where the inner backtick falsely closes the outer
824-
# string state, exposing later braces to the depth counter. Callers detect that via
825-
# `rsc_plugin_options_followed_by_close_paren?` and mark the section unparseable rather
826-
# than producing a corrupt rewrite. Regex literals are outside this scanner's supported
827-
# surface for the same reason.
824+
# See `advance_js_scan_state` for the scanner's supported surface — in short, simple
825+
# `${...}` interpolations inside template literals stay inside the string state, while
826+
# nested template literals and regex literals fall outside the scanner. When the depth
827+
# counter is confused by either, the section is caught downstream via
828+
# `rsc_plugin_options_followed_by_close_paren?` and marked unparseable so the migration
829+
# warns the user instead of corrupting the rewrite.
828830
def matching_js_closing_brace(content, open_index)
829831
depth = 0
830832
index = open_index
@@ -853,8 +855,47 @@ def matching_js_closing_brace(content, open_index)
853855
nil
854856
end
855857

856-
# Return index is the last consumed character. Line comments leave the newline
857-
# for the caller's normal index increment; block comments consume the closing slash.
858+
# Central dispatcher for the lightweight JS scanner shared by every JS-aware pass in this
859+
# generator (`matching_js_closing_brace`, `js_top_level_position?`, `js_code_position?`,
860+
# `rsc_plugin_options_without_comments`, `first_significant_js_index`,
861+
# `rsc_plugin_options_followed_by_close_paren?`, `last_js_code_char_index`). Return index
862+
# is the last consumed character. Line comments leave the newline for the caller's normal
863+
# index increment; block comments consume the closing slash.
864+
#
865+
# Supported lexical constructs:
866+
# - Line comments (`// ...\n`) and block comments (`/* ... */`).
867+
# - Single-quoted (`'...'`), double-quoted (`"..."`), and template-literal (`` `...` ``)
868+
# strings, including escape sequences and the simple `${expr}` interpolation form
869+
# (interpolation braces stay inside the string state and never reach the depth counter).
870+
#
871+
# Outside the supported surface — the scanner cannot distinguish these from the syntax
872+
# they shadow, so `{`/`}` characters they contain can confuse the depth counter:
873+
# - Regex literals (e.g. `/a{2}/`, `/\{/`, `/[{]/`): not recognized as a distinct state,
874+
# so brace-containing patterns walk the depth counter past the real options close. The
875+
# user-facing warning text in `warn_unparseable_rsc_plugin_sections` calls these out
876+
# explicitly.
877+
# - Nested template literals (`` `outer ${`inner`}` ``): the inner backtick falsely closes
878+
# the outer string state, exposing later braces to the depth counter.
879+
#
880+
# The downstream `rsc_plugin_option_sections_partition` catches both failure modes by
881+
# requiring the matched closing `}` to be followed by `)`. When it isn't, the section is
882+
# marked unparseable and `warn_unparseable_rsc_plugin_sections` asks the user to add
883+
# `clientReferences:` manually — the migration declines to rewrite rather than risk
884+
# corrupting the config.
885+
#
886+
# Future expansion (only worth doing if a real-world RSC plugin options block needs it):
887+
# 1. Add a `:regex_literal` state alongside the string and comment states. Track regex
888+
# contexts by detecting `/` after a token that legally precedes a regex literal
889+
# (`=`, `(`, `,`, `:`, `;`, `?`, `!`, `&&`, `||`, `return`, `typeof`, etc.) and consume
890+
# until the unescaped closing `/` plus any flags. The token-context check is necessary
891+
# because the same `/` character means division in expression position.
892+
# 2. Add a stack-based template-literal state so nested `` `...${`inner`}...` `` pairs
893+
# track depth instead of toggling a single boolean state.
894+
# Regex literals require expanding `advance_js_default_scan_state`; nested template
895+
# literals would also require replacing `advance_js_string_state` with stack-aware
896+
# handling. Both changes need a new state-machine branch; the current callers were
897+
# specifically designed around the simpler scanner and would need re-validation against
898+
# the expanded state set.
858899
def advance_js_scan_state(state, escaped, char, next_char, index)
859900
return [char == "\n" ? nil : :line_comment, escaped, index] if state == :line_comment
860901
return advance_js_block_comment_state(escaped, char, next_char, index) if state == :block_comment
@@ -1114,9 +1155,16 @@ def rsc_client_references_setup_anchor?(content, is_server:)
11141155
# this helper deliberately omits the `RSCWebpackPlugin` import that `inject_rsc_*_imports`
11151156
# adds on the from-scratch path — adding it here would produce a duplicate import.
11161157
def add_rsc_client_references_setup(config_path, content, is_server:)
1117-
# Belt-and-suspenders: the only caller, `ensure_rsc_client_references_setup`, already
1118-
# checks both `scoped_rsc_client_references_defined?` and `rsc_client_references_defined?`
1119-
# before delegating here. The guards are kept so the helper is safe to call directly.
1158+
# The only caller, `ensure_rsc_client_references_setup`, already runs these same checks
1159+
# before delegating here, so in normal flow both conditions evaluate to `false` and no
1160+
# early return is triggered. They are kept (rather than deleted) so a future second
1161+
# caller — or a refactor that bypasses `ensure_rsc_client_references_setup` — cannot
1162+
# accidentally splice a second `const rscClientReferences = { ... }` into a file that
1163+
# already declares one. JavaScript would reject that with an
1164+
# `Identifier 'rscClientReferences' has already been declared` SyntaxError at config
1165+
# load, and the cost of the duplicate check is two boolean ops on the already-loaded
1166+
# file body. Leaving the method defensive is cheaper than re-deriving the precondition
1167+
# at each new call site.
11201168
return if scoped_rsc_client_references_defined?(content)
11211169
return if rsc_client_references_defined?(content)
11221170

0 commit comments

Comments
 (0)