Skip to content

Conversation

@justin808
Copy link
Member

@justin808 justin808 commented Nov 7, 2025

Summary

This PR provides a minimal upgrade of Shakapacker from version 9.2.0 to 9.3.0. This is a focused upgrade that only updates the Shakapacker gem version without any additional changes.

Changes

  • Updated Gemfile.development_dependencies to use Shakapacker 9.3.0
  • Updated Gemfile.lock with new dependency versions

Shakapacker 9.3.0 Highlights

Based on the Shakapacker changelog:

  • Default useContentHash changed to true: The default template no longer triggers production warnings about useContentHash
  • Improved TypeScript support: Fixed type definitions to export proper types instead of any for better IDE autocomplete
  • Custom Rails environments: Fixed support for non-standard environments like staging (previously crashed)
  • Better error handling: Improved error handling in configExporter module

Breaking Changes

None - this is a non-breaking upgrade from 9.2.0.

Test Plan

  • ✅ All gem-only RSpec tests pass
  • ✅ RuboCop passes with no violations
  • ✅ Pre-commit hooks pass successfully

Notes

This is intentionally a minimal PR focused solely on the version upgrade. Any additional improvements or configurations related to Shakapacker 9.3.0 features can be addressed in separate PRs.

🤖 Generated with Claude Code


This change is Reviewable

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 7, 2025

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 33 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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.

📥 Commits

Reviewing files that changed from the base of the PR and between 301a432 and 1532c7b.

⛔ Files ignored due to path filters (5)
  • Gemfile.lock is excluded by !**/*.lock
  • react_on_rails_pro/spec/dummy/yarn.lock is excluded by !**/yarn.lock, !**/*.lock
  • react_on_rails_pro/spec/execjs-compatible-dummy/yarn.lock is excluded by !**/yarn.lock, !**/*.lock
  • spec/dummy/Gemfile.lock is excluded by !**/*.lock
  • spec/dummy/yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (5)
  • Gemfile.development_dependencies (1 hunks)
  • react_on_rails_pro/Gemfile.development_dependencies (1 hunks)
  • react_on_rails_pro/spec/dummy/package.json (1 hunks)
  • react_on_rails_pro/spec/execjs-compatible-dummy/package.json (1 hunks)
  • spec/dummy/package.json (1 hunks)
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch justin808/shakapacker-9.3.0-simple

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@claude
Copy link

claude bot commented Nov 7, 2025

Code Review: Shakapacker 9.2.0 to 9.3.0 Upgrade

Overview

This PR upgrades Shakapacker from 9.2.0 to 9.3.0 in a minimal, focused approach. I've reviewed the changes against the repository's CLAUDE.md guidelines and best practices.


✅ Strengths

  1. Minimal Scope: The PR correctly focuses on a single dependency upgrade without mixing in unrelated changes. This makes it easy to review and revert if needed.

  2. Good PR Description: The summary clearly lists Shakapacker 9.3.0 highlights and confirms no breaking changes, which helps reviewers understand the impact.

  3. Test Plan: The PR mentions that gem-only RSpec tests pass and RuboCop passes, which aligns with the mandatory pre-commit requirements.


⚠️ Issues & Concerns

1. CRITICAL: Incomplete Upgrade Scope

The PR only updates 2 files (Gemfile.development_dependencies and Gemfile.lock), but there are additional files that reference shakapacker 9.2.0:

  • spec/dummy/package.json:54 - Still has "shakapacker": "9.2.0"
  • react_on_rails_pro/Gemfile.development_dependencies:10 - Still has gem "shakapacker", "9.2.0"
  • Likely spec/dummy/yarn.lock needs updating
  • Likely Pro dummy app package.json files need updating

Impact: This creates version inconsistency across the monorepo. The dummy apps and Pro package will still use 9.2.0 while the main gem expects 9.3.0.

Recommendation: Update all shakapacker references to 9.3.0 across:

  • /spec/dummy/package.json
  • /react_on_rails_pro/Gemfile.development_dependencies
  • Run yarn install in spec/dummy to update yarn.lock
  • Run bundle install in react_on_rails_pro to update its Gemfile.lock
  • Check Pro dummy apps for consistency

2. Missing CHANGELOG Entry

According to CLAUDE.md:

Update CHANGELOG.md for user-visible changes only (features, bug fixes, breaking changes, deprecations, performance improvements)

