-
-
Notifications
You must be signed in to change notification settings - Fork 638
Upgrade Shakapacker from 9.1.0 to 9.2.0 #1931
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
|
Warning Rate limit exceeded@justin808 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 26 minutes and 54 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 (5)
📒 Files selected for processing (9)
WalkthroughShakapacker dependency updated from 9.1.0 to 9.2.0 across Gemfile and package.json files. CHANGELOG.md documents the upgrade and new bin/shakapacker-config utility. Three identical bin/export-bundler-config scripts added as thin wrappers forwarding to shakapacker/configExporter module. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 - Shakapacker 9.2.0 UpgradeSummaryThis PR upgrades Shakapacker from 9.1.0 to 9.2.0 across all relevant files. The changes are well-documented and appear to be a straightforward dependency update. ✅ Strengths
|
| Category | Rating | Notes |
|---|---|---|
| Correctness | ✅ Good | All dependency references updated consistently |
| Best Practices | CHANGELOG formatting needs fix | |
| Security | ✅ Good | Minor version bump, no security concerns |
| Performance | ✅ N/A | Dependency update only |
| Test Coverage | Basic testing confirmed, full suite recommended |
📋 Action Items
- Fix CHANGELOG.md to include
[PR 1931](...)link - Run full test suite to verify compatibility
- Consider documenting the dummy app version discrepancy if intentional
🎯 Verdict
Approve with changes - This is a solid dependency upgrade PR. Once the CHANGELOG formatting is fixed, this should be ready to merge. The upgrade adds useful debugging utilities without breaking changes.
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: 0
🧹 Nitpick comments (2)
CHANGELOG.md (1)
46-47: Add PR reference for traceability.The changelog entry should include a PR reference for consistency with other entries and to maintain traceability.
Apply this diff:
-- **Shakapacker 9.2.0 Upgrade**: Upgraded Shakapacker from 9.1.0 to 9.2.0. This minor version update adds a new `bin/shakapacker-config` utility for debugging webpack/rspack configurations with doctor mode, save mode, and stdout mode options. Supports YAML, JSON, and Node.js inspect output formats. by [justin808](https://github.com/justin808). +- **Shakapacker 9.2.0 Upgrade**: Upgraded Shakapacker from 9.1.0 to 9.2.0. This minor version update adds a new `bin/shakapacker-config` utility for debugging webpack/rspack configurations with doctor mode, save mode, and stdout mode options. Supports YAML, JSON, and Node.js inspect output formats. [PR 1931](https://github.com/shakacode/react_on_rails/pull/1931) by [justin808](https://github.com/justin808).react_on_rails_pro/spec/execjs-compatible-dummy/bin/shakapacker-config (1)
6-11: Clean implementation with minor optional improvement.The promise handling and exit code propagation are correct. The use of
process.argv.slice(2)properly excludes the node executable and script path.Consider making the error logging more robust to handle edge cases where
error.messagemight be undefined:run(process.argv.slice(2)) .then((exitCode) => process.exit(exitCode)) .catch((error) => { - console.error(error.message) + console.error(error?.message || error) process.exit(1) })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
Gemfile.lockis excluded by!**/*.lockreact_on_rails_pro/spec/dummy/yarn.lockis excluded by!**/yarn.lock,!**/*.lockreact_on_rails_pro/spec/execjs-compatible-dummy/yarn.lockis excluded by!**/yarn.lock,!**/*.lockspec/dummy/Gemfile.lockis excluded by!**/*.lockspec/dummy/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (9)
CHANGELOG.md(1 hunks)Gemfile.development_dependencies(1 hunks)react_on_rails_pro/Gemfile.development_dependencies(1 hunks)react_on_rails_pro/spec/dummy/bin/shakapacker-config(1 hunks)react_on_rails_pro/spec/dummy/package.json(1 hunks)react_on_rails_pro/spec/execjs-compatible-dummy/bin/shakapacker-config(1 hunks)react_on_rails_pro/spec/execjs-compatible-dummy/package.json(1 hunks)spec/dummy/bin/shakapacker-config(1 hunks)spec/dummy/package.json(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 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-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_pro/spec/dummy/package.jsonreact_on_rails_pro/spec/dummy/bin/shakapacker-configreact_on_rails_pro/spec/execjs-compatible-dummy/package.jsonspec/dummy/package.jsonspec/dummy/bin/shakapacker-configreact_on_rails_pro/spec/execjs-compatible-dummy/bin/shakapacker-config
📚 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_pro/spec/dummy/package.jsonreact_on_rails_pro/spec/dummy/bin/shakapacker-configreact_on_rails_pro/spec/execjs-compatible-dummy/package.jsonreact_on_rails_pro/Gemfile.development_dependencies
📚 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_pro/spec/dummy/package.jsonreact_on_rails_pro/spec/dummy/bin/shakapacker-configreact_on_rails_pro/spec/execjs-compatible-dummy/package.jsonreact_on_rails_pro/Gemfile.development_dependenciesreact_on_rails_pro/spec/execjs-compatible-dummy/bin/shakapacker-config
📚 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_pro/spec/dummy/bin/shakapacker-configreact_on_rails_pro/spec/execjs-compatible-dummy/package.jsonreact_on_rails_pro/Gemfile.development_dependenciesCHANGELOG.md
⏰ 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). (8)
- GitHub Check: lint-js-and-ruby
- GitHub Check: build
- GitHub Check: rspec-package-tests (3.2, minimum)
- GitHub Check: rspec-package-tests (3.4, latest)
- GitHub Check: build-dummy-app-webpack-test-bundles
- GitHub Check: build-dummy-app-webpack-test-bundles
- GitHub Check: markdown-link-check
- GitHub Check: claude-review
🔇 Additional comments (10)
spec/dummy/package.json (1)
54-54: LGTM! Version update is consistent across the codebase.The shakapacker version bump to 9.2.0 is consistently applied across all package.json files in this PR, which aligns with the learning about maintaining version consistency to avoid conflicts.
react_on_rails_pro/spec/execjs-compatible-dummy/package.json (1)
33-33: LGTM! Consistent version upgrade.The shakapacker dependency update to 9.2.0 is properly applied and matches the versions used throughout the codebase.
Gemfile.development_dependencies (1)
3-3: LGTM! Gem version properly synchronized.The shakapacker gem version is correctly updated to 9.2.0, maintaining version synchronization between the Ruby gem and npm package as expected for this project.
spec/dummy/bin/shakapacker-config (1)
1-11: LGTM! Clean CLI wrapper implementation.The script is a well-structured minimal shim that properly:
- Delegates to the shakapacker/configExporter module
- Handles CLI arguments correctly (excluding node and script path)
- Manages async execution with appropriate exit codes
- Provides user-friendly error messages
The implementation is consistent across all instances of this script in the codebase.
react_on_rails_pro/Gemfile.development_dependencies (1)
10-10: LGTM! Version properly synchronized.The shakapacker gem is correctly updated to 9.2.0 and remains synchronized with the root Gemfile.development_dependencies as indicated by the comment on Line 1.
react_on_rails_pro/spec/dummy/package.json (1)
66-66: LGTM! Consistent dependency update.The shakapacker version is correctly updated to 9.2.0, maintaining consistency across all package.json files in the codebase.
react_on_rails_pro/spec/dummy/bin/shakapacker-config (1)
1-11: LGTM! Consistent with other CLI wrappers.This script correctly implements the same clean wrapper pattern as the other shakapacker-config scripts, maintaining consistency across the codebase.
react_on_rails_pro/spec/execjs-compatible-dummy/bin/shakapacker-config (3)
1-1: LGTM: Standard Node.js shebang.The shebang is correct and allows the script to be executed as a standalone executable.
4-4: LGTM: Clean module import.The require statement correctly imports the
runfunction from the new Shakapacker 9.2.0 configExporter module.
1-11: All three shakapacker-config files are identical.Verification confirms that
react_on_rails_pro/spec/execjs-compatible-dummy/bin/shakapacker-config,react_on_rails_pro/spec/dummy/bin/shakapacker-config, andspec/dummy/bin/shakapacker-configshare the same md5sum hash (4ee691b75fc24189bbabad2d1a9a7591), ensuring consistency and preventing maintenance issues across all dummy app locations.
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: 0
🧹 Nitpick comments (2)
react_on_rails_pro/spec/execjs-compatible-dummy/bin/export-bundler-config (1)
1-11: LGTM! Consider consolidation and error logging enhancement.The thin wrapper correctly delegates to
shakapacker/configExporterwith proper promise-based exit handling. However, this exact logic is duplicated inspec/dummy/bin/export-bundler-configandreact_on_rails_pro/spec/dummy/bin/export-bundler-config, creating maintenance overhead.Additionally, logging only
error.message(line 9) may make debugging harder. Consider logging the full error or stack trace.If consolidation is desired, consider extracting the common logic to a shared module or script. For improved debugging, apply this optional enhancement:
.catch((error) => { - console.error(error.message) + console.error(error.stack || error.message) process.exit(1) })spec/dummy/bin/export-bundler-config (1)
4-11: Inconsistent semicolon usage across duplicate files.This file uses semicolons (lines 4, 10, 11) while the other two
bin/export-bundler-configfiles (react_on_rails_pro/spec/execjs-compatible-dummy/bin/export-bundler-configandreact_on_rails_pro/spec/dummy/bin/export-bundler-config) do not. For consistency across these duplicated files, align the style.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
react_on_rails_pro/spec/dummy/bin/export-bundler-config(1 hunks)react_on_rails_pro/spec/execjs-compatible-dummy/bin/export-bundler-config(1 hunks)spec/dummy/bin/export-bundler-config(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 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-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_pro/spec/dummy/bin/export-bundler-configreact_on_rails_pro/spec/execjs-compatible-dummy/bin/export-bundler-config
📚 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_pro/spec/dummy/bin/export-bundler-configreact_on_rails_pro/spec/execjs-compatible-dummy/bin/export-bundler-config
📚 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_pro/spec/dummy/bin/export-bundler-configreact_on_rails_pro/spec/execjs-compatible-dummy/bin/export-bundler-config
📚 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_pro/spec/dummy/bin/export-bundler-configspec/dummy/bin/export-bundler-config
🔇 Additional comments (1)
react_on_rails_pro/spec/dummy/bin/export-bundler-config (1)
1-11: LGTM! See comments on other duplicate files.This wrapper is functionally correct. Refer to comments on
react_on_rails_pro/spec/execjs-compatible-dummy/bin/export-bundler-configregarding code duplication and error logging enhancement.
Code Review: Shakapacker 9.1.0 → 9.2.0 UpgradeOverall Assessment✅ LGTM - This is a well-executed dependency upgrade with appropriate additions. The PR follows project conventions and properly documents the changes. Strengths
Code Quality ObservationsBinstub ImplementationThe
Note: There's a minor inconsistency in the binstubs:
Both Pro dummy apps are consistent with each other (no semicolons), but differ from the main dummy app. This suggests possible formatter configuration differences or the files were formatted at different times. Recommendation: Run Changelog Entry ReviewCurrent entry: - **Shakapacker 9.2.0 Upgrade**: Upgraded Shakapacker from 9.1.0 to 9.2.0. This minor version update adds a new `bin/shakapacker-config` utility...Issue: The changelog mentions Fix needed: Update the changelog to reference the correct binstub name: - **Shakapacker 9.2.0 Upgrade**: Upgraded Shakapacker from 9.1.0 to 9.2.0. This minor version update adds a new `bin/export-bundler-config` utility for debugging webpack/rspack configurations with doctor mode, save mode, and stdout mode options. Supports YAML, JSON, and Node.js inspect output formats. by [justin808](https://github.com/justin808).Also missing the PR number - should follow the format from PR 1921 above it: [PR 1931](https://github.com/shakacode/react_on_rails/pull/1931) by [justin808](https://github.com/justin808).Testing RecommendationsBased on CLAUDE.md, verify:
Security & Performance
Action ItemsBefore merging:
Minor but recommended: VerdictThis is solid work with just two minor issues to address. The upgrade is straightforward, properly tested, and well-documented. Once the changelog is corrected and formatting is finalized, this is ready to merge. Great attention to detail in adding the new utility binstub to all dummy apps! 🎯 |
Code Review: Shakapacker 9.2.0 UpgradeThank you for this dependency upgrade PR! I've reviewed the changes and here's my feedback: ✅ Strengths
|
This upgrade adds a new `bin/shakapacker-config` utility for debugging webpack/rspack configurations with the following features: - Doctor mode (--doctor): Exports all configs for comprehensive troubleshooting - Save mode (--save): Export current environment configurations to files - Stdout mode (default): View configurations in terminal - Supports YAML, JSON, and Node.js inspect output formats Changes: - Update Shakapacker version to 9.2.0 in all Gemfile.development_dependencies - Update Shakapacker version to 9.2.0 in all package.json files - Update Gemfile.lock and yarn.lock files - Add CHANGELOG.md entry documenting the upgrade 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Update yarn.lock files in Pro dummy apps to reflect Shakapacker 9.2.0 upgrade. This fixes CI failures caused by frozen lockfile validation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Add the new bin/shakapacker-config utility introduced in Shakapacker 9.2.0 to all dummy apps. This tool exports webpack/rspack configurations in human-readable formats for debugging and comparison. The binstub provides: - --doctor mode for comprehensive troubleshooting - --save mode to export configurations - Multiple output formats (YAML, JSON, inspect) - Support for both webpack and rspack Added to: - spec/dummy/bin/shakapacker-config - react_on_rails_pro/spec/dummy/bin/shakapacker-config - react_on_rails_pro/spec/execjs-compatible-dummy/bin/shakapacker-config 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Format the new shakapacker-config binstubs to match Prettier code style: - Change double quotes to single quotes - Add missing semicolons This fixes the CI formatting check failure. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The canonical binstub name in Shakapacker 9.2.0 is 'export-bundler-config', not 'shakapacker-config'. This was discovered by examining the gem's lib/install/bin/ directory. Changes: - Renamed bin/shakapacker-config → bin/export-bundler-config in all dummy apps - Applied project's Prettier formatting (semi: true, singleQuote: true) - Verified binstub works with --help flag The binstub provides the new config export utility for debugging webpack/rspack configurations. Previous commits incorrectly: 1. Used wrong binstub name (shakapacker-config vs export-bundler-config) 2. Initially had wrong formatting (needed to match project's Prettier config) This commit uses the correct name from the Shakapacker gem installation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
bdf172a to
a4a85b6
Compare
Pull Request Review - Shakapacker 9.1.0 → 9.2.0 UpgradeOverall Assessment✅ Approved - This is a clean, well-executed dependency upgrade that follows best practices. Code Quality & Best Practices✅ Strengths
Issues Found
|
PR Review: Shakapacker 9.2.0 UpgradeOverall AssessmentThis PR looks clean and well-executed. It's a straightforward dependency upgrade that follows the project's conventions properly. ✅ Strengths1. Code Quality & Consistency
2. Documentation
3. Best Practices
🔍 Minor Issues Found1. CHANGELOG Formatting Inconsistency
Location: Current: - **Shakapacker 9.2.0 Upgrade**: ... by [justin808](https://github.com/justin808).Should be: - **Shakapacker 9.2.0 Upgrade**: ... [PR 1931](https://github.com/shakacode/react_on_rails/pull/1931) by [justin808](https://github.com/justin808).2. Semicolon Inconsistency in
While this won't cause functional issues, consistency is better. However, since these are generated files from Shakapacker, this might be intentional or fixed automatically by Prettier. 🔒 Security Considerations
🧪 Testing & Performance
📝 Recommendations
VerdictAPPROVED with minor fix needed ✅ This is a solid upgrade PR. Just need to add the PR reference to the CHANGELOG entry per project conventions, then it's ready to merge. Great work documenting the new |
Summary
This PR upgrades Shakapacker from version 9.1.0 to 9.2.0.
Key Changes
New Features in Shakapacker 9.2.0
The main addition in this version is the new
bin/shakapacker-configutility for debugging webpack/rspack configurations:--doctor): Exports all configs (dev + prod, client + server) toshakapacker-config-exports/directory for comprehensive troubleshooting--save): Export current environment configurations to filesshakapacker.yml.gitignoresuggestionsThis utility is useful for comparing webpack vs rspack configurations and provides groundwork for future config comparison features.
Testing
🤖 Generated with Claude Code
This change is
Summary by CodeRabbit
Release Notes
New Features
Chores