Add Shakapacker precompile hook support for auto pack generation #1878
Conversation
This change 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. Changes: - Add bin/shakapacker-precompile-hook script to templates and spec/dummy - Update shakapacker.yml to configure precompile_hook - Add PackerUtils.shakapacker_precompile_hook_configured? method - Skip generate_packs in configuration and PackGenerator when hook configured - Update generator to copy hook script and make it executable The precompile hook runs before webpack compilation and is properly validated by Shakapacker to ensure it points to a file within the project root. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughThis PR introduces a Shakapacker precompile hook mechanism to auto-generate React on Rails component packs before webpack compilation. A new executable bin/shakapacker-precompile-hook script runs during the precompile phase, with conditional logic to prevent duplicate generation when the hook is configured. Changes
Sequence DiagramsequenceDiagram
autonumber
actor User
participant Precompile as Precompile Task
participant PG as PackGenerator
participant Utils as PackerUtils
participant Hook as Hook Script
participant Webpack
User->>Precompile: rake assets:precompile
Precompile->>Utils: shakapacker_precompile_hook_configured?
rect rgb(200, 220, 255)
Note over Utils: Check if hook exists
alt Hook configured
Utils-->>Precompile: true
Precompile->>PG: generate (skipped early)
PG-->>Precompile: (returns early)
Precompile->>Hook: (hook runs separately)
Hook->>Hook: generate_packs_if_stale
else Hook not configured
Utils-->>Precompile: false
Precompile->>PG: generate
PG->>PG: generate_packs
end
end
Precompile->>Webpack: assets:webpack
Webpack-->>User: ✓ Complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes The changes introduce a new hook mechanism across multiple file types (scripts, config, utilities, and control flow), requiring understanding of the precompile pipeline and hook integration. While individual edits are straightforward conditionals and configuration, the distributed nature and interaction between multiple components warrants moderate review effort. Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
lib/react_on_rails/packer_utils.rb (1)
169-181: Consider coupling risk with internal Shakapacker API.The method uses
send(:data)to access a private method onShakapacker.config, which couples to Shakapacker's internal implementation. While the broadStandardErrorrescue provides safety, this approach may break if Shakapacker refactors its internals.Consider checking if Shakapacker exposes a public API for accessing configuration values, or document this as a known coupling point that requires monitoring across Shakapacker version upgrades. Alternatively, you could add a comment explaining the rationale for using the private API.
spec/dummy/bin/shakapacker-precompile-hook (1)
16-19: Consider showing more backtrace lines for debugging.The error handler currently displays only the first 5 lines of the backtrace. For complex errors or deep call stacks during precompilation, this may be insufficient for debugging. Consider increasing this to 10-15 lines or making it configurable via an environment variable.
Apply this diff:
- warn e.backtrace.first(5).join("\n") + warn e.backtrace.first(10).join("\n")lib/generators/react_on_rails/templates/base/base/bin/shakapacker-precompile-hook (1)
16-19: Consider showing more backtrace lines for debugging.The error handler currently displays only the first 5 lines of the backtrace. For complex errors or deep call stacks during precompilation, this may be insufficient for debugging. Consider increasing this to 10-15 lines or making it configurable via an environment variable.
Apply this diff:
- warn e.backtrace.first(5).join("\n") + warn e.backtrace.first(10).join("\n")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
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/packer_utils.rb(1 hunks)spec/dummy/bin/shakapacker-precompile-hook(1 hunks)spec/dummy/config/shakapacker.yml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
{Gemfile,Rakefile,config.ru,**/*.{rb,rake,gemspec,ru}}
📄 CodeRabbit inference engine (CLAUDE.md)
{Gemfile,Rakefile,config.ru,**/*.{rb,rake,gemspec,ru}}: All Ruby code must pass RuboCop with zero offenses before commit/push
RuboCop is the sole authority for Ruby file formatting; never manually format Ruby files
Files:
lib/react_on_rails/configuration.rblib/react_on_rails/packer_utils.rblib/react_on_rails/dev/pack_generator.rblib/generators/react_on_rails/base_generator.rb
**/*.{js,jsx,ts,tsx,css,scss,json,yml,yaml,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Prettier is the sole authority for formatting all non-Ruby files; never manually format them
Files:
lib/generators/react_on_rails/templates/base/base/config/shakapacker.ymlspec/dummy/config/shakapacker.yml
🧬 Code graph analysis (3)
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/packer_utils.rb (1)
lib/react_on_rails/engine.rb (1)
config(6-17)
lib/react_on_rails/dev/pack_generator.rb (1)
lib/react_on_rails/packer_utils.rb (1)
shakapacker_precompile_hook_configured?(172-181)
🪛 GitHub Actions: Lint JS and Ruby
lib/generators/react_on_rails/templates/base/base/bin/shakapacker-precompile-hook
[warning] 1-1: Lint/ScriptPermission: Script file shakapacker-precompile-hook doesn't have execute permission.
⏰ 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). (12)
- GitHub Check: dummy-app-integration-tests (3.2, 20)
- GitHub Check: dummy-app-integration-tests (3.4, 22)
- GitHub Check: examples (3.2, minimum)
- GitHub Check: examples (3.4, latest)
- GitHub Check: rspec-package-tests (3.2, minimum)
- GitHub Check: build-dummy-app-webpack-test-bundles
- GitHub Check: rspec-package-tests (3.4, minimum)
- GitHub Check: rspec-package-tests (3.4, latest)
- GitHub Check: rspec-package-tests (3.2, latest)
- GitHub Check: lint-js-and-ruby
- GitHub Check: build-dummy-app-webpack-test-bundles
- GitHub Check: claude-review
🔇 Additional comments (8)
spec/dummy/bin/shakapacker-precompile-hook (1)
1-20: Pipeline warning about script permissions is expected.The RuboCop warning about missing execute permission is expected for this file in the spec/dummy directory. The generator correctly handles this by running
File.chmod(0o755, "bin/shakapacker-precompile-hook")inlib/generators/react_on_rails/base_generator.rb(line 49) after copying the file.lib/generators/react_on_rails/base_generator.rb (2)
40-41: LGTM!Adding the Shakapacker precompile hook to the base files list is correct. The hook script will be copied during generator execution.
47-49: LGTM!The file permission handling is correctly implemented. Using
chmod 0o755after copying the file ensures the hook script is executable, and the existence check provides appropriate safety.lib/react_on_rails/dev/pack_generator.rb (1)
10-14: LGTM!The preflight check correctly skips pack generation when the Shakapacker precompile hook is configured, preventing duplicate generation. The conditional verbose message and early return pattern are clean and appropriate.
lib/generators/react_on_rails/templates/base/base/config/shakapacker.yml (1)
45-48: LGTM!The precompile hook configuration is well-documented with appropriate security warnings. The placement among default settings is logical, and the default value correctly points to the hook script added by the generator.
lib/generators/react_on_rails/templates/base/base/bin/shakapacker-precompile-hook (1)
1-20: Well-structured hook script with clear documentation.The script structure is sound with appropriate error handling and informative output messages. The inline documentation clearly explains the hook's purpose and configuration requirements.
spec/dummy/config/shakapacker.yml (1)
20-23: LGTM!The precompile hook configuration in the spec/dummy setup correctly mirrors the template configuration, with appropriate security warnings and documentation. This ensures the hook mechanism is tested in the dummy application.
lib/react_on_rails/configuration.rb (1)
241-244: LGTM!The conditional gating of
generate_packscorrectly prevents duplicate pack generation when the Shakapacker precompile hook is configured. The comment clearly explains the behavior, and the integration with the existing precompile task flow is sound.
1 participant
Summary
This PR leverages Shakapacker's new
precompile_hookfeature to automatically generate packs before webpack compilation, replacing the need to manually modify assets:precompile tasks or bin/dev scripts.Changes
bin/shakapacker-precompile-hookscript to generator templates and spec/dummyshakapacker.ymlto configureprecompile_hook: 'bin/shakapacker-precompile-hook'PackerUtils.shakapacker_precompile_hook_configured?method to detect when hook is configuredconfiguration.rbto skip generate_packs when hook is configuredPackGeneratorto skip pack generation when hook is configuredBenefits
Test plan
🤖 Generated with Claude Code
This change is
Summary by CodeRabbit