Fix invalid precompile_hook in Pro dummy

[alexeyr-ci2]

Summary

The Pro dummy app was looking for a non-existent file in its precompile_hook.

Pull Request checklist

• ~[ ] Add/update test to cover these changes~
• ~[ ] Update documentation~
• ~[ ] Update CHANGELOG file~

Summary by CodeRabbit

• Chores
  • Improved build precompile hook configuration to resolve dependencies from the repository root, enhancing build consistency across environments.
  • Strengthened error handling with stricter exit codes when required build dependencies are missing.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai[bot]]

Walkthrough

Updated the shakapacker precompile hook in the dummy application to load the shared hook from the repository root instead of the gem root, enforced strict error handling with exit status 1 for missing dependencies, and added explicit hook loading at script completion.

Changes

Cohort / File(s)	Summary
Shakapacker Precompile Hook Path and Error Handling react_on_rails_pro/spec/dummy/bin/shakapacker-precompile-hook	Changed shared hook path from gem_root/spec/support/shakapacker_precompile_hook_shared.rb to repo_root/react_on_rails/spec/support/shakapacker_precompile_hook_shared.rb; upgraded error handling to exit with status 1 (from warning only) if shared hook is missing; added load shared_hook statement at script end

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

• Single file with focused, straightforward modifications (path adjustment, error handling tightening, load statement addition)
• No complex logic or control flow changes to evaluate

Possibly related issues

• Issue #2195: Addresses the root cause where using load prevents the shared file's __FILE__ == $PROGRAM_NAME block from executing as expected

Possibly related PRs

• PR #1977: Both modify the same shakapacker-precompile-hook script's logic for path resolution and loading shared hook with error handling adjustments
• PR #2053: Both modify the hook to reference and load shared precompile hook configuration with repository root path resolution and strict error enforcement
• PR #1916: Both affect shakapacker precompile hook modifications (path/behavior adjustments in relation to generator templates)

Suggested labels

bug, review-needed

Suggested reviewers

• AbanoubGhadban

Poem

🐰 A hook that once searched in shadows and gem, Now knows where its shared friend truly lives—in stem! With exit codes strict and paths rerouted home, The precompiler won't stumble or roam. 📦✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Fix invalid precompile_hook in Pro dummy' directly and clearly describes the main change: fixing an invalid precompile hook configuration in the Pro dummy app.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

🧪 Generate unit tests (beta)

• [ ] Create PR with unit tests
• [ ] Post copyable unit tests in a comment
• [ ] Commit unit tests in branch alexeyr/fix-pro-precompile-hook

---

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[bot]]

Code Review

✅ Overall Assessment

This is a good, targeted fix that resolves a path resolution bug in the Pro dummy app's precompile hook. The change is minimal, correct, and follows the project's conventions.

---

🎯 What This Fixes

Problem: The Pro dummy app's shakapacker-precompile-hook was looking for the shared hook implementation at an incorrect path:

• Old path: spec/support/shakapacker_precompile_hook_shared.rb (relative to repo root)
• Actual location: react_on_rails/spec/support/shakapacker_precompile_hook_shared.rb

Root cause: The monorepo has two top-level directories (react_on_rails/ and react_on_rails_pro/), and the Pro dummy hook needs to navigate into the react_on_rails/ directory to find the shared implementation.

---

✅ What's Good

1. Correct path resolution: The fix properly accounts for the monorepo structure
2. Consistent with open-source version: The open-source dummy app uses the same pattern (though with 3 levels up instead of 4)
3. Clear variable naming: Renaming gem_root → repo_root better reflects what the variable represents
4. No breaking changes: This is purely a bug fix with no API changes

---

📋 Verification

I verified the fix is correct:

# Starting from: react_on_rails_pro/spec/dummy/bin/
# Four levels up: ../../../.. → repo root
# Path: react_on_rails/spec/support/shakapacker_precompile_hook_shared.rb ✅ EXISTS

The shared hook file exists at the new path and the logic is sound.

---

💡 Minor Suggestions (Optional)

While not blocking, consider these follow-ups:

1. Comment clarity: The comment says "four levels up from react_on_rails_pro/spec/dummy/bin" but it might be clearer to show the expected result:

   # Navigate from react_on_rails_pro/spec/dummy/bin/ up to repo root,
   # then into react_on_rails/spec/support/
   

2. Path validation in CI: Since path-related bugs can be silent (per CLAUDE.md guidelines), consider adding a test that verifies this hook can load successfully. However, this might be overkill for a simple path fix.

