Add Shakapacker precompile hook support for auto pack generation

[justin808]

Summary

This PR leverages Shakapacker's new precompile_hook feature to automatically generate packs before webpack compilation, replacing the need to manually modify assets:precompile tasks or bin/dev scripts.

Base branch: justin808/shakapacker-9.3.0 (to be merged after that PR is merged to master)

Changes

• Added PackerUtils.shakapacker_precompile_hook_configured? method to detect when hook is configured
• Updated configuration.rb to skip generate_packs when hook is configured
• Updated PackGenerator to skip pack generation when hook is configured
• Updated ServerManager to avoid redundant pack generation in bin/dev
• Updated base generator to copy the hook script and make it executable
• Added bin/shakapacker-precompile-hook script template for generator
• Updated shakapacker.yml templates to configure precompile_hook

Benefits

• Cleaner integration with Shakapacker - leverages built-in hook system
• No need to modify assets:precompile tasks manually
• bin/dev won't redundantly run generate_packs when hook is configured
• Automatic pack generation happens at the right time (before webpack compilation)
• Properly validated by Shakapacker to ensure script is within project root

Note on spec/dummy/bin/shakapacker-precompile-hook

The shakapacker-9.3.0 branch already has a comprehensive precompile hook that includes ReScript support and cross-platform compatibility. This PR adds the React on Rails specific logic to work with that existing hook infrastructure.

Test plan

• Cherry-picked changes on top of shakapacker-9.3.0 branch
• Resolved merge conflicts, keeping the better precompile hook version
• All RuboCop checks pass
• Generator correctly copies and makes hook script executable

🤖 Generated with Claude Code

---

This change is 

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch justin808/precompile-hook-on-9.3.0

---

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 - PR #1905: Add Shakapacker precompile hook support

Summary

This PR successfully implements integration with Shakapacker's precompile hook feature to automatically generate packs before webpack compilation. The implementation is clean, well-structured, and follows the repository's conventions.

✅ Strengths

1. Smart Detection Logic - The shakapacker_precompile_hook_configured? method properly handles errors and gracefully returns false on failure, making it robust.

2. Clean Integration Points - The hook is checked in three appropriate places:

   • configuration.rb:242 - Skip during assets:precompile
   • dev/pack_generator.rb:11 - Skip during bin/dev
   • dev/server_manager.rb:413 - Conditional UI messaging

3. Good Error Handling - The hook script template properly catches errors, displays them, and exits with status 1.

4. File Permissions - The generator correctly sets executable permissions (0755) on the hook script at base_generator.rb:49.

5. User Experience - Clear messaging to users about what's happening, with conditional display based on hook configuration.

🔍 Potential Issues & Suggestions

1. Missing Test Coverage (High Priority)

The new shakapacker_precompile_hook_configured? method in packer_utils.rb:172-181 lacks test coverage.

Recommendation: Add tests to spec/react_on_rails/packer_utils_spec.rb:

describe ".shakapacker_precompile_hook_configured?" do
  let(:mock_config) { instance_double("Shakapacker::Config") }
  
  before do
    allow(::Shakapacker).to receive(:config).and_return(mock_config)
  end

  it "returns true when precompile_hook is configured" do
    allow(mock_config).to receive(:send).with(:data).and_return({ precompile_hook: 'bin/shakapacker-precompile-hook' })
    expect(described_class.shakapacker_precompile_hook_configured?).to be(true)
  end

  it "returns false when precompile_hook is nil" do
    allow(mock_config).to receive(:send).with(:data).and_return({ precompile_hook: nil })
    expect(described_class.shakapacker_precompile_hook_configured?).to be(false)
  end

  it "returns false when precompile_hook is empty string" do
    allow(mock_config).to receive(:send).with(:data).and_return({ precompile_hook: '' })
    expect(described_class.shakapacker_precompile_hook_configured?).to be(false)
  end

  it "returns false when Shakapacker is not defined" do
    hide_const("::Shakapacker")
    expect(described_class.shakapacker_precompile_hook_configured?).to be(false)
  end

  it "returns false on StandardError" do
    allow(mock_config).to receive(:send).and_raise(StandardError.new("test error"))
    expect(described_class.shakapacker_precompile_hook_configured?).to be(false)
  end
end

2. Private Method Access (Medium Priority)

packer_utils.rb:175 uses .send(:data) to access a private method on Shakapacker config.

Concerns:

• Fragile - breaks if Shakapacker changes internal structure
• Violates encapsulation principle

Recommendation: Consider alternatives:

• Request Shakapacker add a public accessor for precompile_hook
• Access via YAML parsing: YAML.load_file(Rails.root.join("config/shakapacker.yml"))[Rails.env]["precompile_hook"]
• Add a comment documenting this dependency on internal API

3. Rainbow Dependency (Low Priority)

The hook script template (shakapacker-precompile-hook:14,17) uses Rainbow for colored output, but doesn't check if it's available.

Recommendation: Add graceful fallback:

def colorize(text, color)
  defined?(Rainbow) ? Rainbow(text).send(color) : text
end

puts colorize("🔄 Running React on Rails precompile hook...", :cyan)

4. File Permission Check Timing (Low Priority)

base_generator.rb:49 - The permission change happens after copy_file, but copy_file should preserve source permissions if the source has them set.

Recommendation: Consider adding the shebang's execute bit to the source file in the repo, then the chmod becomes defensive rather than required.

📊 Performance Considerations

• The hook detection (shakapacker_precompile_hook_configured?) is called multiple times but not memoized. Consider adding instance variable caching:

  def self.shakapacker_precompile_hook_configured?
    return @hook_configured if defined?(@hook_configured)
    
    @hook_configured = begin
      # ... existing logic
    end
  end

🔒 Security Review

✅ The PR description correctly notes that Shakapacker validates the hook script is within project root, preventing path traversal attacks.

📝 Code Quality

• ✅ Follows Ruby style guidelines
• ✅ Consistent with existing codebase patterns
• ✅ Clear, descriptive variable names
• ✅ Appropriate use of comments
• ✅ Proper error handling

🎯 Recommendations Priority

Before Merge:

1. Add comprehensive test coverage for shakapacker_precompile_hook_configured?

Consider for Follow-up:
2. Add memoization to reduce redundant checks
3. Add graceful fallback for Rainbow dependency
4. Document the private method dependency or find alternative

Overall Assessment

This is a well-implemented feature that elegantly integrates with Shakapacker's hook system. The main gap is test coverage for the new detection method. Once tests are added, this will be ready to merge.

🤖 Generated with Claude Code

[justin808]

Closing this PR as the precompile hook functionality has already been fully implemented on master via PRs #1861 and #1916. The master branch has a more comprehensive implementation that includes script file reading and better error handling.