Fix Pro generator multiline and template-literal rewrites#2918
Conversation
WalkthroughRefactors pro generator parsing and template-literal scanning: extracts helper to consume non-parenthesized multiline gem declarations, reworks backtick detection and multiline-start logic, tweaks gem-replacement suffix handling, and adds/specifies tests for multiline gem forms and template-literal edge cases. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 fixes two separate but related rewrite-logic bugs in the Pro generator: multiline non-parenthesized Key changes:
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["swap_base_gem_for_pro_in_gemfile\n(outer loop)"] --> B{match base_gem_pattern?}
B -- no --> C[append line unchanged]
B -- yes --> D["consume_non_parenthesized_base_gem_declaration\n(NEW helper)"]
D --> E["collect start line\n+ comma-continuation lines\ninto declaration_lines"]
E --> F["trailing_suffix =\nlines[start][match_end..]\n+ drop(1).join"]
F --> G{pro_entry_added?}
G -- no --> H["build_pro_gem_replacement_line\n(strips version args,\nkeeps non-version options)"]
G -- yes --> I[skip — already have pro entry]
H --> J[append replacement line]
J --> K["line_index = declaration[:next_index]\n(skip consumed continuation lines)"]
I --> K
subgraph "JS rewrite — template literal handling"
L["rewrite_outside_inline_template_literals"] --> M["gsub: replace inline\ntemplate literals with\nplaceholders"]
M --> N["yield line_without_templates\n(pattern rewrites happen here)"]
N --> O["sub(placeholder) { template_literal }\n(block form — FIX:\nprevents \\1 back-ref corruption)"]
end
subgraph "JS rewrite — multiline template detection"
P["rewrite_line_before_template_literal_open"] --> Q["opening_backtick_index_for_multiline_start\n(NEW: returns LAST backtick\nwhen count is odd)"]
Q --> R{index positive?}
R -- yes --> S[rewrite prefix before backtick]
R -- no / nil --> T[return line unchanged]
end
Reviews (1): Last reviewed commit: "Handle multiline gem swaps and template ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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: Version stripping leaves blank line in multiline output
- When removing a multiline version argument with a trailing comma, the replacement now trims trailing indentation from the preserved prefix so no blank line is left behind, and the spec now asserts that formatting.
Or push these changes by commenting:
@cursor push ea0384721c
Preview (ea0384721c)
diff --git a/react_on_rails/lib/generators/react_on_rails/pro_generator.rb b/react_on_rails/lib/generators/react_on_rails/pro_generator.rb
--- a/react_on_rails/lib/generators/react_on_rails/pro_generator.rb
+++ b/react_on_rails/lib/generators/react_on_rails/pro_generator.rb
@@ -865,7 +865,12 @@
version_arg_pattern = /\A(?<prefix>\s*,(?:\s*#.*\n|\s++)*)["'][^"']*["'](?<trailing_comma>\s*,)?/
loop do
updated_suffix = normalized_suffix.sub(version_arg_pattern) do
- Regexp.last_match[:trailing_comma] ? Regexp.last_match[:prefix] : ""
+ if Regexp.last_match[:trailing_comma]
+ # Preserve separators/comments while removing indentation that belonged to the stripped version line.
+ Regexp.last_match[:prefix].sub(/\n[ \t]*\z/, "")
+ else
+ ""
+ end
end
break if updated_suffix == normalized_suffix
diff --git a/react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb b/react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb
--- a/react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb
+++ b/react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb
@@ -230,6 +230,7 @@
gemfile_content = File.read(gemfile_path)
expected_version = generator.send(:pro_gem_version_requirement)
expect(gemfile_content).to include("gem \"react_on_rails_pro\", \"#{expected_version}\",")
+ expect(gemfile_content).not_to include(",\n \n require: false")
expect(gemfile_content).to include("require: false")
expect(gemfile_content).not_to include("\"~> 16.0\"")
endYou can send follow-ups to this agent here.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
react_on_rails/lib/generators/react_on_rails/pro_generator.rb (1)
743-779:⚠️ Potential issue | 🟡 MinorMake the multiline-open backtick scan honor block-comment state.
line_contains_unescaped_backtickis decided from the comment-aware tracking line, butopening_backtick_index_for_multiline_startrescans the raw line withoutin_block_commenthandling. On a line that closes/* ... */and then opens a multiline template, an odd backtick count inside the comment can makeopening_indexwrong ornil, so theimport("react-on-rails")before the real template opener is skipped. Please drive the split-point calculation from the same comment-aware scan, and add a regression around*/ import("react-on-rails"); \start` after a comment line containing a lone backtick.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d95d10dc-8d32-4bc8-a5bb-c8ee46a9daf3
📒 Files selected for processing (2)
react_on_rails/lib/generators/react_on_rails/pro_generator.rbreact_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb
|
@claude review this PR. |
![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: e93a57e4df
ℹ️ 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 2 potential issues.
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Stale
in_block_commentflag after template literal branch- Template-literal rewrite branches now recompute
in_block_commentfrom the rewritten line so stale true state is cleared after comments close.
- Template-literal rewrite branches now recompute
- ✅ Fixed: Backtick finder incorrectly skips
/*inside template literals- Backtick scanning now allows disabling block-comment tracking for template-literal-close detection so literal
/*content no longer hides the closing backtick.
- Backtick scanning now allows disabling block-comment tracking for template-literal-close detection so literal
Or push these changes by commenting:
@cursor push 08ed1eecd0
Preview (08ed1eecd0)
diff --git a/react_on_rails/lib/generators/react_on_rails/pro_generator.rb b/react_on_rails/lib/generators/react_on_rails/pro_generator.rb
--- a/react_on_rails/lib/generators/react_on_rails/pro_generator.rb
+++ b/react_on_rails/lib/generators/react_on_rails/pro_generator.rb
@@ -473,7 +473,7 @@
pending_multiline_static_import_specifier
) { |line_fragment| yield line_fragment }
in_multiline_template_literal = updated_template_literal_state
- in_block_comment = true if unclosed_block_comment_starts?(rewritten_line)
+ in_block_comment = unclosed_block_comment_starts?(rewritten_line)
rewritten_line
elsif line_contains_unescaped_backtick
rewritten_line, pending_multiline_module_call_depth, pending_multiline_static_import_specifier =
@@ -484,7 +484,7 @@
in_block_comment: in_block_comment
) { |line_fragment| yield line_fragment }
in_multiline_template_literal = updated_template_literal_state
- in_block_comment = true if unclosed_block_comment_starts?(rewritten_line)
+ in_block_comment = unclosed_block_comment_starts?(rewritten_line)
rewritten_line
else
in_multiline_template_literal = updated_template_literal_state
@@ -760,7 +760,7 @@
end
def first_unescaped_backtick_index(line)
- unescaped_backtick_indexes(line).first
+ unescaped_backtick_indexes(line, track_block_comments: false).first
end
def opening_backtick_index_for_multiline_start(line, in_block_comment: false)
@@ -771,13 +771,13 @@
end
# rubocop:disable Metrics/CyclomaticComplexity
- def unescaped_backtick_indexes(line, in_block_comment: false)
+ def unescaped_backtick_indexes(line, in_block_comment: false, track_block_comments: true)
quote_state = nil
backtick_indexes = []
scan_index = 0
while scan_index < line.length
- if in_block_comment
+ if track_block_comments && in_block_comment
closing_index = line.index("*/", scan_index)
break unless closing_index
@@ -795,7 +795,7 @@
break if line[scan_index, 2] == "//"
- if line[scan_index, 2] == "/*"
+ if track_block_comments && line[scan_index, 2] == "/*"
in_block_comment = true
scan_index += 2
next
diff --git a/react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb b/react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb
--- a/react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb
+++ b/react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb
@@ -1038,6 +1038,35 @@
expect(rewritten).to include('odd backtick ` still comment */ import("react-on-rails-pro"); `multiline-start')
end
+ it "resets block-comment state after multiline template literals opened on comment-closing lines" do
+ source = <<~JS
+ /*
+ odd backtick ` still comment */ import("react-on-rails"); `multiline-start
+ import ReactOnRails from "react-on-rails";
+ `;
+ import ReactOnRails from "react-on-rails";
+ JS
+
+ rewritten = generator.send(:rewrite_react_on_rails_module_specifiers, source)
+
+ expect(rewritten).to include('odd backtick ` still comment */ import("react-on-rails-pro"); `multiline-start')
+ expect(rewritten).to include('import ReactOnRails from "react-on-rails";')
+ expect(rewritten).to include("import ReactOnRails from \"react-on-rails-pro\";")
+ expect(rewritten.scan("react-on-rails-pro").size).to eq(2)
+ end
+
+ it "finds multiline template literal closes when closing lines contain unmatched block-comment markers" do
+ source = <<~JS
+ const template = `
+ /* css comment marker `; const ror = require("react-on-rails");
+ JS
+
+ rewritten = generator.send(:rewrite_react_on_rails_module_specifiers, source)
+
+ expect(rewritten).to include('/* css comment marker `; const ror = require("react-on-rails-pro");')
+ expect(rewritten.scan("react-on-rails-pro").size).to eq(1)
+ end
+
it "rewrites all matching specifiers on a pending continuation line" do
source = <<~JS
import {You can send follow-ups to this agent here.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
ReviewThe three fixes are all correct and address real bugs:
One genuine bug to fix before merge (see inline comment at line 487): When in_block_comment is true and the block comment closes mid-line while a template literal opens on the same line, the elsif line_contains_unescaped_backtick branch runs but never resets in_block_comment to false. After the template literal subsequently closes, every following line falls into the elsif in_block_comment guard and is silently skipped, so any react-on-rails imports after that point are never rewritten. The fix is a one-character change: replace the conditional assignment with an unconditional one (line 487). The new spec at line 1030 correctly exercises the detection half of this fix but does not cover lines that appear after the template literal closes. A suggested extended test case is in the spec inline comment. Smaller notes:
|
- Change in_block_comment assignment from conditional (= true if) to unconditional (=) in both template-literal branches so the flag is properly reset when a block comment closes on the same line a template literal opens. Without this, all lines after the template literal closes were silently skipped for rewriting. - Add skip_comments parameter to unescaped_backtick_indexes so first_unescaped_backtick_index (used when scanning for the closing backtick inside a template literal) does not treat /* as a block comment start. Template literal content is not code, so comment tokens should be ignored when finding the closing backtick. - Add tests for both edge cases. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Review comment triage & resolution (5a6e361)Triaged 11 review comments across 5 bots. Two real bugs were fixed, two claims were investigated and found incorrect, and documentation nits were declined as out of scope. Fixed (2 bugs)
Both fixes include regression tests. All 79 specs pass. Investigated & declined (2 false positives)
Declined as out of scope (4 style/documentation nits)
All 11 review threads are now resolved. |
| backtick_indexes = unescaped_backtick_indexes(line, in_block_comment: in_block_comment) | ||
| return nil if backtick_indexes.empty? || backtick_indexes.length.even? | ||
|
|
||
| backtick_indexes.last |
There was a problem hiding this comment.
The even/odd heuristic is clever but non-obvious — worth a brief comment to explain the invariant:
- Even count → all backticks are paired as inline template literals; no multiline opener on this line.
- Odd count → the last unpaired backtick opens a multiline template literal.
This holds only when ${...} nesting is not tracked (a pre-existing limitation). A one-liner comment here would help the next reader.
| backtick_indexes.last | |
| def opening_backtick_index_for_multiline_start(line, in_block_comment: false) | |
| backtick_indexes = unescaped_backtick_indexes(line, in_block_comment: in_block_comment) | |
| # Even count → all inline literals are closed; odd count → last backtick opens a multiline literal. | |
| return nil if backtick_indexes.empty? || backtick_indexes.length.even? | |
| backtick_indexes.last | |
| end |
| end | ||
|
|
||
| def first_unescaped_backtick_index(line) | ||
| unescaped_backtick_indexes(line, skip_comments: true).first |
There was a problem hiding this comment.
The skip_comments: true parameter name is semantically reversed — it means "don't treat // / /* */ as comments", i.e. "scan as-if inside a template literal". This is correct (template literal content ignores JS comment syntax), but it's easy to misread as "skip backticks that are inside comments". A clearer name like in_template_literal_content: true or just an inline comment would reduce confusion:
| unescaped_backtick_indexes(line, skip_comments: true).first | |
| def first_unescaped_backtick_index(line) | |
| # skip_comments: true because this is called inside a multiline template literal, | |
| # where // and /* */ are content, not JS comments. | |
| unescaped_backtick_indexes(line, skip_comments: true).first | |
| end |
| line_index += 1 | ||
| end | ||
|
|
||
| trailing_suffix = lines[start_index][match_end..].to_s + declaration_lines.drop(1).join |
There was a problem hiding this comment.
Edge case to verify: if there are blank or comment-only continuation lines interspersed between the version argument and non-version options, those lines are accumulated in declaration_lines (and thus in trailing_suffix) verbatim. For example:
gem "react_on_rails",
# pinned version
"~> 16.0",
# needed for assets pipeline
require: falsedeclaration_lines.drop(1) would include the comment lines, so trailing_suffix would contain them. build_pro_gem_replacement_line's version_arg_pattern consumes \s*#.*\n in the prefix group, so the comments adjacent to the version arg get stripped. But the comment adjacent to require: false would be preserved. This all seems intentional, but a quick test covering this specific interleaved-comment case would strengthen confidence.
| ) { |line_fragment| yield line_fragment } | ||
| in_multiline_template_literal = updated_template_literal_state | ||
| in_block_comment = true if unclosed_block_comment_starts?(rewritten_line) | ||
| in_block_comment = unclosed_block_comment_starts?(rewritten_line) |
There was a problem hiding this comment.
Good fix. The original in_block_comment = true if ... pattern could only toggle the flag from false → true in this branch, meaning a line that exits a multiline template literal and doesn't open a block comment would silently leave a stale in_block_comment = true from a prior iteration. Using a direct assignment correctly handles both transitions.
| rewritten_line = yield line_without_inline_templates | ||
| template_placeholders.each do |placeholder, template_literal| | ||
| rewritten_line = rewritten_line.sub(placeholder, template_literal) | ||
| rewritten_line = rewritten_line.sub(placeholder) { template_literal } |
There was a problem hiding this comment.
Important correctness fix. String#sub(pattern, replacement_string) interprets special sequences in the replacement string (\1, \0, \&, \\, etc.) as backreferences. Template literals frequently contain \n, \1, etc., so the old code would silently corrupt content. Using a block sidesteps that entirely.
One minor note: if template_literal could ever be a very large string used many times, the block form might marginally affect performance — but in practice this is negligible for a code generator.
Review: Fix Pro generator multiline and template-literal rewritesOverall this is a solid, targeted fix. The root causes are real and the changes are well-scoped. A few observations inline; high-level notes below. What's fixed well
Minor concerns (see inline)
TestingThe new specs cover the specific edge cases being fixed and are well-structured. No security or performance concerns identified. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a6e361f20
ℹ️ 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".
| updated_suffix = normalized_suffix.sub(version_arg_pattern) do | ||
| Regexp.last_match[:trailing_comma] ? Regexp.last_match[:prefix] : "" | ||
| if Regexp.last_match[:trailing_comma] | ||
| Regexp.last_match[:prefix].sub(/\n[ \t]*\z/, "") |
There was a problem hiding this comment.
When the matched version argument has a trailing comma, trimming Regexp.last_match[:prefix] with .sub(/\n[ \t]*\z/, "") can merge the next line into a preceding # comment, so options like require: false become commented out and silently lost in the rewritten gem "react_on_rails_pro" entry. This happens for declarations like gem "react_on_rails", # note followed by a multiline version/options block. Fresh evidence: the current commit still performs this newline trim at this location without a guard for comment-terminated prefixes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
react_on_rails/lib/generators/react_on_rails/pro_generator.rb (1)
728-810:⚠️ Potential issue | 🟡 MinorWhen scanning for a closing template-literal backtick, disable quote tracking since quotes are just characters in template-literal text.
When
first_unescaped_backtick_indexcallsunescaped_backtick_indexes(..., skip_comments: true), the method still tracks quote state (lines 790–794) and skips backtick detection while inside quotes. In template-literal text, single and double quotes have no syntactic effect—only an unescaped backtick terminates the literal. A line like"foo; require("react-on-rails")` fails to identify the closing backtick and the suffix never gets rewritten.Add a
track_quotes: trueparameter tounescaped_backtick_indexesand passtrack_quotes: falsefromfirst_unescaped_backtick_index:Fix
def first_unescaped_backtick_index(line) - unescaped_backtick_indexes(line, skip_comments: true).first + unescaped_backtick_indexes(line, skip_comments: true, track_quotes: false).first end ... - def unescaped_backtick_indexes(line, in_block_comment: false, skip_comments: false) + def unescaped_backtick_indexes(line, in_block_comment: false, skip_comments: false, track_quotes: true) quote_state = nil backtick_indexes = [] scan_index = 0 while scan_index < line.length ... char = line[scan_index] - quote_state = next_quote_state(quote_state, char, line, scan_index) + quote_state = next_quote_state(quote_state, char, line, scan_index) if track_quotes if quote_state scan_index += 1 next end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@react_on_rails/lib/generators/react_on_rails/pro_generator.rb` around lines 728 - 810, The scanner treats quotes as syntactic when searching for a closing template backtick, causing it to miss backticks inside template text; add a new boolean parameter track_quotes (default true) to unescaped_backtick_indexes, guard the quote-state updates and quote-skipping logic behind track_quotes, and update first_unescaped_backtick_index to call unescaped_backtick_indexes(..., skip_comments: true, track_quotes: false) so quotes are ignored when finding the closing backtick; leave other callers (e.g., opening_backtick_index_for_multiline_start) using the default true behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@react_on_rails/lib/generators/react_on_rails/pro_generator.rb`:
- Around line 875-891: The loop in
consume_non_parenthesized_base_gem_declaration only treats comma-terminated
continuations and misses explicit backslash continuations; update the while
condition to also continue when the current line ends with a backslash (e.g. add
a check like line_ends_with_backslash?(current_line) to the existing
line_continues_with_comma? condition), implement a small helper
line_ends_with_backslash?(line) that returns line.rstrip.end_with?("\\") and
leave the rest of the logic (appending next_line to declaration_lines and
building trailing_suffix) unchanged so backslash-continued version constraints
are consumed into trailing_suffix instead of being left behind.
---
Outside diff comments:
In `@react_on_rails/lib/generators/react_on_rails/pro_generator.rb`:
- Around line 728-810: The scanner treats quotes as syntactic when searching for
a closing template backtick, causing it to miss backticks inside template text;
add a new boolean parameter track_quotes (default true) to
unescaped_backtick_indexes, guard the quote-state updates and quote-skipping
logic behind track_quotes, and update first_unescaped_backtick_index to call
unescaped_backtick_indexes(..., skip_comments: true, track_quotes: false) so
quotes are ignored when finding the closing backtick; leave other callers (e.g.,
opening_backtick_index_for_multiline_start) using the default true behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ba46c65f-c396-474b-b8ab-768d8221ca27
📒 Files selected for processing (2)
react_on_rails/lib/generators/react_on_rails/pro_generator.rbreact_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb
🚧 Files skipped from review as they are similar to previous changes (1)
- react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb
| def consume_non_parenthesized_base_gem_declaration(lines, start_index, match_end) | ||
| line_index = start_index | ||
| current_line = lines[line_index] | ||
| declaration_lines = [current_line] | ||
| line_index += 1 | ||
|
|
||
| while line_index < lines.length && | ||
| line_continues_with_comma?(current_line) && | ||
| gem_declaration_continues_on_next_line?(lines[line_index]) | ||
| next_line = lines[line_index] | ||
| declaration_lines << next_line | ||
| current_line = next_line unless comment_or_blank_line?(next_line) | ||
| line_index += 1 | ||
| end | ||
|
|
||
| trailing_suffix = lines[start_index][match_end..].to_s + declaration_lines.drop(1).join | ||
| { trailing_suffix: trailing_suffix, next_index: line_index } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Ruby, does a trailing backslash (`) continue a multiline method call, for example:
gem "react_on_rails",
"~> 16.0",
require: false
so that the following lines are still arguments to the same gem(...) call?`
💡 Result:
Yes, in Ruby, a trailing backslash () at the end of a line acts as an explicit line continuation character, allowing a single expression or statement—such as a method call—to span multiple lines. The parser treats the lines as if they were a single continuous line, with the backslash escaping the newline. This works for the given example in a Gemfile (which is evaluated as Ruby code): gem "react_on_rails", \ "> 16.0", \ require: false The gem(...) method call continues across lines, passing all three arguments: the string "react_on_rails", the string "> 16.0", and the hash {require: false}.
Citations:
- 1: https://docs.ruby-lang.org/en/trunk/syntax/layout_rdoc.html
- 2: https://docs.ruby-lang.org/en/4.0/syntax/layout_rdoc.html
- 3: https://stackoverflow.com/questions/27792508/ruby-backslash-to-continue-string-on-a-new-line
- 4: https://gist.github.com/eliotsykes/e64786ef97479ba234b3583febb025b2
🏁 Script executed:
# First, locate the file and check if it exists
find . -name "pro_generator.rb" -type f
# Then check the specific lines mentioned in the review (875-891)
# and look for the line_continues_with_comma? method definition
if [ -f "react_on_rails/lib/generators/react_on_rails/pro_generator.rb" ]; then
echo "=== Lines 875-891 ==="
sed -n '875,891p' react_on_rails/lib/generators/react_on_rails/pro_generator.rb | cat -n
echo ""
echo "=== Searching for line_continues_with_comma? method ==="
grep -n "def line_continues_with_comma?" react_on_rails/lib/generators/react_on_rails/pro_generator.rb | head -5
echo ""
echo "=== Context around line_continues_with_comma? definition ==="
grep -A 5 "def line_continues_with_comma?" react_on_rails/lib/generators/react_on_rails/pro_generator.rb
fiRepository: shakacode/react_on_rails
Length of output: 1419
🏁 Script executed:
# Check for line length violations (max 120 chars) in the specified range and method
if [ -f "react_on_rails/lib/generators/react_on_rails/pro_generator.rb" ]; then
echo "=== Checking line lengths in range 875-900 ==="
sed -n '875,900p' react_on_rails/lib/generators/react_on_rails/pro_generator.rb | awk 'length > 120 {print NR + 874 ": " length " chars: " $0}'
echo ""
echo "=== Checking line lengths around line_continues_with_comma? ==="
grep -n "def line_continues_with_comma?" react_on_rails/lib/generators/react_on_rails/pro_generator.rb | while read line_info; do
line_num=$(echo "$line_info" | cut -d: -f1)
start=$((line_num - 2))
end=$((line_num + 5))
sed -n "${start},${end}p" react_on_rails/lib/generators/react_on_rails/pro_generator.rb | awk -v start="$start" 'length > 120 {print NR + start - 1 ": " length " chars: " $0}'
done
fiRepository: shakacode/react_on_rails
Length of output: 179
Handle explicit \ continuations in multiline Gemfile entries.
The consume_non_parenthesized_base_gem_declaration method (line 881) only checks for comma-terminated lines via line_continues_with_comma?. Ruby also permits explicit line continuation with a trailing backslash, so a gem declaration like gem "react_on_rails", \ followed by version constraints will be rewritten to react_on_rails_pro while leaving the old version arguments behind. This can cause conflicting requirements or failed bundle install.
Fix
- def line_continues_with_comma?(line)
+ def line_continues?(line)
line_without_comment = line.sub(/\s*#.*$/, "").rstrip
- line_without_comment.end_with?(",")
+ line_without_comment.end_with?(",", "\\")
end
while line_index < lines.length &&
- line_continues_with_comma?(current_line) &&
+ line_continues?(current_line) &&
gem_declaration_continues_on_next_line?(lines[line_index])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@react_on_rails/lib/generators/react_on_rails/pro_generator.rb` around lines
875 - 891, The loop in consume_non_parenthesized_base_gem_declaration only
treats comma-terminated continuations and misses explicit backslash
continuations; update the while condition to also continue when the current line
ends with a backslash (e.g. add a check like
line_ends_with_backslash?(current_line) to the existing
line_continues_with_comma? condition), implement a small helper
line_ends_with_backslash?(line) that returns line.rstrip.end_with?("\\") and
leave the rest of the logic (appending next_line to declaration_lines and
building trailing_suffix) unchanged so backslash-continued version constraints
are consumed into trailing_suffix instead of being left behind.
…e-dedup * origin/main: (51 commits) Fix Content-Length mismatch and null renderingRequest errors in node renderer (#3069) Improve memory debugging docs with simpler heap snapshot approach (#3072) Enforce strict version matching in doctor and recommend doctor on errors (#3070) Remove immediate_hydration feature from codebase (#2834) Fix infinite fork loop when node renderer worker fails to bind port (#2881) Fix TanStack Router SSR hydration mismatch in Pro async path (#2932) Improve node renderer error messages for malformed render requests (#3068) Add interactive mode prompt to create-react-on-rails-app (#3063) docs: replace hardcoded version numbers with unversioned install commands (#2893) Remove Contributing section from docs sidebar (#3064) Consolidate docs comparison pages into single evaluation entry (#3065) Docs route cleanup: canonicalize worst verbose URL slugs (#3067) Add agent summary blocks to high-value docs pages (#3066) Fix brittle positional assertions in create-app tests (#2923) Auto-resolve renderer password from ENV in Rails Pro (#2921) Standardize bundle env vars from =yes to =true (#2925) Fix Pro generator multiline and template-literal rewrites (#2918) Fix spec/dummy Procfile.dev SERVER_BUNDLE_ONLY to match template convention (#2922) Add llms docs entry points (#2916) Bump version to 16.6.0.rc.0 ... # Conflicts: # packages/create-react-on-rails-app/src/create-app.ts
…lacement-2835-main * origin/main: (44 commits) Fix Content-Length mismatch and null renderingRequest errors in node renderer (#3069) Improve memory debugging docs with simpler heap snapshot approach (#3072) Enforce strict version matching in doctor and recommend doctor on errors (#3070) Remove immediate_hydration feature from codebase (#2834) Fix infinite fork loop when node renderer worker fails to bind port (#2881) Fix TanStack Router SSR hydration mismatch in Pro async path (#2932) Improve node renderer error messages for malformed render requests (#3068) Add interactive mode prompt to create-react-on-rails-app (#3063) docs: replace hardcoded version numbers with unversioned install commands (#2893) Remove Contributing section from docs sidebar (#3064) Consolidate docs comparison pages into single evaluation entry (#3065) Docs route cleanup: canonicalize worst verbose URL slugs (#3067) Add agent summary blocks to high-value docs pages (#3066) Fix brittle positional assertions in create-app tests (#2923) Auto-resolve renderer password from ENV in Rails Pro (#2921) Standardize bundle env vars from =yes to =true (#2925) Fix Pro generator multiline and template-literal rewrites (#2918) Fix spec/dummy Procfile.dev SERVER_BUNDLE_ONLY to match template convention (#2922) Add llms docs entry points (#2916) Bump version to 16.6.0.rc.0 ... # Conflicts: # CHANGELOG.md
### Summary Adds missing changelog entries to the `[Unreleased]` section for PRs merged since `v16.6.0.rc.0`. Analyzed 19 commits on `origin/main` since the last tag, identified 1 missing user-visible PR (#3069), and skipped 10 non-user-visible PRs (docs, tests, internal refactors). **New entries added:** - **Added**: `[Pro] Configurable HTTP keep-alive timeout for node renderer connections` (PR #3069) - **Fixed**: `[Pro] Fixed SSR failures from stale persistent HTTP/2 connections` (PR #3069) **Entries already present** (added in prior changelog updates): PRs #2834, #2881, #2918, #2921, #2923, #2932, #3063, #3068, #3070. **Skipped** (not user-visible): #2893 (docs), #2916 (docs), #2922 (test fix), #2923 (test fix), #2925 (internal refactor), #3064 (docs), #3065 (docs), #3066 (docs), #3067 (docs), #3072 (docs). ### Pull Request checklist - [x] ~Add/update test to cover these changes~ - [x] ~Update documentation~ - [x] Update CHANGELOG file ### Other Information No code changes — CHANGELOG.md only. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk because this PR only changes documentation (`CHANGELOG.md`) and does not modify runtime code. > > **Overview** > Updates `CHANGELOG.md` *[Unreleased]* to include missing Pro release notes for PR `#3069`, documenting the new `renderer_http_keep_alive_timeout` config and the associated fix for SSR failures from stale persistent HTTP/2 connections to the node renderer. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 8d1a480. 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 * **New Features** * Interactive mode selection prompt for CLI tool when no explicit mode is specified * Configurable keep-alive timeout setting for Pro users (default 30 seconds) * **Bug Fixes** * Enhanced validation and error handling for invalid request payloads * Improved template literal handling in code generation * Better HTTP connection stability with enhanced diagnostic messaging <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
### Summary Improve the Pro install generator rewrite logic so multiline non-parenthesized `gem "react_on_rails"` declarations keep trailing options while replacing the base gem entry. Harden module-specifier rewriting around template literals by preserving escaped sequences and correctly detecting multiline template-literal starts after a closed inline template. ### Pull Request checklist - [x] Add/update test to cover these changes - [x] ~Update documentation~ - [x] ~Update CHANGELOG file~ ### Other Information Validated with `bundle exec rspec react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb:219 react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb:1007 react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb:1019`. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches the Pro install generator’s Gemfile and JS source rewriting logic; subtle parsing/state bugs could cause incorrect dependency swaps or missed/over-eager import rewrites in real-world edge cases. > > **Overview** > Improves the Pro generator’s Gemfile swap to correctly consume *non-parenthesized multiline* `gem "react_on_rails"` declarations and carry forward trailing options/guards while stripping only version constraints, avoiding orphaned lines and stray/trailing commas. > > Hardens JS module-specifier rewriting around template literals by restoring inline template placeholders without altering escaped sequences, and by more accurately detecting multiline template-literal boundaries across inline literals and block-comment transitions (so imports between these regions are rewritten correctly). Adds focused specs for these multiline Gemfile and template-literal edge cases. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 5a6e361. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Preserve non-version gem options during upgrade without leaving trailing commas, extra whitespace, or blank-line artifacts. * More robust template-literal handling across inline and multiline cases, including escaped sequences, correct backtick detection, and block-comment boundary transitions. * **Tests** * Added unit tests for non-parenthesized multiline gem declarations. * Added tests for template-literal edge cases: placeholder restoration, imports between literals, and comment-boundary scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
### Summary Adds missing changelog entries to the `[Unreleased]` section for PRs merged since `v16.6.0.rc.0`. Analyzed 19 commits on `origin/main` since the last tag, identified 1 missing user-visible PR (#3069), and skipped 10 non-user-visible PRs (docs, tests, internal refactors). **New entries added:** - **Added**: `[Pro] Configurable HTTP keep-alive timeout for node renderer connections` (PR #3069) - **Fixed**: `[Pro] Fixed SSR failures from stale persistent HTTP/2 connections` (PR #3069) **Entries already present** (added in prior changelog updates): PRs #2834, #2881, #2918, #2921, #2923, #2932, #3063, #3068, #3070. **Skipped** (not user-visible): #2893 (docs), #2916 (docs), #2922 (test fix), #2923 (test fix), #2925 (internal refactor), #3064 (docs), #3065 (docs), #3066 (docs), #3067 (docs), #3072 (docs). ### Pull Request checklist - [x] ~Add/update test to cover these changes~ - [x] ~Update documentation~ - [x] Update CHANGELOG file ### Other Information No code changes — CHANGELOG.md only. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk because this PR only changes documentation (`CHANGELOG.md`) and does not modify runtime code. > > **Overview** > Updates `CHANGELOG.md` *[Unreleased]* to include missing Pro release notes for PR `#3069`, documenting the new `renderer_http_keep_alive_timeout` config and the associated fix for SSR failures from stale persistent HTTP/2 connections to the node renderer. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 8d1a480. 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 * **New Features** * Interactive mode selection prompt for CLI tool when no explicit mode is specified * Configurable keep-alive timeout setting for Pro users (default 30 seconds) * **Bug Fixes** * Enhanced validation and error handling for invalid request payloads * Improved template literal handling in code generation * Better HTTP connection stability with enhanced diagnostic messaging <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
### Summary Improve the Pro install generator rewrite logic so multiline non-parenthesized `gem "react_on_rails"` declarations keep trailing options while replacing the base gem entry. Harden module-specifier rewriting around template literals by preserving escaped sequences and correctly detecting multiline template-literal starts after a closed inline template. ### Pull Request checklist - [x] Add/update test to cover these changes - [x] ~Update documentation~ - [x] ~Update CHANGELOG file~ ### Other Information Validated with `bundle exec rspec react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb:219 react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb:1007 react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb:1019`. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches the Pro install generator’s Gemfile and JS source rewriting logic; subtle parsing/state bugs could cause incorrect dependency swaps or missed/over-eager import rewrites in real-world edge cases. > > **Overview** > Improves the Pro generator’s Gemfile swap to correctly consume *non-parenthesized multiline* `gem "react_on_rails"` declarations and carry forward trailing options/guards while stripping only version constraints, avoiding orphaned lines and stray/trailing commas. > > Hardens JS module-specifier rewriting around template literals by restoring inline template placeholders without altering escaped sequences, and by more accurately detecting multiline template-literal boundaries across inline literals and block-comment transitions (so imports between these regions are rewritten correctly). Adds focused specs for these multiline Gemfile and template-literal edge cases. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 5a6e361. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Preserve non-version gem options during upgrade without leaving trailing commas, extra whitespace, or blank-line artifacts. * More robust template-literal handling across inline and multiline cases, including escaped sequences, correct backtick detection, and block-comment boundary transitions. * **Tests** * Added unit tests for non-parenthesized multiline gem declarations. * Added tests for template-literal edge cases: placeholder restoration, imports between literals, and comment-boundary scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
### Summary Adds missing changelog entries to the `[Unreleased]` section for PRs merged since `v16.6.0.rc.0`. Analyzed 19 commits on `origin/main` since the last tag, identified 1 missing user-visible PR (#3069), and skipped 10 non-user-visible PRs (docs, tests, internal refactors). **New entries added:** - **Added**: `[Pro] Configurable HTTP keep-alive timeout for node renderer connections` (PR #3069) - **Fixed**: `[Pro] Fixed SSR failures from stale persistent HTTP/2 connections` (PR #3069) **Entries already present** (added in prior changelog updates): PRs #2834, #2881, #2918, #2921, #2923, #2932, #3063, #3068, #3070. **Skipped** (not user-visible): #2893 (docs), #2916 (docs), #2922 (test fix), #2923 (test fix), #2925 (internal refactor), #3064 (docs), #3065 (docs), #3066 (docs), #3067 (docs), #3072 (docs). ### Pull Request checklist - [x] ~Add/update test to cover these changes~ - [x] ~Update documentation~ - [x] Update CHANGELOG file ### Other Information No code changes — CHANGELOG.md only. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk because this PR only changes documentation (`CHANGELOG.md`) and does not modify runtime code. > > **Overview** > Updates `CHANGELOG.md` *[Unreleased]* to include missing Pro release notes for PR `#3069`, documenting the new `renderer_http_keep_alive_timeout` config and the associated fix for SSR failures from stale persistent HTTP/2 connections to the node renderer. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 8d1a480. 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 * **New Features** * Interactive mode selection prompt for CLI tool when no explicit mode is specified * Configurable keep-alive timeout setting for Pro users (default 30 seconds) * **Bug Fixes** * Enhanced validation and error handling for invalid request payloads * Improved template literal handling in code generation * Better HTTP connection stability with enhanced diagnostic messaging <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 participant
Summary
Improve the Pro install generator rewrite logic so multiline non-parenthesized
gem "react_on_rails"declarations keep trailing options while replacing the base gem entry. Harden module-specifier rewriting around template literals by preserving escaped sequences and correctly detecting multiline template-literal starts after a closed inline template.Pull Request checklist
Update documentationUpdate CHANGELOG fileOther Information
Validated with
bundle exec rspec react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb:219 react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb:1007 react_on_rails/spec/react_on_rails/generators/pro_generator_spec.rb:1019.Note
Medium Risk
Touches the Pro install generator’s Gemfile and JS source rewriting logic; subtle parsing/state bugs could cause incorrect dependency swaps or missed/over-eager import rewrites in real-world edge cases.
Overview
Improves the Pro generator’s Gemfile swap to correctly consume non-parenthesized multiline
gem "react_on_rails"declarations and carry forward trailing options/guards while stripping only version constraints, avoiding orphaned lines and stray/trailing commas.Hardens JS module-specifier rewriting around template literals by restoring inline template placeholders without altering escaped sequences, and by more accurately detecting multiline template-literal boundaries across inline literals and block-comment transitions (so imports between these regions are rewritten correctly). Adds focused specs for these multiline Gemfile and template-literal edge cases.
Written by Cursor Bugbot for commit 5a6e361. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
Bug Fixes
Tests