Move React/Shakapacker version compatibility to generator smoke tests #2125
Conversation
This moves React and Shakapacker version compatibility testing from spec/dummy to the generator smoke tests, as suggested in PR #2114 review. Changes: - Update spec/dummy to always use latest React 19 and Shakapacker 9.4.0 - Add minimum version example apps (basic-minimum, basic-server-rendering-minimum) that use React 18.0.0 and Shakapacker 8.2.0 - Add ExampleType.minimum_versions flag to support version-specific examples - Add rake tasks for filtered testing: - run_rspec:shakapacker_examples_latest (for latest versions only) - run_rspec:shakapacker_examples_minimum (for minimum versions only) - Simplify script/convert to only handle Node.js tooling compatibility (removed React/Shakapacker version modifications) - Update CI workflows to run appropriate examples per dependency level Benefits: - Clearer separation: spec/dummy tests latest, generators test compatibility - Simpler CI configuration for integration tests - Better reflects real-world usage patterns Closes #2123 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
Warning Rate limit exceeded@justin808 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 20 minutes and 43 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdds minimum-version example generator tests and CI split (latest/minimum), bumps React to ^19.0.0 in root and dummy, introduces minimum_versions flags/constants, pins generated examples to minimum React/Shakapacker, updates rake tasks and the conversion script, and adjusts dummy app helmet/thunk usage. Changes
Sequence Diagram(s)sequenceDiagram
participant GHA as GitHub Actions
participant Rake as Rake
participant ExType as ExampleType
participant Gen as Generator
participant Patch as apply_minimum_versions
rect rgba(200,230,255,0.18)
Note over GHA,Rake: Latest path (matrix.dependency-level = latest)
GHA->>Rake: trigger shakapacker_examples_latest
Rake->>ExType: latest_examples()
Rake->>Gen: generate selected examples
Gen-->>Rake: examples generated
Rake->>Rake: run RSpec for latest examples
end
rect rgba(230,255,220,0.18)
Note over GHA,Patch: Minimum path (matrix.dependency-level = minimum)
GHA->>Rake: trigger shakapacker_examples_minimum
Rake->>ExType: minimum_examples()
Rake->>Gen: generate selected examples
Gen-->>Rake: examples generated
Rake->>Patch: apply_minimum_versions(example_dir)
Patch-->>Rake: package.json/Gemfile pinned to min versions
Rake->>Rake: run RSpec for minimum examples
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
Code Review - PR #2125: Move React/Shakapacker version compatibility to generator smoke tests✅ Overall AssessmentThis is a well-structured refactoring that improves the testing strategy by separating version compatibility testing from integration tests. The approach is sound and aligns with the original issue #2123 objectives. 🎯 Strengths1. Clear Separation of Concerns
2. Cleaner CI Configuration
3. Better Real-World Representation
4. Code Quality
|
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Code Review - PR #2125OverviewThis PR successfully refactors React/Shakapacker version compatibility testing from spec/dummy to generator smoke tests. The approach is clean and improves the separation of concerns. Overall, this is well-structured with good benefits. ✅ Strengths
|
Code Review - PR #2125I've reviewed the changes for moving React/Shakapacker version compatibility testing to generator smoke tests. Overall, this is a well-structured refactoring that improves the separation of concerns. Here's my detailed feedback: ✅ Strengths
🔍 Observations & Suggestions1. Testing CoverageThe PR description mentions:
Suggestion: Before merging, verify the CI workflow runs as expected. Consider testing locally with: # Simulate the CI environment
cd react_on_rails
bundle exec rake run_rspec:shakapacker_examples_latest
bundle exec rake run_rspec:shakapacker_examples_minimum2. Potential Edge Case in
|
Code Review - PR #2125SummaryThis PR successfully moves React/Shakapacker version compatibility testing from spec/dummy to generator smoke tests. The overall approach is sound and aligns well with the project's testing strategy. ✅ Strengths
🔍 Issues & Concerns1. CRITICAL: Missing attr_reader and minimum_versions? methodIn attr_reader :packer_type, :name, :generator_options, :minimum_versionsBut the current file only has: attr_reader :packer_type, :name, :generator_optionsThe
Required fix: attr_reader :packer_type, :name, :generator_options, :minimum_versions
def initialize(packer_type: nil, name: nil, generator_options: nil, minimum_versions: false)
@packer_type = packer_type
@name = name
@generator_options = generator_options
@minimum_versions = minimum_versions
self.class.all[packer_type.to_sym] << self
end
def minimum_versions?
@minimum_versions
end2. Missing run_rspec.rake changesThe PR diff shows new rake tasks should be added to
These changes are not in the current file. Without these:
Required fix: Add the rake tasks as shown in the PR diff. 3. Missing shakapacker_examples.rake changesThe PR diff shows a new
This code is not present in the current file. Required fix: Add the 4. Missing examples_config.yml entriesThe current # Missing from current file:
- name: basic-minimum
generator_options: ''
minimum_versions: true
- name: basic-server-rendering-minimum
generator_options: --example-server-rendering
minimum_versions: trueWithout these entries, no minimum version examples will be generated. 🐛 Potential Bugs5. Predicate method naming inconsistencyIn def latest_examples
ExampleType.all[:shakapacker_examples].reject(&:minimum_versions)
end
def minimum_examples
ExampleType.all[:shakapacker_examples].select(&:minimum_versions)
endThis calls
The symbol-to-proc syntax 6. JSON formatting in apply_minimum_versionsThe PR adds: File.write(package_json_path, "\#{JSON.pretty_generate(package_json)}\n")Good: This preserves consistent formatting with a trailing newline (as required by CLAUDE.md). 📝 Code Quality7. Constants placement is goodThe minimum version constants are well-placed: MINIMUM_REACT_VERSION = "18.0.0"
MINIMUM_SHAKAPACKER_VERSION = "8.2.0"These make it easy to update supported versions in the future. 8. Script/convert simplificationThe removal of React/Shakapacker logic from ⚡ Performance ConsiderationsNo performance concerns. The changes are primarily organizational and don't introduce performance overhead. 🔒 Security ConcernsNo security issues identified. The changes involve:
All are low-risk from a security perspective. 🧪 Test CoverageMissing verification:According to the PR description test plan:
The last item is unchecked, but it's the most critical one! Before merging, you should:
🎯 RecommendationsMust fix before merge:
Nice to have:
📋 Checklist for AuthorBefore marking as ready to merge:
💡 Overall AssessmentArchitecture: Excellent - The separation of concerns is much cleaner. Implementation: Incomplete - Several critical code changes are missing. This appears to be an issue with the PR branch not containing all the changes shown in the diff, or the diff was generated from a different state than what's currently in the branch. Recommendation: Request changes - The PR cannot be merged until all the missing code is added and tested locally. Great work on the design! Just need to ensure all the implementation is present. 🚀 |
Code Review - PR #2125Great work on separating version compatibility testing from spec/dummy! This is a cleaner architecture. Here's my detailed review: ✅ Strengths
🔍 Issues & Suggestions1. Missing
|
Improves error handling by catching and logging JSON parse errors when reading package.json during minimum version configuration. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Code Review - PR #2125Overall Assessment: This is a well-structured refactoring that improves the separation of concerns between spec/dummy integration tests and generator smoke tests. The approach is cleaner and more maintainable. ✅ ✅ Strengths
🔍 Issues & Concerns1. Potential Race Condition in Rake Task (CRITICAL)Location: def latest_examples
ExampleType.all[:shakapacker_examples].reject(&:minimum_versions)
end
def minimum_examples
ExampleType.all[:shakapacker_examples].select(&:minimum_versions)
endProblem: These helper methods are defined at the top level of the Recommendation: Move the filtering logic into the task bodies or use lambdas: desc "Runs Rspec for latest version example apps only"
task shakapacker_examples_latest: "shakapacker_examples:gen_all" do
latest_examples = ExampleType.all[:shakapacker_examples].reject(&:minimum_versions)
latest_examples.each { |example_type| Rake::Task[example_type.rspec_task_name].invoke }
endOr ensure 2. Silent Failure Risk in
|
Code Review - PR #2125SummaryThis PR successfully moves React and Shakapacker version compatibility testing from spec/dummy to generator smoke tests. The approach is sound and aligns well with the suggested improvements from PR #2114. ✅ Strengths1. Clear Separation of ConcernsThe PR correctly separates:
This makes the testing strategy much clearer and easier to maintain. 2. Good Architectural Design
3. Simplified CI Logic
4. Proper Version Updates
🔍 Observations & Questions1. Task Invocation Pattern (Minor)In task shakapacker_examples: "shakapacker_examples:gen_all" do
ExampleType.all[:shakapacker_examples].each { |example_type| Rake::Task[example_type.rspec_task_name].invoke }
endThis runs ALL examples (both latest and minimum). Is this intentional? The new tasks Recommendation: Consider whether
2. Dependency Generation OrderIn sh_in_dir(example_type.dir, generator_commands)
# Apply minimum versions for compatibility testing examples
apply_minimum_versions(example_type.dir) if example_type.minimum_versions
sh_in_dir(example_type.dir, "npm install")The 3. Error Handling in JSON ParsingThe rescue JSON::ParserError => e
puts " ERROR: Failed to parse package.json in #{dir}: #{e.message}"
raise
endGood defensive programming! ✅ 4. Workflow Step NamesThe workflow step names were updated from:
To:
This is a good clarification! ✅ 🔒 Security & Performance
📋 Test CoverageBased on the PR description, the following are marked as complete:
Recommendation: Once CI passes, verify that:
🎯 Overall AssessmentLGTM with minor clarification needed on the This is a well-structured PR that:
RecommendationOnce CI passes and the question about 🤖 Generated with Claude Code |
Code Review - PR #2125✅ Overall AssessmentThis is a well-designed refactoring that improves the test architecture by separating concerns: spec/dummy now tests latest versions, while generators test the compatibility matrix. All CI checks are passing. Strengths1. Excellent Separation of Concerns ✨
2. Clean Implementation
3. Improved CI Organization
4. Good Documentation
Issues & Recommendations
|
Code Review for PR #2125SummaryThis PR successfully refactors version compatibility testing from spec/dummy to dedicated generator smoke test examples. The approach is clean and follows the project's architecture well. ✅ Strengths
🔍 Issues Found1. CRITICAL: Missing Implementation in
|
Add a predicate method with ? suffix for boolean checks, following Ruby convention. Update run_rspec.rake to use the predicate method with &:minimum_versions? instead of &:minimum_versions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Review Comments AddressedThank you for the thorough reviews! All the issues mentioned have been addressed in the commits on this branch: ✅ All Critical Issues Resolved
Files Ready for Review
All RuboCop violations have been fixed and the code follows project conventions. |
Code Review: Move React/Shakapacker version compatibility to generator smoke testsOverall Assessment✅ Strong refactoring - This PR successfully separates concerns by moving version compatibility testing from spec/dummy to generator smoke tests. The approach is cleaner and more maintainable. Code Quality & Best Practices✅ Excellent Structure
✅ Helper Methodsdef latest_examples
ExampleType.all[:shakapacker_examples].reject(&:minimum_versions?)
end
def minimum_examples
ExampleType.all[:shakapacker_examples].select(&:minimum_versions?)
endThese filtering methods (lines 94-100 in run_rspec.rake) are clean and readable. 💡 Minor Suggestion - Method LocationThe helper methods
Potential Issues
|
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package.json (1)
31-32: Update@types/reactand@types/react-domto v19 to match React 19 runtime.TypeScript types must be updated when upgrading to React 19. Install @types/react@^19.0.0 and @types/react-dom@^19.0.0. React 19 includes significant TypeScript type changes, with removed types and cleanups that require matching type definitions.
Mismatched versions (v18 types with v19 runtime) prevent builds and cause errors like "It looks like you're trying to use TypeScript but do not have the required package(s) installed." Update lines 31-32 in
package.jsonand align with the correct versions already used inspec/dummy/package.json.
🧹 Nitpick comments (2)
script/convert (1)
15-21: Fallback path logic only handles Gemfile, not package.json.The fallback from
/react_on_rails/Gemfileto/Gemfile(line 19) doesn't apply to the package.json paths used throughout this script. If backward compatibility with the old repository layout is needed for package.json files, consider generalizing the pattern:- old_path = path.sub(%r{/react_on_rails/Gemfile}, "/Gemfile") + # Try removing react_on_rails/ prefix for old layout compatibility + old_path = path.sub(%r{/react_on_rails/}, "/")Alternatively, if the Gemfile fallback is no longer needed, the entire block could be removed to simplify the code.
react_on_rails/rakelib/run_rspec.rake (1)
93-100: Consider moving helper methods earlier in the namespace for better organization.The helper methods work correctly, using
rejectandselectwith theminimum_versions?predicate to filter examples. However, for better readability, consider moving these helpers to the top of the namespace block (after line 12) before they're used.namespace :run_rspec do include ReactOnRails::TaskHelpers + + # Helper methods for filtering examples + def latest_examples + ExampleType.all[:shakapacker_examples].reject(&:minimum_versions?) + end + + def minimum_examples + ExampleType.all[:shakapacker_examples].select(&:minimum_versions?) + end # Loads data from examples_config.yml and instantiates corresponding ExampleType objects examples_config_file = File.expand_path("examples_config.yml", __dir__)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
react_on_rails/spec/dummy/yarn.lockis excluded by!**/yarn.lock,!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (9)
.github/workflows/examples.yml(2 hunks).github/workflows/integration-tests.yml(2 hunks)package.json(1 hunks)react_on_rails/rakelib/example_type.rb(1 hunks)react_on_rails/rakelib/examples_config.yml(1 hunks)react_on_rails/rakelib/run_rspec.rake(1 hunks)react_on_rails/rakelib/shakapacker_examples.rake(2 hunks)react_on_rails/spec/dummy/package.json(1 hunks)script/convert(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
**/*: CRITICAL: Ensure all files end with a newline character
When making changes to directory structure or file paths, run comprehensive grep searches to find all affected files before committing:grep -r 'old/path' . --exclude-dir=node_modules --exclude-dir=.git
Files:
package.jsonreact_on_rails/spec/dummy/package.jsonreact_on_rails/rakelib/examples_config.ymlreact_on_rails/rakelib/shakapacker_examples.rakereact_on_rails/rakelib/example_type.rbreact_on_rails/rakelib/run_rspec.rakescript/convert
**/*.{js,jsx,ts,tsx,json,yaml,yml,css,scss}
📄 CodeRabbit inference engine (CLAUDE.md)
NEVER manually format code - let Prettier and RuboCop handle ALL formatting automatically via
rake autofix
Files:
package.jsonreact_on_rails/spec/dummy/package.jsonreact_on_rails/rakelib/examples_config.yml
package.json
📄 CodeRabbit inference engine (CLAUDE.md)
Never run
npmcommands - only use equivalent Yarn Classic commands
Files:
package.json
.github/workflows/*.yml
📄 CodeRabbit inference engine (CLAUDE.md)
When modifying CI workflows, test all changes locally first via manual execution of equivalent commands before pushing - CI iteration is expensive (10-30 min/cycle)
Files:
.github/workflows/integration-tests.yml.github/workflows/examples.yml
**/*.rb
📄 CodeRabbit inference engine (CLAUDE.md)
CRITICAL: Always run
bundle exec rubocopand fix ALL violations before every commit/push
Files:
react_on_rails/rakelib/example_type.rb
🧠 Learnings (18)
📓 Common learnings
Learnt from: alexeyr-ci
Repo: shakacode/react_on_rails PR: 1687
File: spec/dummy/package.json:0-0
Timestamp: 2025-01-23T18:20:45.824Z
Learning: When adding or updating dependencies in spec/dummy/package.json, maintain version consistency with other package.json files in the codebase to avoid potential version conflicts.
📚 Learning: 2025-11-25T04:54:35.286Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T04:54:35.286Z
Learning: Applies to react_on_rails_pro/**/*.{js,jsx,ts,tsx,json,yaml,yml,css,scss} : The react_on_rails_pro directory has its own Prettier and ESLint configuration - CI lints both directories separately
Applied to files:
package.jsonreact_on_rails/spec/dummy/package.jsonreact_on_rails/rakelib/examples_config.ymlreact_on_rails/rakelib/shakapacker_examples.rakescript/convert
📚 Learning: 2025-11-25T04:54:35.286Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T04:54:35.286Z
Learning: Applies to packages/react-on-rails/src/**/*.{ts,tsx} : TypeScript files in the npm package must be compiled to JavaScript in `packages/react-on-rails/lib/` via `yarn run build`
Applied to files:
package.json.github/workflows/integration-tests.ymlreact_on_rails/spec/dummy/package.jsonreact_on_rails/rakelib/examples_config.ymlreact_on_rails/rakelib/shakapacker_examples.rakereact_on_rails/rakelib/example_type.rbreact_on_rails/rakelib/run_rspec.rakescript/convert
📚 Learning: 2025-09-16T08:01:11.146Z
Learnt from: justin808
Repo: shakacode/react_on_rails PR: 1770
File: lib/generators/react_on_rails/templates/base/base/app/javascript/src/HelloWorld/ror_components/HelloWorld.client.jsx:2-2
Timestamp: 2025-09-16T08:01:11.146Z
Learning: React on Rails uses webpack CSS Modules configuration with namedExports: true, which requires the import syntax `import * as style from './file.module.css'` rather than the default export pattern. This configuration enables better tree shaking and bundle size optimization for CSS modules.
Applied to files:
package.jsonreact_on_rails/spec/dummy/package.jsonreact_on_rails/rakelib/examples_config.ymlreact_on_rails/rakelib/example_type.rbscript/convert
📚 Learning: 2025-04-26T21:55:55.874Z
Learnt from: alexeyr-ci2
Repo: shakacode/react_on_rails PR: 1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
Applied to files:
package.jsonreact_on_rails/spec/dummy/package.jsonreact_on_rails/rakelib/examples_config.ymlreact_on_rails/rakelib/shakapacker_examples.rakereact_on_rails/rakelib/example_type.rbscript/convert
📚 Learning: 2025-02-12T16:38:06.537Z
Learnt from: Romex91
Repo: shakacode/react_on_rails PR: 1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.
Applied to files:
package.jsonreact_on_rails/spec/dummy/package.jsonreact_on_rails/rakelib/examples_config.ymlreact_on_rails/rakelib/shakapacker_examples.rakescript/convert
📚 Learning: 2025-11-25T04:54:35.286Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T04:54:35.286Z
Learning: Applies to spec/react_on_rails/**/*.rb : Ruby unit tests should be located in `spec/react_on_rails/` and tested via `rake run_rspec:gem`
Applied to files:
package.jsonreact_on_rails/spec/dummy/package.jsonreact_on_rails/rakelib/examples_config.ymlreact_on_rails/rakelib/run_rspec.rakescript/convert
📚 Learning: 2025-11-25T04:54:35.286Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T04:54:35.286Z
Learning: Use `yalc` for local development when testing React on Rails with external applications - this allows local package linking without publishing to npm
Applied to files:
package.jsonreact_on_rails/spec/dummy/package.json
📚 Learning: 2025-11-25T04:54:35.286Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T04:54:35.286Z
Learning: CRITICAL: For infrastructure/config changes affecting directory structure, build configs, or CI workflows, perform comprehensive local testing before pushing including: grep for affected files, test build pipeline, run relevant specs, and run full lint check
Applied to files:
.github/workflows/integration-tests.ymlreact_on_rails/rakelib/examples_config.yml.github/workflows/examples.ymlscript/convert
📚 Learning: 2025-11-25T04:54:35.286Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T04:54:35.286Z
Learning: Applies to .github/workflows/*.yml : When modifying CI workflows, test all changes locally first via manual execution of equivalent commands before pushing - CI iteration is expensive (10-30 min/cycle)
Applied to files:
.github/workflows/integration-tests.yml.github/workflows/examples.yml
📚 Learning: 2025-01-23T18:20:45.824Z
Learnt from: alexeyr-ci
Repo: shakacode/react_on_rails PR: 1687
File: spec/dummy/package.json:0-0
Timestamp: 2025-01-23T18:20:45.824Z
Learning: When adding or updating dependencies in spec/dummy/package.json, maintain version consistency with other package.json files in the codebase to avoid potential version conflicts.
Applied to files:
react_on_rails/spec/dummy/package.jsonreact_on_rails/rakelib/shakapacker_examples.rakescript/convert
📚 Learning: 2025-11-25T04:54:35.286Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T04:54:35.286Z
Learning: Applies to lib/react_on_rails/**/*.rb : When creating new Ruby files in `lib/react_on_rails/`, create corresponding RBS signature file in `sig/react_on_rails/` and add to Steepfile for type checking
Applied to files:
react_on_rails/rakelib/examples_config.ymlreact_on_rails/rakelib/shakapacker_examples.rakereact_on_rails/rakelib/example_type.rbscript/convert
📚 Learning: 2025-02-18T13:08:01.477Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation in React on Rails Pro is handled through a chain of validations:
1. Pro version check in `run_stream_inside_fiber`
2. RSC support check during pack generation via `ReactOnRailsPro.configuration.enable_rsc_support`
3. RSC support validation during component registration
This makes additional validation in the helper methods unnecessary.
Applied to files:
react_on_rails/rakelib/examples_config.ymlscript/convert
📚 Learning: 2025-11-25T04:54:35.286Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T04:54:35.286Z
Learning: Applies to lib/generators/react_on_rails/**/*.rb : Rails generators run in host app context and should not assume specific project structure (e.g., `app/javascript/` may not exist) - use `Rails.root` for path resolution
Applied to files:
react_on_rails/rakelib/shakapacker_examples.rakescript/convert
📚 Learning: 2025-11-25T04:54:35.286Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T04:54:35.286Z
Learning: Applies to **/*.{js,jsx,ts,tsx,json,yaml,yml,css,scss} : NEVER manually format code - let Prettier and RuboCop handle ALL formatting automatically via `rake autofix`
Applied to files:
react_on_rails/rakelib/shakapacker_examples.rakescript/convert
📚 Learning: 2025-11-25T04:54:35.286Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T04:54:35.286Z
Learning: React on Rails is a Rails Engine - automatically loads all `.rake` files from `lib/tasks/` directory without explicit loading via `rake_tasks` block to avoid duplicate task execution
Applied to files:
react_on_rails/rakelib/run_rspec.rake
📚 Learning: 2025-11-25T04:54:35.286Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T04:54:35.286Z
Learning: Applies to lib/tasks/*.rake : Rake tasks in lib/tasks/ are automatically loaded by Rails::Engine - verify tasks don't cause duplicate execution during `rake assets:precompile`
Applied to files:
react_on_rails/rakelib/run_rspec.rake
📚 Learning: 2025-11-25T04:54:35.286Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T04:54:35.286Z
Learning: Applies to spec/dummy/**/*.rb : Integration tests in the dummy app should be located in `spec/dummy/` and tested via `rake run_rspec:dummy`
Applied to files:
react_on_rails/rakelib/run_rspec.rake
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: rspec-package-tests (3.4, latest)
- GitHub Check: examples (3.4, latest)
- GitHub Check: pro-lint-js-and-ruby
- GitHub Check: build-dummy-app-webpack-test-bundles
- GitHub Check: build-dummy-app-webpack-test-bundles
- GitHub Check: build
- GitHub Check: claude-review
🔇 Additional comments (11)
react_on_rails/rakelib/example_type.rb (1)
16-31: LGTM!Clean implementation following Ruby conventions:
- Constants are appropriately scoped within the class
- Predicate method
minimum_versions?follows Ruby idioms- Default parameter
minimum_versions: falsemaintains backward compatibilityscript/convert (1)
40-51: LGTM!Appropriately skips RSC-related tests when running on React 18, as RSC requires React 19. The
exit 0no-op fortest:rscis a clean approach..github/workflows/integration-tests.yml (2)
135-137: LGTM!Step name accurately reflects the updated purpose of
script/convert(Node.js tooling compatibility rather than Shakapacker v6 support).
214-216: Consistent rename with the other workflow step.react_on_rails/rakelib/shakapacker_examples.rake (3)
11-11: LGTM!The
require "json"statement is necessary for the newapply_minimum_versionsmethod that parses and writes package.json files.
82-83: LGTM!The conditional invocation of
apply_minimum_versionscorrectly applies minimum version constraints only to examples marked withminimum_versions: true.
19-48: All dependencies verified and confirmed in place.The verification confirms:
minimum_versions?predicate is properly defined inExampleType(line 23 in example_type.rb)- Constants
MINIMUM_REACT_VERSION("18.0.0") andMINIMUM_SHAKAPACKER_VERSION("8.2.0") are defined (lines 17-18 in example_type.rb)- The
apply_minimum_versionsmethod correctly uses these constants to update package.json filesThe method logic is sound with appropriate error handling and safe navigation checks.
.github/workflows/examples.yml (2)
133-133: LGTM!The step name update accurately reflects the script's broader purpose of handling Node.js tooling compatibility.
175-180: Conditional CI workflow changes verified and correct.The conditional steps properly split testing based on
matrix.dependency-level. Verification confirms:
- Both rake tasks exist in
react_on_rails/rakelib/run_rspec.rake(lines 103, 108)- Matrix configuration properly defines
dependency-levelvalues as 'latest' and 'minimum' (line 81)- Conditional syntax correctly matches matrix values
- Environment setup responds appropriately to dependency-level (BUNDLE_FROZEN, line 105)
react_on_rails/rakelib/examples_config.yml (1)
10-16: LGTM!The new minimum-version test configurations are well-structured and clearly documented. The
minimum_versions: trueflag properly integrates with the new testing infrastructure to verify compatibility with React 18.0.0 and Shakapacker 8.2.0.react_on_rails/rakelib/run_rspec.rake (1)
102-110: LGTM!The new rake tasks properly separate latest and minimum version testing. The task structure is consistent with existing patterns, correctly setting up dependencies on generation tasks and invoking RSpec for the appropriate example subsets.
Code ReviewThis is a well-structured PR that successfully moves React/Shakapacker version compatibility testing from spec/dummy to generator smoke tests. The approach is clean and maintainable. Here's my detailed feedback: ✅ Strengths
🔍 Potential Issues1. Critical: Missing RuboCop validationPer CLAUDE.md requirements, you MUST run def apply_minimum_versions(dir) # rubocop:disable Metrics/CyclomaticComplexityThis suggests the method may have complexity issues. Consider:
2. Error handling incompletenessIn File.write(package_json_path, "\#{JSON.pretty_generate(package_json)}\n")This should have error handling for write failures: begin
File.write(package_json_path, "\#{JSON.pretty_generate(package_json)}\n")
puts " Updated package.json with minimum versions:"
# ...
rescue Errno::EACCES, Errno::ENOENT => e
puts " ERROR: Failed to write package.json: \#{e.message}"
raise
end3. Missing test coverage validationPer CLAUDE.md's "CRITICAL - LOCAL TESTING REQUIREMENTS", before claiming tests are fixed:
The test plan shows checkboxes for rake task verification but not for actual test execution. 4. Potential race condition in CIIn - name: Main CI - Latest version examples
if: matrix.dependency-level == 'latest'
run: cd react_on_rails && bundle exec rake run_rspec:shakapacker_examples_latest
- name: Main CI - Minimum version examples
if: matrix.dependency-level == 'minimum'
run: cd react_on_rails && bundle exec rake run_rspec:shakapacker_examples_minimumBoth steps run unconditionally based on matrix values. This is correct, but consider adding a validation step to ensure the examples were generated with the correct versions before running tests. 💡 Suggestions for Improvement1. Add version verificationConsider adding a verification step in the rake task: def verify_minimum_versions(dir)
package_json = JSON.parse(File.read(File.join(dir, "package.json")))
react_version = package_json.dig("dependencies", "react")
shakapacker_version = package_json.dig("devDependencies", "shakapacker") ||
package_json.dig("dependencies", "shakapacker")
unless react_version == MINIMUM_REACT_VERSION && shakapacker_version == MINIMUM_SHAKAPACKER_VERSION
raise "Version mismatch in \#{dir}: React=\#{react_version}, Shakapacker=\#{shakapacker_version}"
end
endCall this after 2. Document the script/convert changesThe script/convert file removes a lot of code. Consider adding a comment or git commit message explaining that React/Shakapacker conversion is now handled by generator examples, not by script/convert. 3. Constants visibilityThe constants
|
Addressing Review Comment #3573928031Thank you for the detailed review! I've addressed the concerns and also made critical React 19 compatibility updates to the spec/dummy app. ✅ Review Concerns Addressed1. Clarification: spec/dummy in minimum CIGood catch! You're right to ask about this. The spec/dummy integration tests should only run with latest versions (React 19, Shakapacker 9.4.0). The minimum CI workflow only runs the generator example tasks ( 2. JSON Parsing Error HandlingThe begin
package_json = JSON.parse(File.read(package_json_path))
rescue JSON::ParserError => e
puts " ERROR: Failed to parse package.json in \#{dir}: \#{e.message}"
raise
endFail-fast behavior is correct for CI - we want to know immediately if generated examples have malformed package.json. 3. Documentation AddedAdded comment in # Minimum version compatibility tests - tests React 18 and Shakapacker 8.2.0
- name: basic-minimum
generator_options: ''
minimum_versions: true🚀 React 19 Compatibility UpdatesI've also updated the spec/dummy app to be fully compatible with React 19, addressing peer dependency issues: Dependencies Updated in
|
| Package | Old Version | New Version | Reason |
|---|---|---|---|
react-redux |
^8.0.2 | ^9.2.0 | React 19 support |
redux |
^4.0.1 | ^5.0.1 | Required by react-redux 9.x |
redux-thunk |
^2.2.0 | ^3.1.0 | Required by redux 5.x |
react-helmet |
^6.1.0 | Replaced with @dr.pogodin/react-helmet@^3.0.4 |
Thread-safe React 19 fork |
Code Changes:
-
redux-thunk import changes: Updated from default export to named export:
import middleware from 'redux-thunk'→import { thunk } from 'redux-thunk'- Files updated:
SharedReduxStore.jsx,ReduxApp.server.jsx,ReduxApp.client.jsx,app-react16/ReduxApp.client.jsx
-
react-helmet migration: Updated to
@dr.pogodin/react-helmetwhich is the React 19 compatible fork:- Uses
HelmetProviderwithcontextprop for SSR (thread-safe) - Updated
ReactHelmetApp.server.jsxandReactHelmetAppBroken.server.jsxwith new SSR pattern - Updated
ReactHelmet.jsxcomponent
- Uses
Verification:
- ✅
yarn installcompletes without peer dependency warnings (only unrelated babel-eslint warning) - ✅
yarn run build:testcompletes successfully - ✅ RuboCop passes with no offenses
Source
The react-helmet migration follows the official Dr. Pogodin React Helmet documentation for server-side rendering with context-based data capture.
Code Review - PR #2125Thank you for this well-structured refactoring! This PR successfully moves React/Shakapacker version compatibility testing from spec/dummy to generator smoke tests. ✅ StrengthsArchitecture & Design
Implementation Quality
🔍 Suggestions & Questions1. RuboCop Disable - Is It Necessary?def apply_minimum_versions(dir) # rubocop:disable Metrics/CyclomaticComplexityThis method appears relatively simple (early return, JSON parse, 2-3 conditionals). Have you verified this disable is actually needed? Try removing it and running RuboCop. 2. JSON FormattingFile.write(package_json_path, "\#{JSON.pretty_generate(package_json)}\n")Good that you're adding the trailing newline! Note that this will reformat the entire package.json (key ordering, spacing). Consider documenting this behavior in a comment if it's intentional. 3. Error Handling EnhancementConsider handling rescue JSON::ParserError => e
puts " ERROR: Invalid JSON in \#{package_json_path}: \#{e.message}"
raise
rescue Errno::ENOENT
puts " ERROR: package.json not found at \#{package_json_path}"
raise4. CI Workflow Edge CaseWhat if 5. Test CoverageAre there tests verifying:
6. DocumentationThe comment in 🧪 Testing Recommendations# Verify rake tasks exist
bundle exec rake -T | grep shakapacker_examples
# Test latest examples
bundle exec rake shakapacker_examples:gen_basic
cat gen-examples/basic/package.json | grep '"react"' # Should be ^19.0.0
# Test minimum examples
bundle exec rake shakapacker_examples:gen_basic-minimum
cat gen-examples/basic-minimum/package.json | grep '"react"' # Should be 18.0.0
# Verify filtering
bundle exec rake run_rspec:shakapacker_examples_latest
bundle exec rake run_rspec:shakapacker_examples_minimum🏁 VerdictOverall: ✅ Approve This is a well-executed refactoring. The architectural approach is sound, implementation is clean, and CI is passing. The suggestions above are minor improvements, not blockers. Great work! 🚀 This approach is much more maintainable than modifying spec/dummy on the fly. Review generated by Claude Code following CLAUDE.md conventions |
Code Review - PR #2125OverviewThis PR successfully moves React and Shakapacker version compatibility testing from the dummy app to dedicated generator smoke tests. The approach is well-architected and aligns with best practices for testing compatibility matrices. ✅ Strengths1. Clear Separation of Concerns
2. Excellent Code Quality
3. CI Integration
4. Helper Methods Are Well Structured def latest_examples
ExampleType.all[:shakapacker_examples].reject(&:minimum_versions?)
end
def minimum_examples
ExampleType.all[:shakapacker_examples].select(&:minimum_versions?)
endThese are readable and leverage Ruby's functional collection methods appropriately. 🔍 Areas for Consideration1. Missing Cleanup in The Current state: # Lines 30-31 - Should these be removed?
gsub_file_content("../react_on_rails/Gemfile.development_dependencies", /gem "shakapacker", "[^"]*"/, 'gem "shakapacker", "8.2.0"')
gsub_file_content("../react_on_rails/spec/dummy/package.json", /"shakapacker": "[^"]*",/, '"shakapacker": "8.2.0",')
# Lines 45-49 - Should these be removed?
gsub_file_content("../package.json", /"react": "[^"]*",/, '"react": "18.0.0",')
gsub_file_content("../package.json", /"react-dom": "[^"]*",/, '"react-dom": "18.0.0",')
gsub_file_content("../react_on_rails/spec/dummy/package.json", /"react": "[^"]*",/, '"react": "18.0.0",')
gsub_file_content("../react_on_rails/spec/dummy/package.json", /"react-dom": "[^"]*",/, '"react-dom": "18.0.0",')Expected: Based on the PR description, 2. Potential Complexity in The method has reasonable complexity (RuboCop disabled with def apply_minimum_versions(dir)
# ...
if package_json["dependencies"]
package_json["dependencies"]["react"] = ExampleType::MINIMUM_REACT_VERSION
package_json["dependencies"]["react-dom"] = ExampleType::MINIMUM_REACT_VERSION
end
if package_json["devDependencies"]&.key?("shakapacker")
package_json["devDependencies"]["shakapacker"] = ExampleType::MINIMUM_SHAKAPACKER_VERSION
elsif package_json["dependencies"]&.key?("shakapacker")
package_json["dependencies"]["shakapacker"] = ExampleType::MINIMUM_SHAKAPACKER_VERSION
end
# ...
endSuggestions:
3. Test Coverage Gaps The test plan in the PR description shows:
Question: Will you be monitoring the CI run to ensure the minimum version examples actually run and pass? This is critical validation that the approach works end-to-end. 4. Documentation Consider adding:
🛡️ Security & PerformanceSecurity: ✅ No security concerns. The JSON parsing includes error handling. Performance: ✅ The filtering operations are performed on small arrays (handful of example types), so performance impact is negligible. 🧪 Testing RecommendationsBefore merging, verify:
📋 Minor Suggestions
🎯 RecommendationApprove with minor revisions This is a solid architectural improvement that makes the codebase more maintainable. The only blocking concern is confirming that All other suggestions are optional refinements. The core logic is sound, well-tested, and follows Ruby best practices. Great work on this refactoring! 🚀 Related: Closes #2123 |
Code ReviewOverall Assessment ✅This is a well-structured PR that successfully moves React/Shakapacker version compatibility testing from spec/dummy to the generator smoke tests. The approach is sound and the implementation is clean. CI is passing successfully. Strengths 💪
Code Quality Issues 🔍1. Missing Implementation in
|
Code Review - PR #2125: Move React/Shakapacker version compatibility to generator smoke testsOverviewThis PR successfully refactors version compatibility testing from spec/dummy to generator smoke tests. The approach is clean and well-architected. Strengths1. Clean Architecture
2. Smart Implementation
3. Simplified Conversion Script
Issues and RecommendationsCRITICAL: Missing Test CoverageProblem: No tests verify the new rake tasks work correctly. Recommendation: Add tests to verify latest_examples and minimum_examples filtering. Why critical: The PR checklist shows "Verify rake tasks are created correctly" but this should be automated, not manual. MEDIUM: CI Workflow TestingIssue: From examples.yml lines 175-179 - conditional execution based on dependency-level Recommendation per CLAUDE.md:
MEDIUM: Potential Race ConditionIssue in shakapacker_examples.rake:56-58 - Modifies package.json AFTER generation but BEFORE npm install Recommendation: Add a comment explaining the ordering to prevent future refactoring issues LOW: Missing RuboCop VerificationFrom CLAUDE.md: ALWAYS run bundle exec rubocop before commit Recommendation: Run locally before merge Security and PerformanceSecurity: No concerns
Performance: Acceptable
Testing ChecklistPer CLAUDE.md "15-Minute Rule": spend 15 minutes testing locally before pushing Before merging, verify:
Recommendations SummaryBefore Merge (Required)
Future Improvements (Optional)
ConclusionOverall: Strong PR with good architecture. Main concern is lack of automated tests for new rake tasks and missing local verification before CI runs. Recommendation: APPROVE with conditions
Great work on the refactoring! The separation of concerns is much cleaner now. |
- Upgrade react-redux from ^8.0.2 to ^9.2.0 for React 19 support
- Upgrade redux from ^4.0.1 to ^5.0.1 (required by react-redux 9.x)
- Upgrade redux-thunk from ^2.2.0 to ^3.1.0 (required by redux 5.x)
- Replace react-helmet@^6.1.0 with @dr.pogodin/react-helmet@^3.0.4
(thread-safe React 19 compatible fork)
Code changes:
- Update redux-thunk imports to use named export: { thunk }
- Update react-helmet SSR to use HelmetProvider with context prop
- Remove @types/react-helmet (new package has built-in types)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
Code Review: PR #2125Great work on this refactor! This is a well-thought-out improvement that simplifies the testing strategy. Here's my detailed feedback: ✅ Strengths
🔍 Potential Issues & Suggestions1. Missing
|
Add HelmetProvider wrapper to client-side entry points for ReactHelmet components. The @dr.pogodin/react-helmet package requires HelmetProvider to wrap all Helmet components, on both server and client sides. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
react_on_rails/spec/dummy/package.json (1)
21-28: LGTM — React 19 ecosystem dependencies updated correctly.All dependencies now support React 19:
- react-redux upgraded to ^9.2.0 (adds React 19 peer dependency support)
- @dr.pogodin/react-helmet ^3.0.4 replaces react-helmet (React 19 compatible)
- redux ^5.0.1 and redux-thunk ^3.1.0 are current stable versions
This addresses the compatibility concerns raised in the previous review.
🧹 Nitpick comments (1)
react_on_rails/spec/dummy/client/app/startup/ReactHelmetAppBroken.server.jsx (1)
31-31: Use optional chaining for safer property access.The current fallback only guards against
helmetbeing undefined. Ifhelmetexists buthelmet.titleis undefined,helmet.title.toString()would throw a TypeError.Apply this diff for more defensive access:
- title: helmet ? helmet.title.toString() : '', + title: helmet?.title?.toString() || '',This ensures the code gracefully handles cases where either
helmetorhelmet.titlemight be undefined.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
react_on_rails/spec/dummy/Gemfile.lockis excluded by!**/*.lockreact_on_rails/spec/dummy/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (10)
react_on_rails/spec/dummy/client/app-react16/startup/ReduxApp.client.jsx(2 hunks)react_on_rails/spec/dummy/client/app/components/ReactHelmet.jsx(1 hunks)react_on_rails/spec/dummy/client/app/startup/ReactHelmetApp.client.jsx(1 hunks)react_on_rails/spec/dummy/client/app/startup/ReactHelmetApp.server.jsx(2 hunks)react_on_rails/spec/dummy/client/app/startup/ReactHelmetAppBroken.client.jsx(1 hunks)react_on_rails/spec/dummy/client/app/startup/ReactHelmetAppBroken.server.jsx(2 hunks)react_on_rails/spec/dummy/client/app/startup/ReduxApp.client.jsx(2 hunks)react_on_rails/spec/dummy/client/app/startup/ReduxApp.server.jsx(2 hunks)react_on_rails/spec/dummy/client/app/stores/SharedReduxStore.jsx(2 hunks)react_on_rails/spec/dummy/package.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
ALWAYS ensure files end with a newline character before committing/pushing
Files:
react_on_rails/spec/dummy/client/app/startup/ReactHelmetAppBroken.client.jsxreact_on_rails/spec/dummy/client/app-react16/startup/ReduxApp.client.jsxreact_on_rails/spec/dummy/client/app/startup/ReactHelmetApp.server.jsxreact_on_rails/spec/dummy/client/app/startup/ReactHelmetApp.client.jsxreact_on_rails/spec/dummy/client/app/startup/ReduxApp.client.jsxreact_on_rails/spec/dummy/client/app/stores/SharedReduxStore.jsxreact_on_rails/spec/dummy/package.jsonreact_on_rails/spec/dummy/client/app/components/ReactHelmet.jsxreact_on_rails/spec/dummy/client/app/startup/ReduxApp.server.jsxreact_on_rails/spec/dummy/client/app/startup/ReactHelmetAppBroken.server.jsx
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.778Z
Learning: Applies to /CHANGELOG.md : Update `/CHANGELOG.md` for user-visible changes (features, bug fixes, breaking changes, deprecations, performance improvements) to the open-source React on Rails gem and npm package
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.778Z
Learning: Applies to spec/dummy/e2e/playwright/**/*.spec.js : Use Rails helpers (`appFactories`, `appEval`) in Playwright tests instead of UI-based setup to leverage factory_bot and arbitrary Ruby code execution
Learnt from: alexeyr-ci
Repo: shakacode/react_on_rails PR: 1687
File: spec/dummy/package.json:0-0
Timestamp: 2025-01-23T18:20:45.824Z
Learning: When adding or updating dependencies in spec/dummy/package.json, maintain version consistency with other package.json files in the codebase to avoid potential version conflicts.
Learnt from: alexeyr-ci2
Repo: shakacode/react_on_rails PR: 1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
📚 Learning: 2025-11-25T08:05:17.778Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.778Z
Learning: Applies to /CHANGELOG.md : Update `/CHANGELOG.md` for user-visible changes (features, bug fixes, breaking changes, deprecations, performance improvements) to the open-source React on Rails gem and npm package
Applied to files:
react_on_rails/spec/dummy/client/app/startup/ReactHelmetAppBroken.client.jsxreact_on_rails/spec/dummy/client/app-react16/startup/ReduxApp.client.jsxreact_on_rails/spec/dummy/client/app/startup/ReactHelmetApp.server.jsxreact_on_rails/spec/dummy/client/app/startup/ReactHelmetApp.client.jsxreact_on_rails/spec/dummy/client/app/startup/ReduxApp.client.jsxreact_on_rails/spec/dummy/client/app/stores/SharedReduxStore.jsxreact_on_rails/spec/dummy/package.jsonreact_on_rails/spec/dummy/client/app/components/ReactHelmet.jsxreact_on_rails/spec/dummy/client/app/startup/ReduxApp.server.jsxreact_on_rails/spec/dummy/client/app/startup/ReactHelmetAppBroken.server.jsx
📚 Learning: 2024-10-08T20:53:47.076Z
Learnt from: theforestvn88
Repo: shakacode/react_on_rails PR: 1620
File: spec/dummy/client/app/startup/HelloTurboStream.jsx:3-3
Timestamp: 2024-10-08T20:53:47.076Z
Learning: The `RailsContext` import in `spec/dummy/client/app/startup/HelloTurboStream.jsx` is used later in the project, as clarified by the user theforestvn88.
Applied to files:
react_on_rails/spec/dummy/client/app/startup/ReactHelmetAppBroken.client.jsxreact_on_rails/spec/dummy/client/app-react16/startup/ReduxApp.client.jsxreact_on_rails/spec/dummy/client/app/startup/ReactHelmetApp.server.jsxreact_on_rails/spec/dummy/client/app/startup/ReactHelmetApp.client.jsxreact_on_rails/spec/dummy/client/app/startup/ReduxApp.client.jsxreact_on_rails/spec/dummy/client/app/stores/SharedReduxStore.jsxreact_on_rails/spec/dummy/client/app/components/ReactHelmet.jsxreact_on_rails/spec/dummy/client/app/startup/ReduxApp.server.jsxreact_on_rails/spec/dummy/client/app/startup/ReactHelmetAppBroken.server.jsx
📚 Learning: 2025-04-26T21:55:55.874Z
Learnt from: alexeyr-ci2
Repo: shakacode/react_on_rails PR: 1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
Applied to files:
react_on_rails/spec/dummy/client/app/startup/ReactHelmetAppBroken.client.jsxreact_on_rails/spec/dummy/client/app-react16/startup/ReduxApp.client.jsxreact_on_rails/spec/dummy/client/app/startup/ReactHelmetApp.server.jsxreact_on_rails/spec/dummy/client/app/startup/ReactHelmetApp.client.jsxreact_on_rails/spec/dummy/client/app/startup/ReduxApp.client.jsxreact_on_rails/spec/dummy/client/app/stores/SharedReduxStore.jsxreact_on_rails/spec/dummy/package.jsonreact_on_rails/spec/dummy/client/app/components/ReactHelmet.jsxreact_on_rails/spec/dummy/client/app/startup/ReduxApp.server.jsxreact_on_rails/spec/dummy/client/app/startup/ReactHelmetAppBroken.server.jsx
📚 Learning: 2025-09-16T08:01:11.146Z
Learnt from: justin808
Repo: shakacode/react_on_rails PR: 1770
File: lib/generators/react_on_rails/templates/base/base/app/javascript/src/HelloWorld/ror_components/HelloWorld.client.jsx:2-2
Timestamp: 2025-09-16T08:01:11.146Z
Learning: React on Rails uses webpack CSS Modules configuration with namedExports: true, which requires the import syntax `import * as style from './file.module.css'` rather than the default export pattern. This configuration enables better tree shaking and bundle size optimization for CSS modules.
Applied to files:
react_on_rails/spec/dummy/client/app/startup/ReactHelmetAppBroken.client.jsxreact_on_rails/spec/dummy/client/app-react16/startup/ReduxApp.client.jsxreact_on_rails/spec/dummy/client/app/startup/ReactHelmetApp.server.jsxreact_on_rails/spec/dummy/client/app/startup/ReactHelmetApp.client.jsxreact_on_rails/spec/dummy/client/app/startup/ReduxApp.client.jsxreact_on_rails/spec/dummy/client/app/stores/SharedReduxStore.jsxreact_on_rails/spec/dummy/package.jsonreact_on_rails/spec/dummy/client/app/components/ReactHelmet.jsxreact_on_rails/spec/dummy/client/app/startup/ReduxApp.server.jsxreact_on_rails/spec/dummy/client/app/startup/ReactHelmetAppBroken.server.jsx
📚 Learning: 2025-02-13T16:50:47.848Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: node_package/src/clientStartup.ts:18-21
Timestamp: 2025-02-13T16:50:47.848Z
Learning: In the react_on_rails module, the `reactOnRailsPageUnloaded` function in clientStartup.ts is intentionally kept private as it's only used internally as a callback for `onPageUnloaded`.
Applied to files:
react_on_rails/spec/dummy/client/app/startup/ReactHelmetAppBroken.client.jsxreact_on_rails/spec/dummy/client/app/startup/ReactHelmetApp.server.jsxreact_on_rails/spec/dummy/client/app/startup/ReactHelmetAppBroken.server.jsx
📚 Learning: 2025-02-13T19:09:15.991Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: node_package/src/RSCWebpackLoader.ts:0-0
Timestamp: 2025-02-13T19:09:15.991Z
Learning: In React Server Components webpack loader, using `new Function('return import("react-server-dom-webpack/node-loader")')()` is necessary as a workaround to bypass TypeScript compilation issues with direct dynamic imports.
Applied to files:
react_on_rails/spec/dummy/client/app/startup/ReactHelmetAppBroken.client.jsxreact_on_rails/spec/dummy/client/app/startup/ReactHelmetApp.server.jsxreact_on_rails/spec/dummy/client/app/startup/ReactHelmetApp.client.jsxreact_on_rails/spec/dummy/package.jsonreact_on_rails/spec/dummy/client/app/startup/ReactHelmetAppBroken.server.jsx
📚 Learning: 2025-07-08T05:57:29.630Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1745
File: node_package/src/RSCRequestTracker.ts:8-14
Timestamp: 2025-07-08T05:57:29.630Z
Learning: The global `generateRSCPayload` function in React on Rails Pro (RORP) is provided by the framework during rendering requests, not implemented in application code. The `declare global` statements are used to document the expected interface that RORP will inject at runtime.
Applied to files:
react_on_rails/spec/dummy/client/app/startup/ReactHelmetApp.server.jsx
📚 Learning: 2025-09-15T21:24:48.207Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1781
File: node_package/src/ClientSideRenderer.ts:82-95
Timestamp: 2025-09-15T21:24:48.207Z
Learning: In React on Rails, the force_load feature includes both explicit `data-force-load="true"` usage and the ability to hydrate components during the page loading state (`document.readyState === 'loading'`). Both capabilities require a Pro license, so the condition `!railsContext.rorPro && (isComponentForceLoaded || document.readyState === 'loading')` correctly gates both scenarios.
Applied to files:
react_on_rails/spec/dummy/client/app/startup/ReactHelmetApp.server.jsx
📚 Learning: 2025-02-12T16:38:06.537Z
Learnt from: Romex91
Repo: shakacode/react_on_rails PR: 1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.
Applied to files:
react_on_rails/spec/dummy/client/app/startup/ReactHelmetApp.client.jsxreact_on_rails/spec/dummy/package.json
📚 Learning: 2025-01-23T18:20:45.824Z
Learnt from: alexeyr-ci
Repo: shakacode/react_on_rails PR: 1687
File: spec/dummy/package.json:0-0
Timestamp: 2025-01-23T18:20:45.824Z
Learning: When adding or updating dependencies in spec/dummy/package.json, maintain version consistency with other package.json files in the codebase to avoid potential version conflicts.
Applied to files:
react_on_rails/spec/dummy/package.json
📚 Learning: 2025-11-25T08:05:17.778Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.778Z
Learning: The `react_on_rails_pro/` directory has its own Prettier/ESLint configuration and must be linted separately
Applied to files:
react_on_rails/spec/dummy/package.json
🧬 Code graph analysis (8)
react_on_rails/spec/dummy/client/app/startup/ReactHelmetAppBroken.client.jsx (1)
react_on_rails/spec/dummy/client/app/startup/ReactHelmetApp.client.jsx (1)
props(10-14)
react_on_rails/spec/dummy/client/app-react16/startup/ReduxApp.client.jsx (3)
react_on_rails/spec/dummy/client/app/startup/ReduxApp.client.jsx (3)
store(37-37)combinedReducer(32-32)combinedProps(33-33)react_on_rails/spec/dummy/client/app/startup/ReduxApp.server.jsx (3)
store(31-31)combinedReducer(26-26)combinedProps(27-27)react_on_rails/spec/dummy/client/app/stores/SharedReduxStore.jsx (1)
combinedReducer(13-13)
react_on_rails/spec/dummy/client/app/startup/ReactHelmetApp.server.jsx (1)
react_on_rails/spec/dummy/client/app/startup/ReactHelmetAppBroken.server.jsx (5)
helmetContext(21-21)helmetContext(27-27)componentHtml(22-26)props(20-34)renderedHtml(29-32)
react_on_rails/spec/dummy/client/app/startup/ReactHelmetApp.client.jsx (1)
react_on_rails/spec/dummy/client/app/startup/ReactHelmetAppBroken.client.jsx (1)
props(10-14)
react_on_rails/spec/dummy/client/app/startup/ReduxApp.client.jsx (2)
react_on_rails/spec/dummy/client/app-react16/startup/ReduxApp.client.jsx (3)
store(32-32)combinedReducer(27-27)combinedProps(28-28)react_on_rails/spec/dummy/client/app/startup/ReduxApp.server.jsx (3)
store(31-31)combinedReducer(26-26)combinedProps(27-27)
react_on_rails/spec/dummy/client/app/stores/SharedReduxStore.jsx (3)
react_on_rails/spec/dummy/client/app-react16/startup/ReduxApp.client.jsx (1)
combinedReducer(27-27)react_on_rails/spec/dummy/client/app/startup/ReduxApp.client.jsx (1)
combinedReducer(32-32)react_on_rails/spec/dummy/client/app/startup/ReduxApp.server.jsx (1)
combinedReducer(26-26)
react_on_rails/spec/dummy/client/app/startup/ReduxApp.server.jsx (3)
react_on_rails/spec/dummy/client/app-react16/startup/ReduxApp.client.jsx (3)
store(32-32)combinedReducer(27-27)combinedProps(28-28)react_on_rails/spec/dummy/client/app/startup/ReduxApp.client.jsx (3)
store(37-37)combinedReducer(32-32)combinedProps(33-33)react_on_rails/spec/dummy/client/app/stores/SharedReduxStore.jsx (1)
combinedReducer(13-13)
react_on_rails/spec/dummy/client/app/startup/ReactHelmetAppBroken.server.jsx (2)
packages/react-on-rails/src/ReactDOMServer.cts (1)
renderToString(4-4)react_on_rails/spec/dummy/client/app/components/ReactHelmet.jsx (1)
ReactHelmet(8-16)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build
- GitHub Check: examples (3.4, latest)
- GitHub Check: examples (3.2, minimum)
- GitHub Check: build-dummy-app-webpack-test-bundles (3.2, 20, minimum)
- GitHub Check: build-dummy-app-webpack-test-bundles (3.4, 22, latest)
🔇 Additional comments (11)
react_on_rails/spec/dummy/client/app/startup/ReduxApp.client.jsx (1)
8-8: LGTM — redux-thunk 3.x migration is correct.The switch from default import to named
{ thunk }export aligns with redux-thunk 3.x API. The middleware usage at line 37 is updated consistently.Also applies to: 37-37
react_on_rails/spec/dummy/client/app-react16/startup/ReduxApp.client.jsx (1)
8-8: LGTM — Consistent redux-thunk 3.x migration in React 16 variant.The named
{ thunk }import and middleware usage align with the main app/ variant and redux-thunk 3.x API.Also applies to: 32-32
react_on_rails/spec/dummy/client/app/startup/ReduxApp.server.jsx (1)
9-9: LGTM — Server-side redux-thunk migration is correct.The named import and curried middleware application pattern are both valid for redux-thunk 3.x and consistent with the client-side changes.
Also applies to: 31-31
react_on_rails/spec/dummy/client/app/components/ReactHelmet.jsx (1)
2-2: LGTM — Helmet migration with clear documentation.The switch to
@dr.pogodin/react-helmetis correct for React 19 compatibility. The added comments clearly document the HelmetProvider requirement for both client and server rendering.Also applies to: 5-7
react_on_rails/spec/dummy/client/app/stores/SharedReduxStore.jsx (1)
2-2: LGTM — Shared store migration is consistent.The redux-thunk 3.x named export and middleware usage are correct and align with other Redux store configurations in the codebase.
Also applies to: 15-15
react_on_rails/spec/dummy/client/app/startup/ReactHelmetApp.client.jsx (1)
3-3: LGTM — Client-side HelmetProvider integration is correct.The HelmetProvider wrapper is required by
@dr.pogodin/react-helmetfor both client and server rendering. The implementation and documentation are correct.Also applies to: 9-14
react_on_rails/spec/dummy/client/app/startup/ReactHelmetApp.server.jsx (1)
4-4: LGTM — Server-side helmet rendering migrated correctly.The context-based HelmetProvider pattern is the proper approach for
@dr.pogodin/react-helmetSSR. This replaces the oldHelmet.renderStatic()global approach with thread-safe per-request context extraction.Also applies to: 19-39
react_on_rails/spec/dummy/client/app/startup/ReactHelmetAppBroken.client.jsx (2)
3-4: LGTM: React 19-compatible helmet library migration.The migration from
react-helmetto@dr.pogodin/react-helmetis correct. This fork provides React 19 compatibility as documented in the PR objectives.
9-14: LGTM: HelmetProvider wrapper correctly applied.The HelmetProvider wrapper is correctly implemented for client-side rendering, consistent with the pattern in ReactHelmetApp.client.jsx. The explanatory comment on line 9 is helpful.
react_on_rails/spec/dummy/client/app/startup/ReactHelmetAppBroken.server.jsx (2)
6-6: LGTM: Server-side helmet library migration.The import of
HelmetProviderfrom@dr.pogodin/react-helmetis correct for React 19 compatibility.
21-27: LGTM: Correct server-side HelmetProvider pattern.The helmetContext pattern correctly implements the @dr.pogodin/react-helmet server-side rendering approach. The context object is populated during rendering and then extracted for use in the returned HTML structure.
…lity Shakapacker 8.2.0 requires webpack-assets-manifest ^5.x, but the generator creates apps with ^6.x. The apply_minimum_versions function now also downgrades webpack-assets-manifest to be compatible with Shakapacker 8.2.0. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
The minimum version testing was failing because: 1. Gemfile had 'shakapacker >= 8.2.0' which installs latest (9.4.0) 2. package.json was downgraded to shakapacker 8.2.0 3. The version mismatch caused Shakapacker to error on rake tasks Solution: - apply_minimum_versions now also updates Gemfile to pin shakapacker to the exact minimum version (8.2.0) - Re-run bundle install after updating Gemfile to install the pinned version - This ensures both gem and npm package are at the same version 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
react_on_rails/rakelib/shakapacker_examples.rake (1)
19-73: Good implementation with JSON error handling; consider extracting webpack-assets-manifest version to a constant.The method correctly handles JSON parsing errors (re-raising with context for CI), updates both package.json and Gemfile, and provides clear output. The logic for checking both
depsanddev_depsis thorough.However, the webpack-assets-manifest version
"^5.0.6"is hardcoded on lines 40 and 44, while other versions use named constants (MINIMUM_REACT_VERSION,MINIMUM_SHAKAPACKER_VERSION). For consistency and easier maintenance, consider extracting this to a constant likeMINIMUM_WEBPACK_ASSETS_MANIFEST_VERSIONinExampleType.Example:
# In example_type.rb MINIMUM_WEBPACK_ASSETS_MANIFEST_VERSION = "^5.0.6"Then update lines 40 and 44:
- deps["webpack-assets-manifest"] = "^5.0.6" if deps.key?("webpack-assets-manifest") + deps["webpack-assets-manifest"] = ExampleType::MINIMUM_WEBPACK_ASSETS_MANIFEST_VERSION if deps.key?("webpack-assets-manifest")- dev_deps["webpack-assets-manifest"] = "^5.0.6" if dev_deps&.key?("webpack-assets-manifest") + dev_deps["webpack-assets-manifest"] = ExampleType::MINIMUM_WEBPACK_ASSETS_MANIFEST_VERSION if dev_deps&.key?("webpack-assets-manifest")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
react_on_rails/rakelib/shakapacker_examples.rake(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{rb,rake}
📄 CodeRabbit inference engine (CLAUDE.md)
ALWAYS run
bundle exec rubocopand fix ALL violations before every commit/push
Files:
react_on_rails/rakelib/shakapacker_examples.rake
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
ALWAYS ensure files end with a newline character before committing/pushing
Files:
react_on_rails/rakelib/shakapacker_examples.rake
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.778Z
Learning: Applies to /CHANGELOG.md : Update `/CHANGELOG.md` for user-visible changes (features, bug fixes, breaking changes, deprecations, performance improvements) to the open-source React on Rails gem and npm package
Learnt from: alexeyr-ci
Repo: shakacode/react_on_rails PR: 1687
File: spec/dummy/package.json:0-0
Timestamp: 2025-01-23T18:20:45.824Z
Learning: When adding or updating dependencies in spec/dummy/package.json, maintain version consistency with other package.json files in the codebase to avoid potential version conflicts.
📚 Learning: 2025-11-25T08:05:17.778Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.778Z
Learning: Applies to /CHANGELOG.md : Update `/CHANGELOG.md` for user-visible changes (features, bug fixes, breaking changes, deprecations, performance improvements) to the open-source React on Rails gem and npm package
Applied to files:
react_on_rails/rakelib/shakapacker_examples.rake
📚 Learning: 2025-11-25T08:05:17.778Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.778Z
Learning: Applies to lib/react_on_rails/*.rb : Create RBS signature files for new Ruby files in `lib/react_on_rails/` by adding corresponding `.rbs` file in `sig/react_on_rails/`, adding to Steepfile, and validating before committing
Applied to files:
react_on_rails/rakelib/shakapacker_examples.rake
📚 Learning: 2025-01-23T18:20:45.824Z
Learnt from: alexeyr-ci
Repo: shakacode/react_on_rails PR: 1687
File: spec/dummy/package.json:0-0
Timestamp: 2025-01-23T18:20:45.824Z
Learning: When adding or updating dependencies in spec/dummy/package.json, maintain version consistency with other package.json files in the codebase to avoid potential version conflicts.
Applied to files:
react_on_rails/rakelib/shakapacker_examples.rake
📚 Learning: 2025-04-26T21:55:55.874Z
Learnt from: alexeyr-ci2
Repo: shakacode/react_on_rails PR: 1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
Applied to files:
react_on_rails/rakelib/shakapacker_examples.rake
📚 Learning: 2025-09-16T08:01:11.146Z
Learnt from: justin808
Repo: shakacode/react_on_rails PR: 1770
File: lib/generators/react_on_rails/templates/base/base/app/javascript/src/HelloWorld/ror_components/HelloWorld.client.jsx:2-2
Timestamp: 2025-09-16T08:01:11.146Z
Learning: React on Rails uses webpack CSS Modules configuration with namedExports: true, which requires the import syntax `import * as style from './file.module.css'` rather than the default export pattern. This configuration enables better tree shaking and bundle size optimization for CSS modules.
Applied to files:
react_on_rails/rakelib/shakapacker_examples.rake
📚 Learning: 2025-11-25T08:05:17.778Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.778Z
Learning: Applies to **/*.{rb,rake} : ALWAYS run `bundle exec rubocop` and fix ALL violations before every commit/push
Applied to files:
react_on_rails/rakelib/shakapacker_examples.rake
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: examples (3.4, latest)
- GitHub Check: examples (3.2, minimum)
- GitHub Check: build
- GitHub Check: build-dummy-app-webpack-test-bundles (3.2, 20, minimum)
🔇 Additional comments (2)
react_on_rails/rakelib/shakapacker_examples.rake (2)
11-11: LGTM: JSON library required for new functionality.The addition of
require "json"is necessary to support JSON parsing and generation in the newapply_minimum_versionsmethod.
107-112: LGTM: Clean integration of minimum version pinning.The conditional application of
apply_minimum_versionsis correctly placed after generator commands and before npm install. Re-runningbundle_install_inafter Gemfile modification ensures dependencies are properly updated. The comment clearly explains the rationale.
CI Status Update✅ Fixed
🔧 In Progress: Generator tests (minimum version)The minimum version generator tests are encountering a Bundler isolation issue where the generated example app is loading Shakapacker from the parent This is happening because:
Potential solutions:
The Integration Tests pass which validates the core React 19 compatibility changes. The Generator test issue is a CI environment isolation problem, not a code issue. |
Use BUNDLE_GEMFILE to explicitly point to the generated app's Gemfile when running rake tasks. This prevents bundler from using gems from a parent workspace's vendor/bundle, which could have different versions. This fixes the Shakapacker version mismatch error in CI where the generated app had shakapacker 8.2.0 installed but Rails was loading shakapacker 9.4.0 from the parent react_on_rails vendor/bundle. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
react_on_rails/rakelib/shakapacker_examples.rake (1)
40-40: Consider extracting webpack-assets-manifest version as a constant.The hardcoded
"^5.0.6"appears twice and is coupled toMINIMUM_SHAKAPACKER_VERSION. Extracting it asMINIMUM_WEBPACK_ASSETS_MANIFEST_VERSIONinExampleTypewould improve maintainability and consistency with the other minimum version constants.Example:
# In example_type.rb MINIMUM_WEBPACK_ASSETS_MANIFEST_VERSION = "^5.0.6" # In this file, lines 40 and 44: deps["webpack-assets-manifest"] = ExampleType::MINIMUM_WEBPACK_ASSETS_MANIFEST_VERSION if deps.key?("webpack-assets-manifest")Also applies to: 44-44
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
react_on_rails/rakelib/shakapacker_examples.rake(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{rb,rake}
📄 CodeRabbit inference engine (CLAUDE.md)
ALWAYS run
bundle exec rubocopand fix ALL violations before every commit/push
Files:
react_on_rails/rakelib/shakapacker_examples.rake
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
ALWAYS ensure files end with a newline character before committing/pushing
Files:
react_on_rails/rakelib/shakapacker_examples.rake
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.778Z
Learning: Applies to /CHANGELOG.md : Update `/CHANGELOG.md` for user-visible changes (features, bug fixes, breaking changes, deprecations, performance improvements) to the open-source React on Rails gem and npm package
Learnt from: alexeyr-ci
Repo: shakacode/react_on_rails PR: 1687
File: spec/dummy/package.json:0-0
Timestamp: 2025-01-23T18:20:45.824Z
Learning: When adding or updating dependencies in spec/dummy/package.json, maintain version consistency with other package.json files in the codebase to avoid potential version conflicts.
Learnt from: alexeyr-ci2
Repo: shakacode/react_on_rails PR: 1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
📚 Learning: 2025-11-25T08:05:17.778Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.778Z
Learning: Applies to /CHANGELOG.md : Update `/CHANGELOG.md` for user-visible changes (features, bug fixes, breaking changes, deprecations, performance improvements) to the open-source React on Rails gem and npm package
Applied to files:
react_on_rails/rakelib/shakapacker_examples.rake
📚 Learning: 2025-11-25T08:05:17.778Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.778Z
Learning: Applies to lib/react_on_rails/*.rb : Create RBS signature files for new Ruby files in `lib/react_on_rails/` by adding corresponding `.rbs` file in `sig/react_on_rails/`, adding to Steepfile, and validating before committing
Applied to files:
react_on_rails/rakelib/shakapacker_examples.rake
📚 Learning: 2025-01-23T18:20:45.824Z
Learnt from: alexeyr-ci
Repo: shakacode/react_on_rails PR: 1687
File: spec/dummy/package.json:0-0
Timestamp: 2025-01-23T18:20:45.824Z
Learning: When adding or updating dependencies in spec/dummy/package.json, maintain version consistency with other package.json files in the codebase to avoid potential version conflicts.
Applied to files:
react_on_rails/rakelib/shakapacker_examples.rake
📚 Learning: 2025-09-16T08:01:11.146Z
Learnt from: justin808
Repo: shakacode/react_on_rails PR: 1770
File: lib/generators/react_on_rails/templates/base/base/app/javascript/src/HelloWorld/ror_components/HelloWorld.client.jsx:2-2
Timestamp: 2025-09-16T08:01:11.146Z
Learning: React on Rails uses webpack CSS Modules configuration with namedExports: true, which requires the import syntax `import * as style from './file.module.css'` rather than the default export pattern. This configuration enables better tree shaking and bundle size optimization for CSS modules.
Applied to files:
react_on_rails/rakelib/shakapacker_examples.rake
📚 Learning: 2025-04-26T21:55:55.874Z
Learnt from: alexeyr-ci2
Repo: shakacode/react_on_rails PR: 1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
Applied to files:
react_on_rails/rakelib/shakapacker_examples.rake
📚 Learning: 2025-07-08T05:57:29.630Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1745
File: node_package/src/RSCRequestTracker.ts:8-14
Timestamp: 2025-07-08T05:57:29.630Z
Learning: The global `generateRSCPayload` function in React on Rails Pro (RORP) is provided by the framework during rendering requests, not implemented in application code. The `declare global` statements are used to document the expected interface that RORP will inject at runtime.
Applied to files:
react_on_rails/rakelib/shakapacker_examples.rake
📚 Learning: 2025-11-25T08:05:17.778Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.778Z
Learning: Applies to **/*.{rb,rake} : ALWAYS run `bundle exec rubocop` and fix ALL violations before every commit/push
Applied to files:
react_on_rails/rakelib/shakapacker_examples.rake
📚 Learning: 2025-11-25T08:05:17.778Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.778Z
Learning: Applies to spec/dummy/e2e/playwright/app_commands/**/*.rb : Add custom Playwright app commands in `e2e/playwright/app_commands/` directory as Ruby files using CypressOnRails SmartFactoryWrapper pattern
Applied to files:
react_on_rails/rakelib/shakapacker_examples.rake
📚 Learning: 2025-11-25T08:05:17.778Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T08:05:17.778Z
Learning: The `react_on_rails_pro/` directory has its own Prettier/ESLint configuration and must be linted separately
Applied to files:
react_on_rails/rakelib/shakapacker_examples.rake
📚 Learning: 2025-02-12T16:38:06.537Z
Learnt from: Romex91
Repo: shakacode/react_on_rails PR: 1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.
Applied to files:
react_on_rails/rakelib/shakapacker_examples.rake
🔇 Additional comments (4)
react_on_rails/rakelib/shakapacker_examples.rake (4)
11-11: LGTM – JSON library addition.The JSON library require is necessary for the package.json manipulation in
apply_minimum_versions.
19-73: Well-implemented minimum version updates with proper error handling.The method correctly:
- Handles JSON parse errors by logging context and re-raising to fail fast in CI
- Updates React, Shakapacker, and webpack-assets-manifest versions in package.json
- Pins Shakapacker in Gemfile using regex replacement (appropriate for generated apps with standard gem declarations)
- Adds trailing newline when writing package.json
107-112: LGTM – Correct integration of minimum version updates.The logic correctly applies minimum versions when the flag is set and re-runs
bundle_install_inafter modifying the Gemfile. The placement after generator commands and before npm install is appropriate.
116-120: Excellent fix for the Bundler isolation issue.Setting
BUNDLE_GEMFILEensures the rake task uses the generated app's Gemfile and bundle, preventing version mismatches where the parent workspace's gems (e.g., Shakapacker 9.4.0) could be loaded instead of the generated app's pinned versions (e.g., Shakapacker 8.2.0). This directly resolves the CI failure described in the PR objectives.
The BUNDLE_GEMFILE env var was causing issues for latest version examples because bundler was detecting Gemfile/lockfile mismatches in frozen mode. Changes: - Only set BUNDLE_GEMFILE and BUNDLE_FROZEN=false for minimum version examples - Latest examples continue using standard bundle exec This ensures: - Latest examples work normally without bundler isolation issues - Minimum examples are properly isolated from parent workspace gems 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
PR Split NoticeThis PR has been split into two smaller, focused PRs: PR 1: React 19 Compatibility ✅ (Ready for merge)PR #2127 - Updates spec/dummy for React 19 compatibility
PR 2: Minimum Version Testing Infrastructure (This PR)This PR will be rebased/updated to contain only the minimum version testing infrastructure changes once PR #2127 is merged. Current blockers:
Recommendation: Merge PR #2127 first, then continue work on this PR for the minimum version testing. |
Summary
This PR moves React and Shakapacker version compatibility testing from spec/dummy to the generator smoke tests, as suggested in PR #2114 review.
Key changes:
basic-minimum,basic-server-rendering-minimum) that test React 18.0.0 and Shakapacker 8.2.0script/convertto only handle Node.js tooling compatibility (removed React/Shakapacker version modifications)run_rspec:shakapacker_examples_latest(basic, redux, etc.)run_rspec:shakapacker_examples_minimum(basic-minimum, etc.)Benefits
Test plan
run_rspec:shakapacker_examples_latestincludes only non-minimum examplesrun_rspec:shakapacker_examples_minimumincludes only minimum examplesCloses #2123
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.