---

🔍 Alignment with CLAUDE.md Guidelines

✅ No changelog needed - Internal test infrastructure fix (not user-facing)
✅ Variable naming improved - repo_root is more accurate than gem_root
✅ Follows path management guidelines - Change was verified against actual file structure

---

✅ Recommendation

LGTM (Looks Good To Me) - Ready to merge. This is a straightforward bug fix with no risks.

---

Note: According to CLAUDE.md guidelines for managing file paths, this type of bug (hardcoded paths after refactors) is a known issue. The fix correctly addresses it by updating the path to match the current directory structure.

[claude[bot]]

Code Review - PR #2202: Fix invalid precompile_hook in Pro dummy

✅ Summary

This PR fixes a path resolution bug in the Pro dummy app's precompile hook. The shared hook file is correctly located in react_on_rails/spec/support/, not at the repository root's spec/support/. Good catch!

🔍 Code Quality Assessment

Strengths:

1. ✅ Correct path resolution: The fix properly resolves to react_on_rails/spec/support/shakapacker_precompile_hook_shared.rb which exists
2. ✅ Consistent pattern: Mirrors the approach in the open-source dummy (react_on_rails/spec/dummy/bin/shakapacker-precompile-hook)
3. ✅ Good variable naming: gem_root → repo_root is more accurate for a monorepo context
4. ✅ Fail-fast behavior: Maintains exit 1 when shared hook is missing
5. ✅ Updated comment: Changed from "gem root" to "repo root" for clarity

Path Verification:

