fix: Ruby 3.4 compat — remove strip_heredoc, add missing GitUtils require#2599
fix: Ruby 3.4 compat — remove strip_heredoc, add missing GitUtils require#2599
Conversation
…uire Replace all `<<-X.strip_heredoc` with `<<~X` (squiggly heredoc) and remove redundant `.strip_heredoc` from `<<~X.strip_heredoc` patterns. The `strip_heredoc` method from ActiveSupport is no longer automatically available in Ruby 3.4, causing `undefined method` errors at runtime. Also add missing `require "react_on_rails/git_utils"` to the install generator, which caused `uninitialized constant ReactOnRails::GitUtils` during Pro install flows. Fixes #2598 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
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:
WalkthroughStandardized many heredocs to squiggly heredocs (<<~), added Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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)
📝 Coding Plan
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 |
Review: Ruby 3.4
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Greptile SummaryThis PR fixes two Ruby 3.4 compatibility issues: it mechanically replaces all Key changes:
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Rails as Rails CLI
participant IG as InstallGenerator
participant GU as ReactOnRails::GitUtils
participant GM as GeneratorMessages
Rails->>IG: rails generate react_on_rails:install
Note over IG: require "react_on_rails/git_utils"<br/>(NEW — fixes uninitialized constant crash)
IG->>IG: installation_prerequisites_met?
IG->>GU: uncommitted_changes?(GeneratorMessages)
GU-->>IG: true / false
alt uncommitted changes detected
GU->>GM: add_error("You have uncommitted changes...")
IG-->>Rails: prerequisites not met
else clean working tree
IG->>IG: invoke_generators
IG-->>Rails: installation complete
end
Last reviewed commit: 8a8d81a |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/react_on_rails/helper.rb`:
- Around line 234-237: The code currently assumes renderingError is an Error and
directly reads renderingError.message and renderingError.stack; change
renderingErrorObject creation to first guard that renderingError is an object
and not null (and has message/stack properties) before accessing them, otherwise
populate renderingErrorObject.message with String(renderingError) (or a fallback
like "Non-error thrown") and set stack to undefined/empty; ensure this logic is
used where throw_js_errors is false so non-Error throws (null, undefined,
primitives) do not cause a secondary exception while serializing renderingError.
🪄 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: e41a936e-23ca-4287-a91f-067e12d14577
📒 Files selected for processing (19)
react_on_rails/lib/generators/react_on_rails/base_generator.rbreact_on_rails/lib/generators/react_on_rails/install_generator.rbreact_on_rails/lib/react_on_rails/configuration.rbreact_on_rails/lib/react_on_rails/helper.rbreact_on_rails/lib/react_on_rails/locales/base.rbreact_on_rails/lib/react_on_rails/locales/to_js.rbreact_on_rails/lib/react_on_rails/packer_utils.rbreact_on_rails/lib/react_on_rails/pro_helper.rbreact_on_rails/lib/react_on_rails/server_rendering_pool/ruby_embedded_java_script.rbreact_on_rails/lib/react_on_rails/test_helper.rbreact_on_rails/spec/dummy/spec/helpers/react_on_rails_helper_spec.rbreact_on_rails/spec/react_on_rails/support/generator_spec_helper.rbreact_on_rails/spec/react_on_rails/support/shared_examples/base_generator_examples.rbreact_on_rails/spec/react_on_rails/utils_spec.rbreact_on_rails_pro/lib/react_on_rails_pro/license_public_key.rbreact_on_rails_pro/rakelib/public_key_management.rakereact_on_rails_pro/spec/dummy/app/controllers/application_controller.rbreact_on_rails_pro/spec/dummy/app/controllers/pages_controller.rbreact_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb
Review SummaryOverall this PR looks correct and well-scoped. The mechanical heredoc replacement is a clean Ruby 3.4 compatibility fix, and the missing require addition resolves a legitimate load-order bug in the install generator. What looks good:
Minor issues:
No blocking issues — the core fixes are correct and low-risk. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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_pro/spec/dummy/app/controllers/application_controller.rb`:
- Around line 45-46: Remove the debug stdout prints in ApplicationController:
delete the two debug lines "puts \"js_redirect\"" and "puts js_redirect" (inside
the js_redirect handler/method in ApplicationController) so no debug output is
emitted in production; keep the original redirect/response behavior intact and
run tests to ensure no behavior changed.
🪄 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: 97dfed15-ee1a-476c-8797-3ca2afa7b310
📒 Files selected for processing (5)
react_on_rails/lib/generators/react_on_rails/install_generator.rbreact_on_rails/lib/react_on_rails/helper.rbreact_on_rails/spec/dummy/spec/helpers/react_on_rails_helper_spec.rbreact_on_rails_pro/lib/react_on_rails_pro/license_public_key.rbreact_on_rails_pro/spec/dummy/app/controllers/application_controller.rb
🚧 Files skipped from review as they are similar to previous changes (2)
- react_on_rails/lib/generators/react_on_rails/install_generator.rb
- react_on_rails/lib/react_on_rails/helper.rb
|
Overall this is a clean and correct PR. The mechanical strip_heredoc to squiggly heredoc replacements are semantically equivalent and idiomatic for Ruby 2.3+. The require react_on_rails/git_utils fix addresses a real generator load-order issue. Substantive change beyond the stated scope: helper.rb includes a genuine bug fix in server_render_js beyond the formatting change. The old code accessed renderingError.message and renderingError.stack directly, which throws TypeError if anything other than an Error object is thrown (e.g. throw null, throw 'string', throw 42). The new code defensively checks typeof renderingError === 'object' before accessing those properties, and falls back to String(renderingError). A corresponding test is added. Worth calling out in the PR description since it is a real behaviour change. Debug output removed: application_controller.rb (pro spec dummy) removes leftover puts debug statements and refactors the rescue block into private methods — good housekeeping. Minor nit: The comment added to install_generator.rb explaining the load-path require is helpful context. |
Review SummaryOverall this is a solid, well-scoped fix for Ruby 3.4 compatibility. The mechanical Must-fixCHANGELOG — The single entry only documents the JS error-serialization improvement. The two Ruby 3.4 compat fixes (heredoc syntax + missing Worth addressing
Looks good
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Around line 28-31: Replace the incorrect Unreleased bullet about
`server_render_js` with a concise entry describing the actual user-visible fixes
from this PR: note the Ruby 3.4 compatibility fix for `strip_heredoc` and the
addition of an explicit `require "react_on_rails/git_utils"` (or
`ReactOnRails::GitUtils` require) in the install generator; remove the unrelated
`server_render_js` sentence and ensure the new changelog line references that
these are bugfixes and cites the PR/author as the existing entry format does.
🪄 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: 396d0240-952a-4351-a15f-f0493274fe03
📒 Files selected for processing (2)
CHANGELOG.mdreact_on_rails/spec/dummy/spec/helpers/react_on_rails_helper_spec.rb
🚧 Files skipped from review as they are similar to previous changes (1)
- react_on_rails/spec/dummy/spec/helpers/react_on_rails_helper_spec.rb
|
PR Review: Solid Ruby 3.4 compatibility fix. Heredoc conversion is mechanical and correct, GitUtils require fix is legitimate, non-Error throw hardening in server_render_js addresses a real crash path. Positives: squiggly heredoc is semantically identical to strip_heredoc in every case, removing .strip_heredoc after squiggly PEM heredoc is correct, application_controller.rb refactoring removes debug puts and adds ensure for stream closure. Issues noted in inline comments: (1) rescue ExecJS::ProgramError before rescue StandardError looks like a no-op but is load-bearing - without it ProgramError falls into the StandardError branch and gets re-wrapped. Needs a clarifying comment. (2) eval comment says no user-provided input but generatedCode contains the interpolated js_expression. (3) ExecJS.compile bootstrap duplicated verbatim in both test cases, should be a shared let block. (4) Minor: extra blank line in generated JS in server_render_js. |
Review: Ruby 3.4 Compatibility — strip_heredoc + GitUtils + non-Error SSR throwsOverall this is a well-scoped, low-risk fix. The three concerns it addresses are real and the solutions are mechanically correct. A few notes below. ✅ What's solid
|
|
Overall: Clean, well-motivated fix for real Ruby 3.4 breakage. Most changes are mechanical and correct. Heredoc conversions: All replacements look correct. One case in license_public_key.rb is not purely mechanical (see inline comment) -- the old code had inconsistent indentation where the PEM header lines were indented further than the body content, so those lines still carried extra leading spaces after <<~PEM normalization. The new code aligns all PEM lines consistently, which is a latent correctness fix worth noting in the PR description. server_render_js non-Error throw handling: Meaningful safety improvement. The old code accessed renderingError.message and renderingError.stack unconditionally, which raises TypeError for throw null, throw 42, throw string primitives, etc. The new typeof/in guards handle all these cases correctly. GitUtils require fix: Correct. Generators run in a standalone context and cannot rely on full library boot loading ReactOnRails::GitUtils as a side effect. ApplicationController refactor: Clean extraction into private methods. Moving response.stream.close into an ensure block is a real resource-safety fix -- the stream is now guaranteed to close even if an exception occurs while writing the error HTML. The backtrace safe-navigation fix is also correct (backtrace can be nil for some exception types). Tests: The new spec covering throw null, plain-object throws, Error throws, and re-throw behavior when throw_js_errors is true is solid. Validating the generated JS behavior through ExecJS before Ruby post-processing is a good approach. |
…uire (#2599) ## Summary - Replace all `<<-X.strip_heredoc` with `<<~X` (squiggly heredoc) across 19 files - Remove redundant `.strip_heredoc` from `<<~X.strip_heredoc` patterns (e.g., `license_public_key.rb`, `pro_helper.rb`) - Add missing `require "react_on_rails/git_utils"` to `install_generator.rb` - Harden `server_render_js` non-Error throw serialization and add focused regression coverage ## Problem On Ruby 3.4, `String#strip_heredoc` from ActiveSupport is no longer automatically available, causing: 1. `undefined method 'strip_heredoc'` in `react_on_rails_pro/license_public_key.rb` — Pro gem fails to load 2. `uninitialized constant ReactOnRails::GitUtils` in the install generator — Pro install flow crashes ## Fix - **strip_heredoc**: Ruby 2.3+ natively supports `<<~X` (squiggly heredoc) which does the same thing. This is a mechanical replacement with no behavior change. - **GitUtils require**: The install generator uses `ReactOnRails::GitUtils` at line 193 but never requires it. The main library loads it via `react_on_rails.rb`, but generators run in a context where the full library isn't loaded. ## Test plan - [x] RuboCop passes on all 19 changed files (verified locally + pre-push hook) - [x] Ruby syntax check passes on all Pro files - [ ] CI should validate no regressions Fixes #2598 (items 1 and 2 — strip_heredoc and GitUtils) > Note: Item 3 from the issue (RSC postcondition checks in `create-react-on-rails-app --rsc`) should be a separate PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Mostly mechanical heredoc syntax changes with minimal behavioral impact; small risk limited to generators/test helpers if indentation/whitespace expectations differ. > > **Overview** > Improves Ruby 3.4 compatibility by replacing `<<-...strip_heredoc` patterns with `<<~...` squiggly heredocs (and removing redundant `.strip_heredoc` chaining) across core helpers, configuration/messages, locale exporters, and specs. > > Fixes the `react_on_rails:install` generator by explicitly requiring `react_on_rails/git_utils`, preventing `ReactOnRails::GitUtils` constant load errors when the generator runs outside the full library load path. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 8a8d81a. 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 * **New Features** * Safer server-side rendering error handling: non-Error throws are serialized defensively, optional re-throwing, and structured error details returned with render output. * **Chores** * Standardized multiline string formatting across the codebase for consistent indentation and Ruby 3.4 compatibility. * Improved installer/load-path robustness. * **Tests** * Added/updated tests covering server-render error serialization and adjusted string expectations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…uire (#2599) ## Summary - Replace all `<<-X.strip_heredoc` with `<<~X` (squiggly heredoc) across 19 files - Remove redundant `.strip_heredoc` from `<<~X.strip_heredoc` patterns (e.g., `license_public_key.rb`, `pro_helper.rb`) - Add missing `require "react_on_rails/git_utils"` to `install_generator.rb` - Harden `server_render_js` non-Error throw serialization and add focused regression coverage ## Problem On Ruby 3.4, `String#strip_heredoc` from ActiveSupport is no longer automatically available, causing: 1. `undefined method 'strip_heredoc'` in `react_on_rails_pro/license_public_key.rb` — Pro gem fails to load 2. `uninitialized constant ReactOnRails::GitUtils` in the install generator — Pro install flow crashes ## Fix - **strip_heredoc**: Ruby 2.3+ natively supports `<<~X` (squiggly heredoc) which does the same thing. This is a mechanical replacement with no behavior change. - **GitUtils require**: The install generator uses `ReactOnRails::GitUtils` at line 193 but never requires it. The main library loads it via `react_on_rails.rb`, but generators run in a context where the full library isn't loaded. ## Test plan - [x] RuboCop passes on all 19 changed files (verified locally + pre-push hook) - [x] Ruby syntax check passes on all Pro files - [ ] CI should validate no regressions Fixes #2598 (items 1 and 2 — strip_heredoc and GitUtils) > Note: Item 3 from the issue (RSC postcondition checks in `create-react-on-rails-app --rsc`) should be a separate PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Mostly mechanical heredoc syntax changes with minimal behavioral impact; small risk limited to generators/test helpers if indentation/whitespace expectations differ. > > **Overview** > Improves Ruby 3.4 compatibility by replacing `<<-...strip_heredoc` patterns with `<<~...` squiggly heredocs (and removing redundant `.strip_heredoc` chaining) across core helpers, configuration/messages, locale exporters, and specs. > > Fixes the `react_on_rails:install` generator by explicitly requiring `react_on_rails/git_utils`, preventing `ReactOnRails::GitUtils` constant load errors when the generator runs outside the full library load path. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 8a8d81a. 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 * **New Features** * Safer server-side rendering error handling: non-Error throws are serialized defensively, optional re-throwing, and structured error details returned with render output. * **Chores** * Standardized multiline string formatting across the codebase for consistent indentation and Ruby 3.4 compatibility. * Improved installer/load-path robustness. * **Tests** * Added/updated tests covering server-render error serialization and adjusted string expectations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Summary
<<-X.strip_heredocwith<<~X(squiggly heredoc) across 19 files.strip_heredocfrom<<~X.strip_heredocpatterns (e.g.,license_public_key.rb,pro_helper.rb)require "react_on_rails/git_utils"toinstall_generator.rbserver_render_jsnon-Error throw serialization and add focused regression coverageProblem
On Ruby 3.4,
String#strip_heredocfrom ActiveSupport is no longer automatically available, causing:undefined method 'strip_heredoc'inreact_on_rails_pro/license_public_key.rb— Pro gem fails to loaduninitialized constant ReactOnRails::GitUtilsin the install generator — Pro install flow crashesFix
<<~X(squiggly heredoc) which does the same thing. This is a mechanical replacement with no behavior change.ReactOnRails::GitUtilsat line 193 but never requires it. The main library loads it viareact_on_rails.rb, but generators run in a context where the full library isn't loaded.Test plan
Fixes #2598 (items 1 and 2 — strip_heredoc and GitUtils)
🤖 Generated with Claude Code
Note
Low Risk
Mostly mechanical heredoc syntax changes with minimal behavioral impact; small risk limited to generators/test helpers if indentation/whitespace expectations differ.
Overview
Improves Ruby 3.4 compatibility by replacing
<<-...strip_heredocpatterns with<<~...squiggly heredocs (and removing redundant.strip_heredocchaining) across core helpers, configuration/messages, locale exporters, and specs.Fixes the
react_on_rails:installgenerator by explicitly requiringreact_on_rails/git_utils, preventingReactOnRails::GitUtilsconstant load errors when the generator runs outside the full library load path.Written by Cursor Bugbot for commit 8a8d81a. Configure here.
Summary by CodeRabbit
New Features
Chores
Tests