While this is a dependency upgrade, the previous Shakapacker upgrades (9.0.0, 9.1.0, 9.2.0) all have CHANGELOG entries. For consistency, this should also have an entry.

Recommendation: Add a changelog entry following the established pattern:

- **Shakapacker 9.3.0 Upgrade**: Upgraded Shakapacker from 9.2.0 to 9.3.0. This minor version update includes improved TypeScript support, better custom Rails environment handling, and sets useContentHash to true by default. [PR 1935](https://github.com/shakacode/react_on_rails/pull/1935) by [justin808](https://github.com/justin808).

3. Gemfile.lock Dependency Updates

The Gemfile.lock shows numerous transitive dependency updates (nokogiri, rails-html-sanitizer, rack, etc.). While these are expected with a bundle update, some updates are notable:

  • nokogiri: 1.16.6 → 1.18.10 (major jump, spans security fixes)
  • rack: 3.1.4 → 3.2.4 (minor version bump)
  • concurrent-ruby: 1.3.3 → 1.3.5

Question: Did you run bundle update shakapacker (targeted) or bundle update (full update)? The diff suggests a broader update occurred.

Recommendation: Clarify in the PR description whether this was a targeted or full bundle update. If unintentional, consider using bundle update shakapacker to minimize transitive changes.


🔍 Security Considerations

Positive: The nokogiri upgrade (1.16.6 → 1.18.10) likely includes security fixes. This is beneficial.

Note: No obvious security concerns introduced by this upgrade. Shakapacker 9.3.0 has no known CVEs.


🎯 Performance Considerations

According to the Shakapacker 9.3.0 changelog:

  • useContentHash default changed to true: This improves browser caching but may slightly increase build time on first compile. This is a net positive for production performance.

Recommendation: No performance concerns.


✅ Testing & Quality Checks

Based on CLAUDE.md requirements, before merging:

  • bundle exec rubocop passes with zero offenses (claimed in PR, should verify in CI)
  • All files end with newline characters (should be caught by pre-commit hooks)
  • RSpec tests pass (claimed in PR)
  • Dummy app builds successfully with new Shakapacker version
  • Pro package tests pass (not mentioned in test plan)

Question: Did you test the dummy app and Pro package builds with the new version, or only the gem-level tests?


📋 Action Items Before Merge

  1. Update missing files to Shakapacker 9.3.0:

    • spec/dummy/package.json
    • react_on_rails_pro/Gemfile.development_dependencies
    • Run yarn/bundle installs to update lockfiles
  2. Add CHANGELOG entry following the established pattern

  3. Clarify bundle update scope - was this targeted or full?

  4. Verify Pro package tests pass with the upgrade

  5. Ensure CI passes for all checks (not just gem-level tests)


💡 Suggestions

Nice to have: Consider adding a note in the PR description about any manual testing done (e.g., "Verified dummy app webpack build succeeds" or "Confirmed RSC examples still work").


🎯 Verdict

Status: ⚠️ Needs Changes

The upgrade approach is sound, but the incomplete scope (missing dummy apps and Pro package) needs to be addressed before merging. The PR is on the right track but needs to be comprehensive across the monorepo.

Once the action items are addressed, this should be safe to merge. The Shakapacker 9.3.0 upgrade appears to be low-risk with beneficial improvements.


Note: Review conducted following CLAUDE.md guidelines and React on Rails monorepo conventions.

This is a minimal upgrade that updates only the Shakapacker version across
all locations in the repository.

Files updated:
- Gemfile.development_dependencies (main and Pro)
- package.json files (spec/dummy, Pro dummy, Pro execjs-compatible-dummy)
- yarn.lock files (all dummy apps)
- Gemfile.lock files (main and spec/dummy)

Key changes in Shakapacker 9.3.0:
- Default useContentHash changed to true in shakapacker.yml template
- Improved TypeScript type definitions for better IDE support
- Fixed support for custom Rails environments (e.g., staging)
- Better error handling in configExporter module

No breaking changes or configuration changes required for this upgrade.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@justin808 justin808 force-pushed the justin808/shakapacker-9.3.0-simple branch from b1463f3 to 1532c7b Compare November 7, 2025 03:03
@justin808 justin808 merged commit 57fb6bf into master Nov 7, 2025
32 of 35 checks passed
@justin808 justin808 deleted the justin808/shakapacker-9.3.0-simple branch November 7, 2025 04:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants