Skip to content

Commit f524366

Browse files
justin808claude
andcommitted
Support script file hooks and display hook values in logs
This addresses the remaining code review feedback: 1. Display hook value in log messages - Show actual hook path/command when skipping generate_packs - Helps users understand what's configured 2. Support script file hook detection - Check if hook is a direct command (e.g., "bundle exec rake...") - Or if it's a script file path (e.g., "bin/precompile_hook") - Read script file contents to detect generate_packs task 3. Add shakapacker_precompile_hook_value method - Returns the configured hook value for logging/debugging - Returns nil if no hook is configured or on error 4. Document future enhancement - Note that hooks will support lists in the future - Allow prepending/appending multiple commands - Currently only single hook value is supported 5. Comprehensive test coverage - Test direct command detection - Test script file detection and reading - Test hook value retrieval - All 40 examples passing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent eabd2ff commit f524366

File tree

5 files changed

+116
-17
lines changed

5 files changed

+116
-17
lines changed

lib/react_on_rails/configuration.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,8 @@ def adjust_precompile_task
240240
precompile_tasks = lambda {
241241
# Skip generate_packs if shakapacker has a precompile hook configured
242242
if ReactOnRails::PackerUtils.shakapacker_precompile_hook_configured?
243-
puts "Skipping react_on_rails:generate_packs (configured in shakapacker precompile hook)"
243+
hook_value = ReactOnRails::PackerUtils.shakapacker_precompile_hook_value
244+
puts "Skipping react_on_rails:generate_packs (configured in shakapacker precompile hook: #{hook_value})"
244245
else
245246
Rake::Task["react_on_rails:generate_packs"].invoke
246247
end

lib/react_on_rails/dev/pack_generator.rb

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,10 @@ class << self
2828
def generate(verbose: false)
2929
# Skip if shakapacker has a precompile hook configured
3030
if ReactOnRails::PackerUtils.shakapacker_precompile_hook_configured?
31-
puts "⏭️ Skipping pack generation (handled by shakapacker precompile hook)" if verbose
31+
if verbose
32+
hook_value = ReactOnRails::PackerUtils.shakapacker_precompile_hook_value
33+
puts "⏭️ Skipping pack generation (handled by shakapacker precompile hook: #{hook_value})"
34+
end
3235
return
3336
end
3437

lib/react_on_rails/packer_utils.rb

Lines changed: 45 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -172,13 +172,16 @@ def self.raise_shakapacker_version_incompatible_for_basic_pack_generation
172172
#
173173
# Returns false if detection fails for any reason (missing shakapacker, malformed config, etc.)
174174
# to ensure generate_packs runs rather than being incorrectly skipped
175+
#
176+
# Note: Currently checks a single hook value. Future enhancement will support hook lists
177+
# to allow prepending/appending multiple commands. See related Shakapacker issue for details.
175178
def self.shakapacker_precompile_hook_configured?
176179
return false unless defined?(::Shakapacker)
177180

178-
hooks = extract_precompile_hooks
179-
return false if hooks.nil?
181+
hook_value = extract_precompile_hook
182+
return false if hook_value.nil?
180183

181-
hook_contains_generate_packs?(hooks)
184+
hook_contains_generate_packs?(hook_value)
182185
rescue StandardError => e
183186
# Swallow errors during hook detection to fail safe - if we can't detect the hook,
184187
# we should run generate_packs rather than skip it incorrectly.
@@ -188,19 +191,53 @@ def self.shakapacker_precompile_hook_configured?
188191
false
189192
end
190193

191-
def self.extract_precompile_hooks
194+
def self.extract_precompile_hook
192195
# Access config data using private :data method since there's no public API
193196
# to access the raw configuration hash needed for hook detection
194197
config_data = ::Shakapacker.config.send(:data)
195198

196199
# Try symbol keys first (Shakapacker's internal format), then fall back to string keys
200+
# Note: Currently only one hook value is supported, but this will change to support lists
197201
config_data&.dig(:hooks, :precompile) || config_data&.dig("hooks", "precompile")
198202
end
199203

200-
def self.hook_contains_generate_packs?(hooks)
201-
# Check if any hook contains the generate_packs rake task using word boundary
202-
# to avoid false positives from comments or similar strings
203-
Array(hooks).any? { |hook| hook.to_s.match?(/\breact_on_rails:generate_packs\b/) }
204+
def self.hook_contains_generate_packs?(hook_value)
205+
# The hook value can be either:
206+
# 1. A direct command containing the rake task
207+
# 2. A path to a script file that needs to be read
208+
return false if hook_value.blank?
209+
210+
# Check if it's a direct command first
211+
return true if hook_value.to_s.match?(/\breact_on_rails:generate_packs\b/)
212+
213+
# Check if it's a script file path
214+
script_path = resolve_hook_script_path(hook_value)
215+
return false unless script_path && File.exist?(script_path)
216+
217+
# Read and check script contents
218+
script_contents = File.read(script_path)
219+
script_contents.match?(/\breact_on_rails:generate_packs\b/)
220+
rescue StandardError
221+
# If we can't read the script, assume it doesn't contain generate_packs
222+
false
223+
end
224+
225+
def self.resolve_hook_script_path(hook_value)
226+
# Hook value might be a script path relative to Rails root
227+
return nil unless defined?(Rails) && Rails.respond_to?(:root)
228+
229+
potential_path = Rails.root.join(hook_value.to_s.strip)
230+
potential_path if potential_path.file?
231+
end
232+
233+
# Returns the configured precompile hook value for logging/debugging
234+
# Returns nil if no hook is configured
235+
def self.shakapacker_precompile_hook_value
236+
return nil unless defined?(::Shakapacker)
237+
238+
extract_precompile_hook
239+
rescue StandardError
240+
nil
204241
end
205242
end
206243
end

spec/react_on_rails/dev/pack_generator_spec.rb

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,13 @@
2525

2626
context "when shakapacker precompile hook is configured" do
2727
before do
28-
allow(ReactOnRails::PackerUtils).to receive(:shakapacker_precompile_hook_configured?).and_return(true)
28+
allow(ReactOnRails::PackerUtils).to receive_messages(shakapacker_precompile_hook_configured?: true,
29+
shakapacker_precompile_hook_value: "bin/precompile_hook")
2930
end
3031

31-
it "skips pack generation in verbose mode" do
32+
it "skips pack generation in verbose mode and shows hook value" do
3233
expect { described_class.generate(verbose: true) }
33-
.to output(/⏭️ Skipping pack generation \(handled by shakapacker precompile hook\)/)
34+
.to output(%r{⏭️ Skipping pack generation \(handled by shakapacker precompile hook: bin/precompile_hook\)})
3435
.to_stdout_from_any_process
3536
end
3637

spec/react_on_rails/packer_utils_spec.rb

Lines changed: 61 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -159,29 +159,57 @@ module ReactOnRails
159159
end
160160

161161
context "when precompile hook contains react_on_rails:generate_packs" do
162-
it "returns true for single hook with symbol keys" do
162+
it "returns true for direct command with symbol keys" do
163163
allow(mock_config).to receive(:send).with(:data).and_return(
164164
{ hooks: { precompile: "bundle exec rake react_on_rails:generate_packs" } }
165165
)
166166

167167
expect(described_class.shakapacker_precompile_hook_configured?).to be(true)
168168
end
169169

170-
it "returns true for single hook with string keys" do
170+
it "returns true for direct command with string keys" do
171171
allow(mock_config).to receive(:send).with(:data).and_return(
172172
{ "hooks" => { "precompile" => "bundle exec rake react_on_rails:generate_packs" } }
173173
)
174174

175175
expect(described_class.shakapacker_precompile_hook_configured?).to be(true)
176176
end
177177

178-
it "returns true for hook in array" do
178+
it "returns true when hook points to script file containing the task" do
179179
allow(mock_config).to receive(:send).with(:data).and_return(
180-
{ hooks: { precompile: ["bundle exec rake react_on_rails:generate_packs", "echo done"] } }
180+
{ hooks: { precompile: "bin/shakapacker_precompile" } }
181181
)
182182

183+
# Mock Rails.root
184+
rails_root = instance_double(Pathname)
185+
allow(Rails).to receive(:root).and_return(rails_root)
186+
187+
script_path = instance_double(Pathname, file?: true)
188+
allow(rails_root).to receive(:join).with("bin/shakapacker_precompile").and_return(script_path)
189+
allow(File).to receive(:exist?).with(script_path).and_return(true)
190+
allow(File).to receive(:read).with(script_path)
191+
.and_return("#!/bin/bash\nbundle exec rake react_on_rails:generate_packs\n")
192+
183193
expect(described_class.shakapacker_precompile_hook_configured?).to be(true)
184194
end
195+
196+
it "returns false when hook points to script file without the task" do
197+
allow(mock_config).to receive(:send).with(:data).and_return(
198+
{ hooks: { precompile: "bin/other_script" } }
199+
)
200+
201+
# Mock Rails.root
202+
rails_root = instance_double(Pathname)
203+
allow(Rails).to receive(:root).and_return(rails_root)
204+
205+
script_path = instance_double(Pathname, file?: true)
206+
allow(rails_root).to receive(:join).with("bin/other_script").and_return(script_path)
207+
allow(File).to receive(:exist?).with(script_path).and_return(true)
208+
allow(File).to receive(:read).with(script_path)
209+
.and_return("#!/bin/bash\necho 'doing other stuff'\n")
210+
211+
expect(described_class.shakapacker_precompile_hook_configured?).to be(false)
212+
end
185213
end
186214

187215
context "when precompile hook does not contain react_on_rails:generate_packs" do
@@ -222,6 +250,35 @@ module ReactOnRails
222250
end
223251
end
224252
end
253+
254+
describe ".shakapacker_precompile_hook_value" do
255+
let(:mock_config) { instance_double("::Shakapacker::Config") } # rubocop:disable RSpec/VerifiedDoubleReference
256+
257+
before do
258+
allow(::Shakapacker).to receive(:config).and_return(mock_config)
259+
end
260+
261+
it "returns the hook value when configured" do
262+
hook_value = "bin/shakapacker_precompile"
263+
allow(mock_config).to receive(:send).with(:data).and_return(
264+
{ hooks: { precompile: hook_value } }
265+
)
266+
267+
expect(described_class.shakapacker_precompile_hook_value).to eq(hook_value)
268+
end
269+
270+
it "returns nil when no hook is configured" do
271+
allow(mock_config).to receive(:send).with(:data).and_return({})
272+
273+
expect(described_class.shakapacker_precompile_hook_value).to be_nil
274+
end
275+
276+
it "returns nil when an error occurs" do
277+
allow(mock_config).to receive(:send).with(:data).and_raise(StandardError.new("test error"))
278+
279+
expect(described_class.shakapacker_precompile_hook_value).to be_nil
280+
end
281+
end
225282
end
226283

227284
describe "version constants validation" do

0 commit comments

Comments
 (0)