-
Notifications
You must be signed in to change notification settings - Fork 380
Update react_on_rails to v16.1.1 #654
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🚀 Quick Review App CommandsWelcome! Here are the commands you can use in this PR:
|
Warning Rate limit exceeded@justin808 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 30 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 ignored due to path filters (2)
📒 Files selected for processing (6)
WalkthroughUpgrade React on Rails to 16.1.1 across Ruby and JS dependencies, adjust bin/dev to require dev tools unconditionally, expand Shakapacker configuration (paths, compiler strategies, integrity, test/prod tweaks), add ignore patterns for generated packs, and add clarifying comments to Procfile. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer
participant Bin as bin/dev
participant Bun as Bundler
participant ROR as ReactOnRails Dev
participant Proc as Procfile services
Dev->>Bin: Run bin/dev
Bin->>Bun: require "bundler/setup"
Bun-->>Bin: Load bundler
Bin->>ROR: require "react_on_rails/dev"
ROR-->>Bin: Setup dev environment
Bin->>Proc: Launch foreman/overmind (Procfile)
Proc-->>Dev: App starts with defined services
alt Missing dependency (strict)
Bin-->>Dev: LoadError raised (no fallback)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
/deploy-review-app |
🚀 Enhanced with Development ToolsThis PR has been enhanced with development improvements brought over from ✨ New Development Features
📂 New Files Added
See the enhanced branch: All original React on Rails 16 upgrade functionality is preserved, plus these development workflow improvements. |
Superseded by PR #655 which includes the same React on Rails 16 upgrade plus enhanced development tools. |
545179a
to
1bb4404
Compare
Code Review for PR #654: Update react_on_rails to v16.1.1I've completed a comprehensive review of this PR. Overall, this is a well-executed update with important improvements. Here's my feedback: ✅ Strengths
🔍 Areas for Attention
|
Code Review for PR #654: Update react_on_rails to v16.1.1Thank you for this comprehensive update! I've reviewed the changes and here's my feedback: ✅ Strengths
🔍 Observations & Suggestions
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
Gemfile.lock
is excluded by!**/*.lock
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (6)
.gitignore
(1 hunks)Gemfile
(1 hunks)Procfile.dev-prod-assets
(1 hunks)bin/dev
(2 hunks)config/shakapacker.yml
(2 hunks)package.json
(1 hunks)
⏰ 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). (2)
- GitHub Check: deploy
- GitHub Check: claude-review
🔇 Additional comments (5)
Gemfile (1)
8-8
: Upgrading to 16.1.1 cleanly picks up the manifest fixMatching the gem to the published 16.1.1 release delivers the bundle path bug fix noted in that patch—looks good.
Procfile.dev-prod-assets (1)
1-2
: Nice touch documenting the prod-assets modeThese comments make it much clearer when to reach for this Procfile variant. Thanks for the clarity.
package.json (1)
81-81
: JS dependency stays in sync with the gemKeeping the npm package at 16.1.1 alongside the Ruby gem avoids mismatched generator behavior—looks solid.
.gitignore (1)
56-58
: Good call ignoring generated packsAdding
**/generated/**
andssr-generated
lines up with the React on Rails guidance for auto-loaded component packs, so we won’t accidentally commit generated artifacts. As per React on Rails docs.bin/dev (1)
21-22
: Early failure path for missing deps is welcomeRequiring Bundler setup and the gem’s dev tooling up front keeps the script honest—missing installations will surface immediately instead of falling back silently. 👍
✅ Deployment complete for PR #654, commit 51f6272 |
✅ Deployment complete for PR #654, commit 51f6272 |
Code Review for PR #654: Update react_on_rails to v16.1.1Thank you for this well-structured update! I've reviewed the changes and here's my feedback: ✅ Strengths
|
Code ReviewThanks for updating React on Rails to the latest stable version! I've reviewed the changes and here's my feedback: ✅ Strengths
📝 ObservationsCode Quality & Best Practices:
Performance Considerations:
Security:
🔍 Minor Suggestions
✅ Overall AssessmentThis is a clean, focused upgrade PR that properly updates React on Rails to the latest stable version. The changes are minimal, well-tested, and pose no significant risks. The alignment of gem and npm package versions is correct, and the testing confirms everything works as expected. Approved ✅ - Ready to merge once CI passes. 🤖 Generated with Claude Code Co-Authored-By: Claude [email protected] |
Code Review for PR #654: Update react_on_rails to v16.1.1Thank you for this PR updating React on Rails to the latest stable version! I've reviewed the changes and here's my feedback: ✅ Strengths
🔍 Observations & Suggestions1. Removed error handling in bin/dev (Potential Issue)The fallback # Old version had fallback handling
-begin
- require "bundler/setup"
- require "react_on_rails/dev"
-rescue LoadError
- # Fallback for when gem is not yet installed
- puts "Loading ReactOnRails development tools..."
- require_relative "../../lib/react_on_rails/dev"
-end Concern: This could cause the script to fail if the gem isn't installed yet. Consider whether this fallback mechanism should be preserved for better developer experience during initial setup. 2. Typo fix in comment ✅Good catch fixing the typo in the Procfile comment: -# - bin/dev static: Uses Procfile.dev-static-assets-assets
+# - bin/dev static: Uses Procfile.dev-static-assets 3. Sass loader configuration (Good practice)The webpack configuration properly handles both string and object loader configurations when setting the modern API: if (typeof sassLoader === 'string') {
// Convert string to object format
} else {
// Update existing object
} This defensive coding approach prevents potential configuration errors. 🔒 Security Considerations
🧪 Test CoverageAccording to the PR description, all 38 tests pass successfully, which is reassuring. The existing test suite appears to cover:
🎯 Performance ConsiderationsThe modern Sass API usage should provide better compilation performance, especially for larger stylesheets. 📝 Minor Suggestions
✅ Overall AssessmentThis is a well-executed dependency update that brings the project to a stable version of React on Rails. The changes are focused and appropriate. My only concern is the removal of error handling in Recommendation: ✅ Approve with minor suggestions - Consider addressing the bin/dev error handling before merging. Great work on keeping the dependencies up to date! 🎉 |
✅ Deployment complete for PR #654, commit bcac070 |
✅ Deployment complete for PR #654, commit e59678a |
Code Review for PR #654: Update react_on_rails to v16.1.1I've reviewed the changes in this PR and overall this looks like a solid dependency update. Here's my detailed feedback: ✅ Strengths:
🔍 Observations & Suggestions:1. Sass Loader Configuration (config/webpack/commonWebpackConfig.js:30-55)The new sass-loader modern API configuration is a good update. The implementation correctly handles both string and object loader configurations. This should improve build performance and compatibility with newer sass versions. 2. Simplified bin/dev Script (bin/dev:21-25)The removal of the fallback require mechanism is cleaner, though it assumes the gem is always installed. This is reasonable for a production-ready application and reduces complexity. 3. Dependency Lock FilesThe large changes in yarn.lock (4429 additions, 3819 deletions) and Gemfile.lock are expected for a version update. The Rails framework was also updated (8.0.2.1 → 8.0.3) as a side effect, which is a minor patch update and should be safe.
|
- Update react_on_rails gem: 14.2.1 → 16.0.1.rc.2 - Update react-on-rails npm: 14.2.1 → 16.0.1-rc.2 - Update rescript-react-on-rails: 1.0.1 → 1.1.0 (for React on Rails 16 support) - Add test environment configuration to resolve server bundle path issue - Add .prettierignore rule for ReScript generated files - All 38 tests passing ✅ 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Upgrades from v16.0.1.rc.4 to v16.1.1 - Bug fix release with no breaking changes - Fixes react-server-client-manifest.json file path resolution - All tests passing and RuboCop checks pass 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Ran the react_on_rails:install generator to get latest v16.1.1 improvements: - Enhanced bin/dev script with better development server management - Added Procfile variations for different development modes: - Procfile.dev: HMR mode with webpack-dev-server - Procfile.dev-static-assets: Watch mode without HMR - Procfile.dev-prod-assets: Development with production assets - Updated webpack configurations for v16.1.1 compatibility - Updated shakapacker.yml configuration - Updated babel.config.js - Updated react_on_rails initializer - Added generateWebpackConfigs.js helper These updates provide better development experience and align with React on Rails v16.1.1 best practices. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
The improved bin/dev script provides better development server management through ReactOnRails::Dev::ServerManager with support for different modes (HMR, static assets, production assets)
Configure sass-loader to use the modern API to avoid deprecation warnings about the legacy JS API being removed in Dart Sass 2.0.0
Update configuration to properly handle full path loaders and maintain existing sassOptions while adding the modern API flag
e59678a
to
ba04c6c
Compare
✅ Deployment complete for PR #654, commit ba04c6c |
PR Review: Update react_on_rails to v16.1.1Thank you for updating React on Rails to the latest stable version! The changes look well-organized and tested. Here's my review: ✅ Strengths
🔍 Observations & Suggestions
|
✅ Review app for PR #654 was successfully deleted |
Summary
Details
This PR updates React on Rails to the latest stable version 16.1.1 and includes all the improvements from running the install generator.
Package Updates:
Version 16.1.1 Fix:
bundle_js_file_path
for improved configuration flexibilityGenerator Improvements:
Procfile.dev
: HMR mode with webpack-dev-serverProcfile.dev-static-assets
: Watch mode without HMRProcfile.dev-prod-assets
: Development with production assetsTest plan
🤖 Generated with Claude Code
This change is
Summary by CodeRabbit
Chores
Refactor
Documentation
No user-facing changes.