Add Shakapacker precompile hook template for generator

[justin808]

Summary

Adds the Shakapacker precompile hook template file that will be used by the React on Rails generator when creating new projects. This template provides automatic pack generation support for projects using auto-bundling.

Changes

• Added generator template: lib/generators/react_on_rails/templates/base/base/bin/shakapacker-precompile-hook

  • Uses require_relative "../config/environment" to properly load Rails environment
  • Calls ReactOnRails::PacksGenerator.instance.generate_packs_if_stale to auto-generate packs
  • Includes proper error handling and colored output

• Updated base generator: Modified to copy the precompile hook template to new projects

• Updated shakapacker.yml templates: Added precompile_hook configuration

• Enhanced pack generator and server manager: Added support for detecting and using the precompile hook

Context

The dummy app already has its own version of this hook (merged via PR #1904) which includes the find_rails_root function for robust directory detection. This PR adds the template version that will be used for newly generated projects.

Benefits

• New React on Rails projects will automatically get the precompile hook configured
• Automatic pack generation during Shakapacker compilation
• Consistent setup across all new projects
• Better developer experience with less manual configuration

Testing

• ✅ RuboCop passes with no violations
• ✅ All pre-commit and pre-push hooks pass
• ✅ Clean rebase on latest master

Related to #1912

🤖 Generated with Claude Code

---

This change is 

Summary by CodeRabbit

• New Features

  • Added a Shakapacker precompile hook script and a new precompile_hook configuration option.

• Improvements

  • Pack generation now skips when a precompile hook is configured to avoid redundant builds.
  • Development server help text and displayed features updated; hook script is ensured executable.

• Tests

  • Added tests covering precompile hook detection and generator skipping behavior.

• Chores

  • Minor CI workflow comment/config tuning.

[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 11 minutes and 23 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 6979fff and 6f51900.

📒 Files selected for processing (9)

• .github/workflows/examples.yml (1 hunks)
• lib/generators/react_on_rails/base_generator.rb (1 hunks)
• lib/generators/react_on_rails/templates/base/base/bin/shakapacker-precompile-hook (1 hunks)
• lib/generators/react_on_rails/templates/base/base/config/shakapacker.yml (1 hunks)
• lib/react_on_rails/dev/server_manager.rb (4 hunks)
• lib/react_on_rails/packer_utils.rb (1 hunks)
• spec/dummy/config/shakapacker.yml (1 hunks)
• spec/react_on_rails/dev/pack_generator_spec.rb (1 hunks)
• spec/react_on_rails/packer_utils_spec.rb (1 hunks)

Walkthrough

Adds a Shakapacker precompile hook: generator now installs an executable bin/shakapacker-precompile-hook, shakapacker.yml gains a precompile_hook option, and pack generation is skipped when that hook is configured; supporting utilities, server-manager text, and tests were added.

Changes

Cohort / File(s)	Summary
Generator & Hook lib/generators/react_on_rails/base_generator.rb, lib/generators/react_on_rails/templates/base/base/bin/shakapacker-precompile-hook	Adds bin/shakapacker-precompile-hook to generated base files and makes it executable; new hook script loads Rails and invokes pack generation with error handling and colored logging.
Shakapacker config templates lib/generators/react_on_rails/templates/base/base/config/shakapacker.yml, spec/dummy/config/shakapacker.yml	Adds precompile_hook entry (default 'bin/shakapacker-precompile-hook') with security comments to Shakapacker YAML templates.
Pack generation gating lib/react_on_rails/configuration.rb, lib/react_on_rails/dev/pack_generator.rb	Adjusts precompile task and PackGenerator to skip react_on_rails pack generation when a Shakapacker precompile hook is configured (early-return/conditional invocation).
Packer utilities lib/react_on_rails/packer_utils.rb	Adds self.shakapacker_precompile_hook_configured? to detect presence of precompile_hook in Shakapacker config (safe/rescuing).
Server manager messaging lib/react_on_rails/dev/server_manager.rb	Updates HMR/static help text and feature lists to reflect precompile hook or bin/dev-driven pack generation; introduces dynamic features arrays.
Tests spec/react_on_rails/dev/pack_generator_spec.rb, spec/react_on_rails/packer_utils_spec.rb	Adds tests for hook-configured behavior (skipping generation and not invoking rake task) and unit tests for shakapacker_precompile_hook_configured? across scenarios.
CI workflow comment .github/workflows/examples.yml	Removes/enables commenting out Yarn cache options in examples workflow with TODO comments preserved.

Sequence Diagram(s)

sequenceDiagram
    participant Precompile as Rails Precompile
    participant Config as ReactOnRails::Configuration
    participant PackerUtils as ReactOnRails::PackerUtils
    participant Hook as bin/shakapacker-precompile-hook
    participant Gen as PackGenerator / rake task

    Precompile->>Config: adjust_precompile_task
    Config->>PackerUtils: shakapacker_precompile_hook_configured?
    alt precompile_hook configured
        Config->>Hook: rely on configured hook (exec)
        Hook->>Hook: load Rails env, call generate_packs_if_stale
        Hook-->>Precompile: exit success/failure
    else no hook
        Config->>Gen: invoke react_on_rails:generate_packs
        Gen->>Gen: generate packs inline
        Gen-->>Precompile: return status
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

• Pay attention to interplay between packer_utils.shakapacker_precompile_hook_configured?, configuration.rb adjustments, and PackGenerator.generate early-return logic.
• Review the new hook script for environment loading, security assumptions, and executable permissions.
• Verify tests properly stub Shakapacker presence/absence and rake invocation expectations.

Possibly related issues

• Fix precompile hook to properly locate Rails root #1912 — touches the same bin/shakapacker-precompile-hook; relevant for refinements like Rails root detection and hook robustness.

Possibly related PRs

• Upgrade Shakapacker from 8.2.0 to 9.0.0 #1904 — implements/changes precompile hook behavior and configuration (directly related).
• Fix bin/dev pack generation in Bundler context #1907 — modifies PackGenerator flow around generation/Bundler contexts (related to early-skip behavior).
• 🚀 React on Rails v15 Generator Overhaul & Developer Experience Enhancement for v16  #1770 — also modifies Shakapacker/dev pack generation orchestration (related scope).

Suggested reviewers

• AbanoubGhadban
• alexeyr-ci

Poem

🐇 Hop! The hook is in the bin,

Packs skip when scripts begin —
Rails hums light, no duplicate spin,
A tidy build, a quieter din. 🥕✨

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: adding a Shakapacker precompile hook template for the React on Rails generator.

---

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]

Pull Request Review

Summary

This PR introduces Shakapacker precompile hook support to automatically generate React on Rails packs before webpack compilation. The implementation is well-designed and solves the issue of manual task configuration, but there are several concerns that should be addressed.

---

🔴 Critical Issues

1. Inconsistent Hook Script Templates

The template in lib/generators/react_on_rails/templates/base/base/bin/shakapacker-precompile-hook (20 lines) differs significantly from the one in spec/dummy/bin/shakapacker-precompile-hook (100 lines). This is a major concern:

• Template version (20 lines): Simple, assumes Rails environment is loadable via require_relative "../config/environment"
• Dummy app version (100 lines): Complex, with Rails root finding, cross-platform support, ReScript building

Issue: When users run the generator, they'll get the simple 20-line version, which may fail in contexts where the Rails root isn't easily resolvable (the exact issue #1912 is trying to fix).

Recommendation: The generator template should match the more robust implementation in spec/dummy. Users need the same reliability.

2. Missing Tests for New Functionality

No tests were added for the new shakapacker_precompile_hook_configured? method in PackerUtils.

Required tests (in spec/react_on_rails/packer_utils_spec.rb):

• When precompile_hook is configured in shakapacker.yml
• When precompile_hook is nil/empty
• When Shakapacker is not defined
• When accessing config raises an error

3. Missing Tests for Skip Logic

The skip logic added to PackGenerator.generate and Configuration#adjust_precompile_task lacks test coverage.

Required tests:

• spec/react_on_rails/dev/pack_generator_spec.rb: Test that pack generation is skipped when hook is configured
• Integration test for adjust_precompile_task behavior

---

⚠️ High Priority Issues

4. Security: Rainbow Gem Dependency

The template uses Rainbow() for colored output but doesn't require the gem:

# lib/generators/react_on_rails/templates/base/base/bin/shakapacker-precompile-hook:14
puts Rainbow("🔄 Running React on Rails precompile hook...").cyan

Issue: This will fail if Rainbow isn't loaded. The dummy app version doesn't use Rainbow.

Recommendation: Either add require 'rainbow' or remove Rainbow usage from the template (use plain strings like the dummy version).

5. Error Handling Inconsistency

• Template: Uses Rainbow() for errors (lines 14, 17)
• Dummy app: Uses plain strings with emojis

Recommendation: Use consistent, simple error messages without external dependencies.

6. File Permissions on Windows

# lib/generators/react_on_rails/base_generator.rb:49
File.chmod(0o755, "bin/shakapacker-precompile-hook") if File.exist?("bin/shakapacker-precompile-hook")

Issue: chmod may not work as expected on Windows systems.

Recommendation: Add error handling or platform detection:

begin
  File.chmod(0o755, "bin/shakapacker-precompile-hook") if File.exist?("bin/shakapacker-precompile-hook")
rescue Errno::EACCES, Errno::EPERM => e
  # Silently ignore on Windows or systems where chmod isn't supported
end

---

💡 Code Quality Issues

7. Regex Parsing for Configuration Detection

# spec/dummy/bin/shakapacker-precompile-hook:64-65
has_auto_load = config_file =~ /^\s*config\.auto_load_bundle\s*=/
has_components_subdir = config_file =~ /^\s*config\.components_subdirectory\s*=/

Issues:

• Fragile: Won't catch commented-out configs that span multiple lines
• Won't detect configs with different whitespace patterns
• Could match strings in comments despite the regex attempting to avoid this

Recommendation: Consider a more robust approach, possibly loading and inspecting the actual Rails configuration or using AST parsing.

8. Silent Failures in Dummy Hook

Multiple places return silently without indication:

return unless rails_root  # Line 56
return unless File.exist?(initializer_path)  # Line 60
return unless has_auto_load || has_components_subdir  # Line 66
return unless bundle_available  # Line 72
return unless task_list.include?("react_on_rails:generate_packs")  # Line 76

Issue: In debugging scenarios, it's unclear why pack generation was skipped.

Recommendation: Add optional verbose mode or debug logging:

return unless rails_root
puts "ℹ️  Rails root not found, skipping pack generation" if ENV['DEBUG']

9. Missing Edge Case: What if Hook is Self-Referential?

The method shakapacker_precompile_hook_configured? checks if a hook exists but doesn't validate that it's the React on Rails hook.

Scenario: A project might already have a different precompile hook configured.

Recommendation: Consider checking if the hook points to the React on Rails hook script specifically, or document this limitation.

---

📝 Documentation & Testing Issues

10. Missing Changelog Entry

The PR checklist mentions updating CHANGELOG but it's unchecked. According to CLAUDE.md, user-visible changes should be documented.

Required: Add CHANGELOG entry describing this new feature.

11. Missing Documentation Updates

The PR checklist mentions updating documentation but it's unchecked.

Required: Document:

• How the precompile hook works
• When it's used vs. manual task configuration
• Migration guide for existing projects
• Troubleshooting common issues

12. Pull Request Description is Template Boilerplate

The PR body is still the default template text.

Required: Fill in:

• What this PR does
• Why this approach was chosen
• Links to related issues (Fix precompile hook to properly locate Rails root #1912)
• Breaking changes (if any)
• Migration guide for existing users

---

✅ Positive Aspects

1. Clean Architecture: The hook detection logic properly prevents duplicate pack generation
2. Backward Compatible: Doesn't break existing setups without the hook
3. Graceful Degradation: Multiple fallbacks in the dummy hook script
4. Cross-platform Consideration: The dummy version handles both yarn and npm
5. Security Note: Good security comment in shakapacker.yml about trusted scripts

---

🔧 Recommendations Summary

Must Fix Before Merge:

1. ✅ Sync template file with dummy app implementation (or document why they differ)
2. ✅ Add tests for shakapacker_precompile_hook_configured?
3. ✅ Add tests for skip logic in PackGenerator and Configuration
4. ✅ Fix Rainbow dependency issue in template
5. ✅ Fill out PR description
6. ✅ Add CHANGELOG entry
7. ✅ Update documentation

Should Fix:

8. Add platform-safe chmod error handling
9. Improve configuration detection robustness
10. Add debug logging for silent returns

Nice to Have:

11. Consider validation that hook points to correct script
12. Add integration test for full precompile workflow

---

📋 Checklist Progress

Based on the PR template:

• ❌ Add/update test to cover these changes
• ❌ Update documentation
• ❌ Update CHANGELOG file

Overall Assessment: Good feature implementation but needs test coverage, documentation, and template consistency fixes before merging.

---

Let me know if you'd like me to help implement any of these fixes! 🚀

[claude]

Code Review - PR #1916

Overview

This PR adds a Shakapacker precompile hook template for the React on Rails generator, enabling automatic pack generation for newly generated projects. The implementation is well-structured and addresses an important DX improvement.

---

✅ Strengths

1. Clean Architecture & Separation of Concerns

• Proper separation between the dummy app's complex hook (with find_rails_root, ReScript support) and the simpler generator template
• Smart detection logic via shakapacker_precompile_hook_configured? prevents duplicate pack generation
• Well-integrated with existing PackGenerator and ServerManager

2. Error Handling

• packer_utils.rb:172-181: Robust error handling with rescue StandardError in shakapacker_precompile_hook_configured?
• shakapacker-precompile-hook:13-20: Good error reporting with colored output and backtrace (limited to 5 lines)
• Proper exit codes on failure

3. Documentation & Code Quality

• Clear inline comments explaining the hook's purpose and configuration
• Security note in shakapacker.yml about trusted scripts
• Consistent with CLAUDE.md guidelines (trailing newlines, no manual formatting)

4. File Permissions Handling

• base_generator.rb:48-49: Automatically makes the hook executable after copying (chmod 0755)

---

🔍 Issues & Concerns

1. Missing Test Coverage ⚠️ HIGH PRIORITY

The new shakapacker_precompile_hook_configured? method in packer_utils.rb:172-181 has zero test coverage. This is critical functionality that controls whether pack generation runs.

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

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

  context "when precompile_hook is not configured" do
    it "returns false for 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 for 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
  end

  context "when Shakapacker is not available" do
    before { hide_const("::Shakapacker") }
    
    it "returns false" do
      expect(described_class.shakapacker_precompile_hook_configured?).to be false
    end
  end

  context "when config.send raises an error" do
    it "returns false" do
      allow(mock_config).to receive(:send).and_raise(NoMethodError)
      expect(described_class.shakapacker_precompile_hook_configured?).to be false
    end
  end
end

2. PackGenerator Skip Logic Needs Tests

dev/pack_generator.rb:11-14 adds skip logic but spec/react_on_rails/dev/pack_generator_spec.rb doesn't test this new behavior.

Recommendation: Add test case:

context "when shakapacker precompile hook is configured" do
  before do
    allow(ReactOnRails::PackerUtils).to receive(:shakapacker_precompile_hook_configured?).and_return(true)
  end

  it "skips pack generation in verbose mode" do
    expect { described_class.generate(verbose: true) }
      .to output(/⏭️  Skipping pack generation/).to_stdout_from_any_process
  end

  it "does not invoke the rake task" do
    described_class.generate(verbose: false)
    expect(mock_task).not_to have_received(:invoke)
  end
end

3. Potential Bug: File Existence Race Condition

base_generator.rb:49:

File.chmod(0o755, "bin/shakapacker-precompile-hook") if File.exist?("bin/shakapacker-precompile-hook")

This uses a relative path, but generators might run from different directories. The copy_file on line 43 should guarantee the file exists, making the conditional redundant.

Recommendation: Either:

1. Remove the conditional (file will always exist after copy_file)
2. Use absolute path: File.join(destination_root, "bin/shakapacker-precompile-hook")

4. Template Hook Simpler Than Dummy Hook - Intentional?

The dummy app hook (spec/dummy/bin/shakapacker-precompile-hook) has:

• find_rails_root function for robust directory detection
• ReScript build support
• More defensive checks

The template hook uses require_relative "../config/environment" which assumes:

• The hook is always run from project root, OR
• The hook is run from bin/ directory

Question: Is this simplified approach intentional for generated projects? If Shakapacker runs the hook from a different directory during asset compilation, it might fail.

Recommendation: Consider if generated projects need the find_rails_root robustness, or document that the hook assumes it runs from project root.

5. Rainbow Dependency - Already Present ✅

Verified that rainbow gem is already a runtime dependency in react_on_rails.gemspec, so no issue here.

---

💡 Minor Suggestions

1. Security: Script Path Validation

While the YAML comment mentions "The hook command will be validated to ensure it points to a file within the project root", I don't see validation code in this PR. Is this handled by Shakapacker itself?

2. Documentation String

dev/pack_generator.rb:12: Consider more descriptive message:

puts "⏭️  Skipping pack generation (delegated to Shakapacker precompile hook)" if verbose

3. Consistent Emoji Style

Good use of emojis throughout for better UX! Consider documenting the emoji scheme:

• 🔄 = Running/in-progress
• ✅ = Success
• ❌ = Error
• 📦 = Pack generation
• ⏭️ = Skip

---

🔒 Security Assessment

No Security Issues Found ✅

1. Script properly uses shebang and frozen_string_literal
2. No command injection vectors (no shell interpolation)
3. Proper use of Ruby's native methods vs shell commands
4. Exit codes properly propagated

---

🎯 Performance Considerations

Positive Impact ✅

• Eliminates duplicate pack generation when hook is configured
• Skip checks are lightweight (present? on config hash)
• No performance regressions expected

---

📋 Testing Checklist

Before merging, please ensure:

• Add tests for PackerUtils.shakapacker_precompile_hook_configured?
• Add tests for PackGenerator skip behavior when hook is configured
• Test generator creates executable file with correct permissions
• Verify hook works when run from different working directories
• Test error handling when ReactOnRails::PacksGenerator.instance is unavailable
• Integration test: Generate new project and verify hook runs during compilation

---

📝 Documentation

Consider adding to docs:

1. How the precompile hook interacts with pack generation
2. When to use the hook vs manual pack generation
3. Troubleshooting guide if the hook fails

---

✅ Final Recommendation

APPROVE with minor changes requested

This is solid work that improves the DX for new projects. The main blocker is the missing test coverage for the new shakapacker_precompile_hook_configured? method. Once tests are added, this is ready to merge.

The code quality is high, follows project conventions, and integrates cleanly with existing systems. The skip logic is well-designed and prevents unnecessary duplicate work.

---

Priority Actions:

1. 🔴 HIGH: Add test coverage for new PackerUtils method
2. 🟡 MEDIUM: Add test for PackGenerator skip behavior
3. 🟢 LOW: Review file existence check in generator
4. 🟢 LOW: Document hook directory assumptions

Great work on this feature! 🚀

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

spec/dummy/config/shakapacker.yml (1)

14-14: Critical: Duplicate YAML key - remove this line.

This precompile_hook key is duplicated on lines 28-31. YAML does not allow duplicate keys in the same mapping. Remove this line since the more complete definition with security comments exists below.

Apply this diff:

-  precompile_hook: bin/shakapacker-precompile-hook

lib/react_on_rails/dev/server_manager.rb (1)

251-256: Remove redundant "Fast recompilation" bullet.

Line 255 adds a "Fast recompilation" bullet, but Line 252 already mentions "Webpack dev server for fast recompilation." These convey the same information.

Apply this diff to remove the redundant bullet:

            #{Rainbow('•').yellow} #{Rainbow('React on Rails pack generation (via precompile hook or bin/dev)').white}
            #{Rainbow('•').yellow} #{Rainbow('Webpack dev server for fast recompilation').white}
            #{Rainbow('•').yellow} #{Rainbow('Source maps for debugging').white}
            #{Rainbow('•').yellow} #{Rainbow('May have Flash of Unstyled Content (FOUC)').white}
-            #{Rainbow('•').yellow} #{Rainbow('Fast recompilation').white}
            #{Rainbow('•').yellow} #{Rainbow('Access at:').white} #{Rainbow('http://localhost:3000/<route>').cyan.underline}

🧹 Nitpick comments (2)

lib/react_on_rails/packer_utils.rb (1)

170-181: Consider the fragility of accessing private methods.

The implementation correctly detects the precompile hook configuration. However, line 175 uses send(:data) to access Shakapacker's private method. While this appears necessary to read the raw config data, it's fragile and could break with Shakapacker updates.

The error handling with rescue StandardError provides a safe fallback, which mitigates this risk.

If Shakapacker provides a public API to access config values in the future, consider switching to that approach for better maintainability. For now, the current implementation with error handling is acceptable.

lib/react_on_rails/dev/server_manager.rb (1)

284-293: Consider clarifying when each pack generation mechanism is used.

The comment on lines 291-292 mentions two pack generation mechanisms but doesn't specify when each is used. Consider making this more explicit for maintainability.

-          # NOTE: Pack generation happens automatically during assets:precompile
-          # either via precompile hook or via the configuration.rb adjust_precompile_task
+          # NOTE: Pack generation happens automatically during assets:precompile:
+          # - via precompile hook (if configured in shakapacker.yml)
+          # - or via configuration.rb adjust_precompile_task (fallback if no hook configured)

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ae5425b and 9431cdc.

📒 Files selected for processing (10)

• lib/generators/react_on_rails/base_generator.rb (1 hunks)
• lib/generators/react_on_rails/templates/base/base/bin/shakapacker-precompile-hook (1 hunks)
• lib/generators/react_on_rails/templates/base/base/config/shakapacker.yml (1 hunks)
• lib/react_on_rails/configuration.rb (1 hunks)
• lib/react_on_rails/dev/pack_generator.rb (1 hunks)
• lib/react_on_rails/dev/server_manager.rb (4 hunks)
• lib/react_on_rails/packer_utils.rb (1 hunks)
• spec/dummy/config/shakapacker.yml (1 hunks)
• spec/react_on_rails/dev/pack_generator_spec.rb (2 hunks)
• spec/react_on_rails/packer_utils_spec.rb (1 hunks)

🧰 Additional context used

🧠 Learnings (11)

📚 Learning: 2025-02-18T13:08:01.477Z

Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation in React on Rails Pro is handled through a chain of validations:
1. Pro version check in `run_stream_inside_fiber`
2. RSC support check during pack generation via `ReactOnRailsPro.configuration.enable_rsc_support`
3. RSC support validation during component registration
This makes additional validation in the helper methods unnecessary.

Applied to files:

• spec/react_on_rails/dev/pack_generator_spec.rb
• lib/react_on_rails/packer_utils.rb
• spec/react_on_rails/packer_utils_spec.rb
• lib/generators/react_on_rails/templates/base/base/bin/shakapacker-precompile-hook

📚 Learning: 2025-10-23T17:22:01.074Z

Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1875
File: lib/react_on_rails/utils.rb:112-124
Timestamp: 2025-10-23T17:22:01.074Z
Learning: In React on Rails, when Pro is installed but not licensed, the intended behavior is to raise an error on boot. The `react_on_rails_pro?` method validates licenses and should raise errors early (including during path resolution in methods like `server_bundle?`) to enforce licensing requirements rather than failing later with obscure errors.

Applied to files:

• spec/react_on_rails/dev/pack_generator_spec.rb
• lib/react_on_rails/packer_utils.rb
• lib/generators/react_on_rails/templates/base/base/bin/shakapacker-precompile-hook

📚 Learning: 2024-10-08T20:53:47.076Z

Learnt from: justin808
Repo: shakacode/react_on_rails PR: 1622
File: spec/dummy/spec/rake/assets_precompile_rake_spec.rb:12-12
Timestamp: 2024-10-08T20:53:47.076Z
Learning: When stubbing environment variables in RSpec tests, use `before` and `after` hooks to ensure that the original values are restored after the tests, preventing any side effects on other tests. Example provided by justin808:
```ruby
describe "My test" do
  before do
    original_value = ENV["VARIABLE_NAME"]
    allow(ENV).to receive(:[]).with("VARIABLE_NAME").and_return("stubbed_value")
  end

  after do
    allow(ENV).to receive(:[]).with("VARIABLE_NAME").and_call_original
    ENV["VARIABLE_NAME"] = original_value
  end

  it "tests something" do
    # Your test code here
  end
end
```
This practice ensures test isolation and reliability.

Applied to files:

• spec/react_on_rails/dev/pack_generator_spec.rb

📚 Learning: 2025-02-13T14:29:49.267Z

Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: spec/react_on_rails/utils_spec.rb:218-218
Timestamp: 2025-02-13T14:29:49.267Z
Learning: In RSpec tests, prefer using local variables over constants within test blocks to avoid constant redefinition warnings and maintain better test isolation.

Applied to files:

• spec/react_on_rails/dev/pack_generator_spec.rb
• spec/react_on_rails/packer_utils_spec.rb

📚 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:

• spec/react_on_rails/dev/pack_generator_spec.rb
• lib/generators/react_on_rails/templates/base/base/config/shakapacker.yml
• lib/react_on_rails/configuration.rb
• lib/react_on_rails/dev/server_manager.rb
• lib/generators/react_on_rails/templates/base/base/bin/shakapacker-precompile-hook

📚 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:

• lib/generators/react_on_rails/templates/base/base/config/shakapacker.yml
• lib/generators/react_on_rails/base_generator.rb
• lib/react_on_rails/configuration.rb
• lib/react_on_rails/dev/server_manager.rb
• lib/generators/react_on_rails/templates/base/base/bin/shakapacker-precompile-hook

📚 Learning: 2025-07-08T05:57:29.630Z

Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1745
File: node_package/src/RSCRequestTracker.ts:8-14
Timestamp: 2025-07-08T05:57:29.630Z
Learning: The global `generateRSCPayload` function in React on Rails Pro (RORP) is provided by the framework during rendering requests, not implemented in application code. The `declare global` statements are used to document the expected interface that RORP will inject at runtime.

Applied to files:

• lib/generators/react_on_rails/templates/base/base/config/shakapacker.yml
• lib/generators/react_on_rails/templates/base/base/bin/shakapacker-precompile-hook

📚 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:

• lib/generators/react_on_rails/base_generator.rb
• lib/react_on_rails/configuration.rb
• lib/react_on_rails/dev/server_manager.rb
• lib/generators/react_on_rails/templates/base/base/bin/shakapacker-precompile-hook

📚 Learning: 2025-09-29T14:45:42.687Z

Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1833
File: lib/react_on_rails/dev/process_manager.rb:72-83
Timestamp: 2025-09-29T14:45:42.687Z
Learning: In Ruby bundler contexts, when bundler intercepts system commands for executables not in the Gemfile, both version checks (like `system("foreman", "--version")`) and execution commands (like `system("foreman", "start", ...)`) fail equally, both returning false. This means availability checks using version flags accurately reflect whether execution commands will work in the current bundler context.

Applied to files:

• spec/react_on_rails/packer_utils_spec.rb

📚 Learning: 2025-09-15T21:24:48.207Z

Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1781
File: node_package/src/ClientSideRenderer.ts:82-95
Timestamp: 2025-09-15T21:24:48.207Z
Learning: In React on Rails, the force_load feature includes both explicit `data-force-load="true"` usage and the ability to hydrate components during the page loading state (`document.readyState === 'loading'`). Both capabilities require a Pro license, so the condition `!railsContext.rorPro && (isComponentForceLoaded || document.readyState === 'loading')` correctly gates both scenarios.

Applied to files:

• lib/react_on_rails/dev/server_manager.rb

📚 Learning: 2025-04-09T12:56:10.756Z

Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1696
File: node_package/src/RSCPayloadContainer.ts:0-0
Timestamp: 2025-04-09T12:56:10.756Z
Learning: In the react_on_rails codebase, RSC payloads are already stringified using `JSON.stringify()` before being processed by the `escapeScript` function, which handles escaping of special characters. The function only needs to handle specific HTML markers like comments and closing script tags.

Applied to files:

• lib/generators/react_on_rails/templates/base/base/bin/shakapacker-precompile-hook

🧬 Code graph analysis (5)

spec/react_on_rails/dev/pack_generator_spec.rb (1)

lib/react_on_rails/dev/pack_generator.rb (2)

• generate (8-132)
• generate (10-30)

lib/react_on_rails/dev/pack_generator.rb (1)

lib/react_on_rails/packer_utils.rb (1)

• shakapacker_precompile_hook_configured? (172-181)

spec/react_on_rails/packer_utils_spec.rb (1)

lib/react_on_rails/packer_utils.rb (1)

• shakapacker_precompile_hook_configured? (172-181)

lib/react_on_rails/configuration.rb (1)

lib/react_on_rails/packer_utils.rb (1)

• shakapacker_precompile_hook_configured? (172-181)

lib/react_on_rails/dev/server_manager.rb (1)

lib/react_on_rails/packer_utils.rb (1)

• shakapacker_precompile_hook_configured? (172-181)

🪛 YAMLlint (1.37.1)

spec/dummy/config/shakapacker.yml

[error] 31-31: duplication of key "precompile_hook" in mapping

(key-duplicates)

⏰ 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). (5)

• GitHub Check: examples (3.2, minimum)
• GitHub Check: rspec-package-tests (3.4, latest)
• GitHub Check: rspec-package-tests (3.2, minimum)
• GitHub Check: build
• GitHub Check: claude-review

🔇 Additional comments (11)

lib/react_on_rails/configuration.rb (1)

241-244: LGTM! Clean conditional logic for hook detection.

The implementation correctly skips pack generation when a Shakapacker precompile hook is configured, avoiding redundant work. The comment clearly explains the purpose.

spec/dummy/config/shakapacker.yml (1)

28-31: Keep this definition; remove the duplicate on line 14.

This is the correct precompile_hook definition with proper documentation. The duplicate on line 14 should be removed.

lib/generators/react_on_rails/base_generator.rb (2)

40-41: LGTM! Precompile hook added to base files.

The hook script is correctly included in the base files list for copying.

---

48-49: LGTM! Proper executable permissions set.

The chmod is correctly applied after the file is copied, ensuring the hook script is executable. The 0o755 permissions are appropriate for a shell/script file.

spec/react_on_rails/packer_utils_spec.rb (1)

143-183: LGTM! Comprehensive test coverage for hook detection.

The test suite thoroughly covers all scenarios:

• Hook configured (returns true)
• Hook not configured with nil/empty values (returns false)
• Shakapacker not available (returns false)
• Error handling (returns false)

The use of hide_const and proper mocking demonstrates good testing practices.

lib/react_on_rails/dev/pack_generator.rb (1)

11-15: LGTM! Clean early-exit pattern for hook detection.

The guard clause correctly skips pack generation when a Shakapacker precompile hook is configured, providing appropriate feedback in verbose mode while remaining silent otherwise.

spec/react_on_rails/dev/pack_generator_spec.rb (2)

8-33: LGTM! Comprehensive test coverage for hook-configured behavior.

The new test context thoroughly validates the skip behavior:

• Verbose mode outputs the skip message
• Silent mode produces no output
• The rake task is not invoked

Excellent use of mocking to verify side effects.

---

47-48: Good practice: Explicit stub to maintain test isolation.

Adding the explicit stub to ensure the hook is not configured prevents unintended side effects on existing tests. This demonstrates good test hygiene.

lib/generators/react_on_rails/templates/base/base/bin/shakapacker-precompile-hook (1)

1-30: Verification complete: Rainbow gem is properly declared as a dependency.

The Rainbow gem is declared as a dependency in react_on_rails.gemspec with version constraint "~> 3.0", confirming it will be available when the precompile hook script executes. The script is well-structured with clear documentation, proper environment variable configuration, correct require_relative path, and robust error handling with colored output.

lib/react_on_rails/dev/server_manager.rb (2)

260-260: LGTM! Help text accurately reflects the precompile hook mechanism.

The updated documentation clearly explains how pack generation works in different modes (via precompile hook or bin/dev for development modes, via precompile hook or assets:precompile for production mode).

Also applies to: 268-268

---

417-433: LGTM! Conditional pack generation message is correctly implemented.

The logic correctly shows "Generating React on Rails packs" only when the precompile hook is not configured, since the hook handles pack generation automatically during shakapacker compilation. The feature descriptions are clear and informative.

[justin808]

@claude review this PR

[claude]

Claude Code is working…

I'll analyze this and get back to you.

View job run

[justin808]

@claude review this PR

[claude]

Claude finished @justin808's task —— View job

---

PR Review Complete ✅

I've completed a comprehensive review of PR #1916: Add Shakapacker precompile hook template for generator.

---

📋 Summary

This PR successfully adds a Shakapacker precompile hook template that enables automatic pack generation for newly generated React on Rails projects. The implementation is well-structured and addresses an important developer experience improvement.

Overall Assessment: ✅ Ready to merge - The PR is well-implemented with proper test coverage and addresses previous review issues effectively.

---

✅ Strengths

1. Comprehensive Test Coverage

• ✅ New tests added for shakapacker_precompile_hook_configured? method covering all scenarios (lines 143-189 in packer_utils_spec.rb)
• ✅ Pack generator tests updated to verify skip behavior when hook is configured (lines 6-34 in pack_generator_spec.rb)
• ✅ Tests cover edge cases: nil values, empty strings, missing Shakapacker, and error handling

2. Previous Review Issues Addressed

The commits show excellent response to previous reviews:

• ✅ 6f51900: Fixed precompile_hook detection to use top-level config key (not nested under hooks)
• ✅ 57ddb2c: Corrected misleading security comment in shakapacker.yml
• ✅ a625fcc: Removed duplicate precompile_hook key
• ✅ d329e2a: Added SKIP_VALIDATION to prevent generator failures
• ✅ b92258f: Added execute permissions to the hook template

3. Clean Architecture

• Proper separation: Template hook (30 lines, simple) vs. dummy app hook (100+ lines, complex with ReScript support)
• Smart detection logic: shakapacker_precompile_hook_configured? prevents duplicate pack generation
• Fail-safe defaults: Returns false on errors to ensure packs still generate rather than being incorrectly skipped
• Well-integrated: Works seamlessly with PackGenerator and ServerManager

4. Error Handling

• ✅ Robust rescue block in shakapacker_precompile_hook_configured? (line 185-191 in packer_utils.rb)
• ✅ Proper exit codes and colored error output in the hook script
• ✅ Limited backtrace (5 lines) for better error readability
• ✅ Debug mode support for verbose error reporting

5. Documentation & Code Quality

• ✅ Clear inline comments explaining the hook's purpose
• ✅ Emoji scheme documented in the hook script
• ✅ Security note in shakapacker.yml (properly qualified after fix in commit 57ddb2c)
• ✅ Follows CLAUDE.md conventions (trailing newlines, auto-formatting)

6. File Permissions Handling

• ✅ chmod 0o755 correctly applied in generator (lib/generators/react_on_rails/base_generator.rb:49)
• ✅ Uses absolute path with destination_root for reliability
• ✅ Comment confirms file existence is guaranteed by copy_file

---

🔍 Code Analysis

Key Implementation Details

1. Hook Detection Logic (lib/react_on_rails/packer_utils.rb:178-192)

def self.shakapacker_precompile_hook_configured?
  return false unless defined?(::Shakapacker)
  
  hook_value = extract_precompile_hook
  return false if hook_value.nil?
  
  hook_contains_generate_packs?(hook_value)
rescue StandardError => e
  warn "Warning: Unable to detect..." if ENV["DEBUG"]
  false  # Fail-safe: prefer running packs over skipping
end

Analysis:

• ✅ Correctly checks for both symbol and string keys (line 200)
• ✅ Proper fail-safe behavior on errors
• ✅ Uses private send(:data) method with clear documentation explaining why
• ⚠️ Note: This accesses Shakapacker internals, but error handling mitigates brittleness

2. Skip Logic Integration (lib/react_on_rails/dev/pack_generator.rb:29-36)

if ReactOnRails::PackerUtils.shakapacker_precompile_hook_configured?
  if verbose
    hook_value = ReactOnRails::PackerUtils.shakapacker_precompile_hook_value
    puts "⏭️ Skipping pack generation (handled by shakapacker...)"
  end
  return
end

Analysis:

• ✅ Clean early-exit pattern
• ✅ Provides feedback in verbose mode
• ✅ Silent in quiet mode (good UX)
• ✅ Also integrated in configuration.rb (lines 242-247)

3. Template Hook Script (lib/generators/react_on_rails/templates/base/base/bin/shakapacker-precompile-hook)

ENV["REACT_ON_RAILS_SKIP_VALIDATION"] = "true"
require_relative "../config/environment"

begin
  puts Rainbow("🔄 Running React on Rails precompile hook...").cyan
  ReactOnRails::PacksGenerator.instance.generate_packs_if_stale
rescue StandardError => e
  warn Rainbow("❌ Error in precompile hook: #{e.message}").red
  warn e.backtrace.first(5).join("\n")
  exit 1
end

Analysis:

• ✅ SKIP_VALIDATION: Smart addition (commit d329e2a) - prevents validation failures during build
• ✅ Uses require_relative for Rails environment loading
• ✅ Rainbow gem already declared as dependency in gemspec
• ✅ Proper error handling with exit code 1
• ℹ️ Design choice: Simpler than dummy app hook (no find_rails_root, no ReScript) - this is intentional for generated projects

---

💡 Minor Observations (No Action Required)

1. Template vs. Dummy Hook Differences

The template hook (30 lines) is intentionally simpler than the dummy app hook (100+ lines):

• Template: Assumes require_relative "../config/environment" works from project root
• Dummy: Has find_rails_root for robust directory detection + ReScript support

Assessment: This is a reasonable design choice. The dummy app needs extra robustness for testing, while generated projects can use the simpler approach. If issues arise in production, the hook can be enhanced later.

2. Security Comment Qualification

After commit 57ddb2c, the security comment correctly states:

"SECURITY: Only reference trusted scripts within your project. Ensure the hook path points to a file within the project root that you control."

This is properly qualified - it's guidance for users, not a claim about validation. ✅ Good fix.

3. Accessing Shakapacker Internals

packer_utils.rb:197 uses send(:data) to access private config data:

config_data = ::Shakapacker.config.send(:data)

Assessment: While accessing private methods is fragile, the error handling (rescue StandardError) provides proper mitigation. If Shakapacker's internals change, the method fails safe (returns false → packs still generate).

4. CHANGELOG Not Updated

According to CLAUDE.md guidelines, user-visible features should be documented in CHANGELOG.md. This PR adds a new feature (precompile hook support for generated projects), but no CHANGELOG entry exists.

Recommendation: Consider adding a CHANGELOG entry:

#### [PR 1916](https://github.com/shakacode/react_on_rails/pull/1916) by [justin808](https://github.com/justin808)
- Added Shakapacker precompile hook template to React on Rails generator for automatic pack generation in new projects

However, this can be done in a follow-up or during release preparation.

---

🧪 Test Coverage Analysis

Excellent Coverage ✅

packer_utils_spec.rb (lines 143-189):

• ✅ Hook configured with generate_packs task → returns true
• ✅ Hook configured without generate_packs → returns false
• ✅ Hook nil → returns false
• ✅ Hook empty string → returns false
• ✅ Shakapacker not available → returns false
• ✅ Config access raises error → returns false

pack_generator_spec.rb (lines 6-34):

• ✅ Skip behavior in verbose mode (outputs message)
• ✅ Skip behavior in silent mode (no output)
• ✅ Rake task not invoked when hook configured

Refactoring Quality:
The test refactoring in pack_generator_spec.rb removed unnecessary duplication and improved organization. Tests are now cleaner with explicit hook mocking for non-hook scenarios (line 47-48).

---

🔒 Security Assessment

No Security Issues Found ✅

1. ✅ Script safety: Proper shebang and frozen_string_literal
2. ✅ No injection vectors: No shell interpolation or eval
3. ✅ Ruby native methods: Uses safe Ruby methods vs. shell commands
4. ✅ Exit codes: Properly propagated for CI/CD
5. ✅ Security documentation: Qualified guidance (not false claims)

The security comment advises users to only reference trusted scripts, which is appropriate guidance.

---

📝 Commit History Analysis

The PR shows excellent iterative improvement:

1. 55f039a: Initial implementation
2. b92258f: Added execute permissions (responding to feedback)
3. d329e2a: Fixed validation skip issue (discovered during testing)
4. b76be25: Disabled yarn caching to fix V8 crash
5. a625fcc: Removed duplicate YAML key (CodeRabbit feedback)
6. 57ddb2c: Fixed security comment (CodeRabbit feedback)
7. 6f51900: Fixed config key detection (critical bug fix)

This demonstrates responsive development and attention to review feedback.

---

🎯 Performance Impact

Positive Impact ✅

• Eliminates duplicate work: No redundant pack generation when hook is configured
• Lightweight checks: present? check on config hash is fast
• No regressions expected: Skip logic only adds early-return optimization

---

✅ Final Recommendation

APPROVE - Ready to Merge

This PR demonstrates:

• ✅ Complete test coverage for new functionality
• ✅ Responsive addressing of all previous review issues
• ✅ Clean architecture with proper separation of concerns
• ✅ Robust error handling with fail-safe defaults
• ✅ Good documentation and code quality
• ✅ No security concerns
• ✅ Follows repository conventions (CLAUDE.md)

Optional Follow-up Items (Not Blockers)

1. CHANGELOG entry: Consider adding when convenient (can be done during release prep)
2. Monitor Shakapacker internals: If send(:data) breaks in future Shakapacker versions, add public API request

---

🚀 Testing Suggestions for Merge

Before merging, verify:

1. ✅ All CI checks pass (examples, rspec tests, build)
2. ✅ RuboCop passes with zero violations (per CLAUDE.md)
3. ✅ Generator creates executable hook with correct permissions
4. ✅ Hook works during assets:precompile in fresh generated project

---

Great work on this PR! The implementation is solid, well-tested, and significantly improves the developer experience for new React on Rails projects. The responsive iteration based on review feedback demonstrates excellent attention to detail. 🎉