• ❌ Old path: spec/support/shakapacker_precompile_hook_shared.rb (doesn't exist)
• ✅ New path: react_on_rails/spec/support/shakapacker_precompile_hook_shared.rb (exists)

📝 Minor Observations

1. Comment accuracy: Line 9 still says "four levels up" but could be more descriptive:

   • Current: react_on_rails_pro/spec/dummy/bin → ../../../.. → repo root
   • Suggestion: Consider clarifying that it navigates from react_on_rails_pro/spec/dummy/bin to the monorepo root

   However, this is a very minor nitpick and not worth blocking the PR over.

2. Changelog entry: As CodeRabbit noted, this is a Pro-specific fix. Consider adding to /CHANGELOG_PRO.md per the repository guidelines. However, since this is a test infrastructure fix (not user-facing), it may not warrant a changelog entry per the guidelines:

   "Do NOT add entries for: linting, formatting, refactoring, tests, or documentation fixes"

🧪 Testing Recommendations

Per CLAUDE.md guidelines on file path changes, please verify:

# 1. Verify the new path exists (already confirmed ✅)
ls -la react_on_rails/spec/support/shakapacker_precompile_hook_shared.rb

# 2. Test that the hook loads successfully
cd react_on_rails_pro/spec/dummy
ruby -c bin/shakapacker-precompile-hook

# 3. If possible, test the actual precompile process
# (May require full Rails app setup)

🎯 Verdict

LGTM ✅ - This is a straightforward bug fix with correct implementation.

Estimated Impact: Low risk, high value

• Fixes broken Pro dummy app infrastructure
• No user-facing changes
• Aligns with existing patterns in the codebase

---

Pro tip: When working with file paths in this monorepo, the structure is:

• react_on_rails/ - Open source package
• react_on_rails_pro/ - Pro package
• Shared test utilities are in react_on_rails/spec/support/, not at repo root

[claude[bot]]

Code Review

✅ Summary

This PR fixes a critical path resolution bug in the Pro dummy app's precompile hook. The fix is correct and necessary.

🎯 What Changed

• File: react_on_rails_pro/spec/dummy/bin/shakapacker-precompile-hook
• Fix: Corrected the path to the shared hook implementation to include react_on_rails/ subdirectory
• Result: Hook can now successfully find and load react_on_rails/spec/support/shakapacker_precompile_hook_shared.rb

✅ Code Quality Assessment

Path Resolution Logic - Excellent

• The fix correctly accounts for the monorepo structure
• From react_on_rails_pro/spec/dummy/bin/, going up 4 levels reaches the repo root
• Adding react_on_rails/ subdirectory correctly navigates to the shared hook

Error Handling - Good

• The hook includes proper error checking (unless File.exist?) with clear error messages
• Will exit with status 1 if the file isn't found, preventing silent failures

Comments - Minor Issue (see below)

• Variable renamed from gem_root to repo_root - semantically more accurate ✅
• However, the comment still says "four levels up" but doesn't mention this lands at the repo root, not inside a gem

🐛 Potential Issues

None - This is a straightforward path fix with no bugs.

🚀 Performance Considerations

No impact - This is a startup hook that runs once during asset precompilation.

🔒 Security Concerns

None - The hook:

• Uses File.expand_path for safe path resolution
• Validates file existence before loading
• No user input or external data involved

🧪 Test Coverage

Implicitly tested - This hook is exercised whenever:

• CI runs Pro dummy app tests
• Developers run rake run_rspec:pro_dummy or similar
• Asset precompilation occurs in the Pro dummy app

The fact that CI passed after this change confirms the path is now correct.

📝 Minor Suggestion - Documentation Clarity

Consider updating the comment to be more explicit:

# Find the repo root directory (four levels up from react_on_rails_pro/spec/dummy/bin)
# Then navigate to react_on_rails/spec/support for the shared hook
repo_root = File.expand_path("../../../..", __dir__)
shared_hook = File.join(repo_root, "react_on_rails", "spec", "support", "shakapacker_precompile_hook_shared.rb")

This makes it clearer why we need the react_on_rails/ subdirectory path component.

✅ Recommendation

Approve and merge. This is a necessary bug fix with no downsides. The CI has passed, confirming the path is correct.

---

Nice catch! This would have caused precompile failures when the hook tried to build ReScript or generate packs in the Pro dummy app.

[claude[bot]]

Code Review - PR #2202: Fix invalid precompile_hook in Pro dummy

✅ Overall Assessment

This is a clean, necessary bugfix that correctly resolves a path resolution issue in the Pro dummy app's precompile hook.

---

🎯 What Changed

The Pro dummy app's shakapacker-precompile-hook was incorrectly trying to load the shared hook implementation from:

• Before: <repo_root>/spec/support/shakapacker_precompile_hook_shared.rb
• After: <repo_root>/react_on_rails/spec/support/shakapacker_precompile_hook_shared.rb

✅ Correctness Verification

Path calculation is correct:

• Starting point: react_on_rails_pro/spec/dummy/bin/
• Four levels up: ../../../.. → Repository root
• Shared hook location: react_on_rails/spec/support/shakapacker_precompile_hook_shared.rb
• ✓ File exists at expected path

Consistency with open-source dummy:

• The open-source dummy (react_on_rails/spec/dummy/bin/) uses three levels up because it's one directory shallower
• Pro dummy correctly uses four levels up due to being nested under react_on_rails_pro/

---

📋 Code Quality

Strengths:

1. ✅ Variable renamed from gem_root to repo_root for clarity (accurate naming)
2. ✅ Comment updated to match the variable name
3. ✅ Error message correctly references the updated path
4. ✅ CI passing (all checks green)
5. ✅ Minimal, focused change - only touches what's necessary

No issues found:

• ✅ No security concerns
• ✅ No performance implications
• ✅ No breaking changes
• ✅ Error handling already in place (exits with code 1 if file not found)

---

🧪 Testing Considerations

Covered by existing mechanisms:

• The hook has built-in validation (unless File.exist? check)
• CI integration tests exercise the Pro dummy app build process
• The fix is self-verifying: if the path were wrong, builds would fail immediately

Optional improvement for future consideration: Consider adding a simple spec that validates both dummy apps' precompile hooks can find the shared implementation:

# spec/react_on_rails/precompile_hook_spec.rb
RSpec.describe 'Precompile hooks' do
  it 'open-source dummy hook finds shared implementation' do
    hook_path = Rails.root.join('spec/dummy/bin/shakapacker-precompile-hook')
    expect(File.exist?(hook_path)).to be true
    # Verify the hook doesn't exit with error
  end
  
  it 'Pro dummy hook finds shared implementation' do
    hook_path = Rails.root.join('react_on_rails_pro/spec/dummy/bin/shakapacker-precompile-hook')
    expect(File.exist?(hook_path)).to be true
  end
end

However, this is not required for this PR - the existing integration tests are sufficient.

---

📝 Documentation & Changelog

PR correctly skips changelog update:

• ✓ This is an internal test infrastructure fix
• ✓ Doesn't affect end users or production applications
• ✓ Per CLAUDE.md guidelines: "Do NOT add entries for... tests, or documentation fixes"

---

🚀 Recommendation

LGTM - Ready to merge ✅

This is a straightforward, necessary fix with:

• Correct implementation
• Passing CI
• No side effects
• Improved variable naming for clarity

No changes requested.

[claude[bot]]

Code Review for PR #2202

Summary

This PR fixes a path resolution issue in the Pro dummy app's shakapacker precompile hook. The hook was attempting to load a shared implementation file using an incorrect path.

✅ What's Good

1. Correct Path Resolution: The fix properly accounts for the monorepo structure where the Pro package is in react_on_rails_pro/ and the shared hook is in react_on_rails/spec/support/.

2. Improved Variable Naming: Changing gem_root to repo_root better reflects the actual directory being referenced (the repository root, not a gem root).

3. Maintains Error Handling: The existing error handling with clear error messages and proper exit code is preserved.

4. Consistent with Pattern: The comment and structure align with the open-source dummy app's implementation at react_on_rails/spec/dummy/bin/shakapacker-precompile-hook.

📝 Observations

1. Path Calculation Verification:

   • From react_on_rails_pro/spec/dummy/bin/, going 4 levels up reaches the repo root (/home/runner/work/react_on_rails/react_on_rails/)
   • The shared hook is correctly located at react_on_rails/spec/support/shakapacker_precompile_hook_shared.rb
   • ✅ Path is correct

2. Comment Accuracy: The comment still says "four levels up" which is accurate for the Pro dummy app location.

3. Testing Considerations:

   • This script is executed during asset precompilation
   • Manual testing recommendation: Run RAILS_ENV=production bundle exec rails assets:precompile in the Pro dummy app to verify the hook loads successfully
   • CI will test this automatically during the integration test suite

⚠️ Potential Improvements (Not Blocking)

1. Add Test Coverage: Consider adding a spec that verifies the precompile hook can find and load the shared implementation. This would catch path issues earlier.

2. Documentation: The shared hook file at line 7-8 documents that both dummy apps use it. This is helpful for future maintainers.

Security & Performance

• ✅ No security concerns - just path resolution
• ✅ No performance impact - runs once during asset precompilation
• ✅ No new dependencies introduced

Conclusion

LGTM ✅ - This is a straightforward bug fix that corrects the path to match the actual monorepo structure. The fix is minimal, focused, and maintains all existing error handling.

The change is safe to merge once CI passes.

[claude[bot]]

Code Review: Fix invalid precompile_hook in Pro dummy

Summary

✅ LGTM - This is a correct and necessary bug fix.

Analysis

Problem Identified: The Pro dummy app's precompile hook was looking for the shared helper at an incorrect path. The old path assumed the shared file was at the repo root level, but in this monorepo structure, the shared file lives in the react_on_rails/ subdirectory.

Path Resolution:

• Old path: repo_root/spec/support/shakapacker_precompile_hook_shared.rb ❌ (doesn't exist)
• New path: repo_root/react_on_rails/spec/support/shakapacker_precompile_hook_shared.rb ✅ (correct)

Verification:

• ✅ Target file exists at new path
• ✅ Open source dummy app (react_on_rails/spec/dummy) uses similar 3-level resolution correctly
• ✅ Pro dummy app (react_on_rails_pro/spec/dummy) now uses correct 4-level resolution with proper subdirectory
• ✅ Comment updated to clarify "repo root" vs "gem root"
• ✅ Error handling already in place with clear error message and exit code

Code Quality: Excellent

1. Correctness: The path logic is sound and mirrors the structure used in the open source dummy
2. Error Handling: Good - already exits with code 1 and descriptive error if file not found
3. Documentation: Comment accurately describes the path resolution
4. Consistency: Aligns with how the open source dummy references the shared helper

Testing Recommendations

Since this affects the build precompile process, please verify:

cd react_on_rails_pro/spec/dummy
bundle exec rails assets:precompile

This should now successfully find and execute the shared hook without errors.

Security: No concerns

• No security implications, just a path fix

Performance: No impact

• No performance considerations

Notes

This is a good example of the monorepo structure requiring careful path resolution. The shared file at react_on_rails/spec/support/shakapacker_precompile_hook_shared.rb (line 7-8) explicitly documents that it's shared between both dummy apps, which this fix now properly honors.

Related Files:

• Shared implementation: react_on_rails/spec/support/shakapacker_precompile_hook_shared.rb
• Open source dummy: react_on_rails/spec/dummy/bin/shakapacker-precompile-hook
• Pro dummy (this fix): react_on_rails_pro/spec/dummy/bin/shakapacker-precompile-hook

---

Recommendation: Approve and merge. This unblocks the Pro dummy app's asset precompilation.