Skip version validation when package.json doesn't exist during setup

[justin808]

Summary

Fixes an error that occurs when running rake shakapacker:install on a fresh Rails application. The version validation was introduced in commit 0d87ea7 (#1881) and runs during Rails initialization via an after_initialize hook.

Problem

shakapacker:install needs to load the Rails environment before it can create package.json, but the version validation runs during environment loading and fails because package.json doesn't exist yet.

Error message:

ReactOnRails::Error: **ERROR** ReactOnRails: package.json file not found.

Expected location: /path/to/project/package.json

Solution

Skip validation if package.json doesn't exist yet. This allows installation tasks to complete successfully while still validating versions on normal Rails application startup.

Root Cause Analysis

1. Commit 0d87ea7 (9 days ago) added strict version validation that runs on Rails initialization
2. This validation checks for package.json existence and raises an error if not found
3. The shakapacker:install rake task needs to load the Rails environment to run (via config/environment.rb)
4. The validation runs during environment loading, before the task can create package.json

Testing

• Validated that bundle exec rubocop passes
• Confirmed the fix matches the implementation in branch justin808/shakapacker-9.3.0 (commit 42c109c)
• Tested that the example generation would work with this fix

Related

This fix was already implemented in the justin808/shakapacker-9.3.0 branch (commit 42c109c) but hadn't been merged to master yet.

🤖 Generated with Claude Code

---

This change is 

[coderabbitai]

Warning

Rate limit exceeded

@justin808 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 21 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 @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 fe363a9 and d568ee1.

📒 Files selected for processing (1)

• lib/react_on_rails/engine.rb (1 hunks)

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch justin808/fix-package-json-check

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[claude]

Code Review: Skip version validation when package.json doesn't exist during setup

Summary

This PR adds a guard clause to skip version validation when package.json doesn't exist yet, fixing a chicken-and-egg problem where shakapacker:install needs to load Rails but the validation runs during initialization.

---

✅ Strengths

1. Solves a Real Problem: The fix addresses a genuine initialization order issue where setup tasks fail because validation runs too early.

2. Minimal and Focused: The change is surgical - just 5 lines added with a clear guard clause pattern.

3. Well-Documented: The PR description provides excellent context including root cause analysis and error messages.

4. Good Comment: The inline comment clearly explains why the check is needed.

---

🔍 Code Quality Assessment

Correctness

The implementation is correct and follows Rails patterns:

• Uses File.exist? to check for file existence
• Uses next to exit early from the after_initialize block
• Properly retrieves the package.json path via VersionChecker::NodePackageVersion.package_json_path

Style & Conventions

✅ Follows project conventions per CLAUDE.md:

• Code structure is clean and readable
• Comment style is appropriate

---

📝 Suggestions for Improvement

1. Test Coverage ⚠️

The existing test suite has a test for "when package.json file does not exist" (line 211-234 in version_checker_spec.rb), but it tests the validation method directly, not the Rails initializer skip behavior. Consider adding an integration test that:

• Simulates the Rails initialization process
• Verifies the initializer exits early when package.json doesn't exist
• Confirms no error is raised

Example location: spec/react_on_rails/engine_spec.rb (may need to be created)

2. Logging Consideration 💡

Currently, when package.json doesn't exist:

• The validation logs "Validating package version..." (line 11)
• Then silently skips
• But doesn't log "Package validation successful" (line 13)

This creates an asymmetric log pattern. Consider either:

Option A (Recommended): Move logging inside the check

initializer "react_on_rails.validate_version_and_package_compatibility" do
  config.after_initialize do
    package_json = VersionChecker::NodePackageVersion.package_json_path
    next unless File.exist?(package_json)
    
    Rails.logger.info "[React on Rails] Validating package version and compatibility..."
    VersionChecker.build.validate_version_and_package_compatibility!
    Rails.logger.info "[React on Rails] Package validation successful"
  end
end

Option B: Add skip logging

next unless File.exist?(package_json)
Rails.logger.debug "[React on Rails] Skipping package validation (package.json not found)"

3. Variable Naming

The variable package_json actually holds a path, not the JSON contents. Consider renaming:

package_json_path = VersionChecker::NodePackageVersion.package_json_path
next unless File.exist?(package_json_path)

---

🔒 Security Considerations

✅ No security concerns. The change:

• Only reads file existence (doesn't execute or parse)
• Uses existing path resolution logic
• Doesn't introduce new attack vectors

---

⚡ Performance Considerations

✅ Excellent performance impact:

• Adds one File.exist? check (very fast syscall)
• Prevents expensive validation chain when not needed
• Actually improves performance for setup tasks

---

🐛 Potential Issues

✅ No bugs identified. The logic is sound:

• Early exit pattern is idiomatic Ruby
• Doesn't affect normal Rails application startup (validation still runs)
• Properly handles the installation scenario

---

🎯 Recommendations

Priority: Medium

1. Add integration test for the initializer behavior
2. Improve logging consistency (Option A recommended)
3. Consider variable rename for clarity

Priority: Low
4. Ensure this pattern is documented for future similar cases

---

✅ Approval Status

APPROVED with suggestions for enhancement.

The core fix is solid and solves the immediate problem correctly. The suggestions above would improve observability and test coverage but are not blockers for merging.

Verification Checklist:

• ✅ Solves the stated problem
• ✅ Minimal code changes
• ✅ Follows project conventions
• ⚠️ Test coverage could be improved
• ✅ No security or performance concerns
• ✅ Well-documented in PR description

Great work on the thorough PR description and root cause analysis! 🎉