Skip to content

Commit 2ecec4a

Browse files
justin808claude
andcommitted
Fix shakapacker hook detection to support symbol keys and prevent false positives
Improvements: - Support both symbol and string keys in config data (shakapacker uses symbols internally) - Use word boundary regex to prevent matching similar task names - Add safe navigation operator for better nil handling - Document why private :data method is accessed - Add tests for both key types and false positive prevention This ensures the hook detection works correctly with shakapacker's actual config format. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent 922ebfb commit 2ecec4a

File tree

2 files changed

+31
-8
lines changed

2 files changed

+31
-8
lines changed

lib/react_on_rails/packer_utils.rb

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -172,13 +172,18 @@ def self.raise_shakapacker_version_incompatible_for_basic_pack_generation
172172
def self.shakapacker_precompile_hook_configured?
173173
return false unless defined?(::Shakapacker)
174174

175+
# Access config data using private :data method since there's no public API
176+
# to access the raw configuration hash needed for hook detection
175177
config_data = ::Shakapacker.config.send(:data)
176-
hooks = config_data.dig("hooks", "precompile")
177178

178-
return false unless hooks
179+
# Try symbol keys first (Shakapacker's internal format), then fall back to string keys
180+
hooks = config_data&.dig(:hooks, :precompile) || config_data&.dig("hooks", "precompile")
179181

180-
# Check if any hook contains the generate_packs rake task
181-
Array(hooks).any? { |hook| hook.to_s.include?("react_on_rails:generate_packs") }
182+
return false if hooks.nil?
183+
184+
# Check if any hook contains the generate_packs rake task using word boundary
185+
# to avoid false positives from comments or similar strings
186+
Array(hooks).any? { |hook| hook.to_s.match?(/\breact_on_rails:generate_packs\b/) }
182187
rescue StandardError
183188
false
184189
end

spec/react_on_rails/packer_utils_spec.rb

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
require_relative "spec_helper"
44

5+
# rubocop:disable Metrics/ModuleLength
56
module ReactOnRails
67
describe PackerUtils do
78
describe ".shakapacker_version_requirement_met?" do
@@ -158,7 +159,15 @@ module ReactOnRails
158159
end
159160

160161
context "when precompile hook contains react_on_rails:generate_packs" do
161-
it "returns true for single hook" do
162+
it "returns true for single hook with symbol keys" do
163+
allow(mock_config).to receive(:send).with(:data).and_return(
164+
{ hooks: { precompile: "bundle exec rake react_on_rails:generate_packs" } }
165+
)
166+
167+
expect(described_class.shakapacker_precompile_hook_configured?).to be(true)
168+
end
169+
170+
it "returns true for single hook with string keys" do
162171
allow(mock_config).to receive(:send).with(:data).and_return(
163172
{ "hooks" => { "precompile" => "bundle exec rake react_on_rails:generate_packs" } }
164173
)
@@ -168,7 +177,7 @@ module ReactOnRails
168177

169178
it "returns true for hook in array" do
170179
allow(mock_config).to receive(:send).with(:data).and_return(
171-
{ "hooks" => { "precompile" => ["bundle exec rake react_on_rails:generate_packs", "echo done"] } }
180+
{ hooks: { precompile: ["bundle exec rake react_on_rails:generate_packs", "echo done"] } }
172181
)
173182

174183
expect(described_class.shakapacker_precompile_hook_configured?).to be(true)
@@ -178,7 +187,15 @@ module ReactOnRails
178187
context "when precompile hook does not contain react_on_rails:generate_packs" do
179188
it "returns false for different hook" do
180189
allow(mock_config).to receive(:send).with(:data).and_return(
181-
{ "hooks" => { "precompile" => "bundle exec rake some_other_task" } }
190+
{ hooks: { precompile: "bundle exec rake some_other_task" } }
191+
)
192+
193+
expect(described_class.shakapacker_precompile_hook_configured?).to be(false)
194+
end
195+
196+
it "returns false for similar but different task name" do
197+
allow(mock_config).to receive(:send).with(:data).and_return(
198+
{ hooks: { precompile: "bundle exec rake react_on_rails:generate_packs_extra" } }
182199
)
183200

184201
expect(described_class.shakapacker_precompile_hook_configured?).to be(false)
@@ -191,7 +208,7 @@ module ReactOnRails
191208
end
192209

193210
it "returns false when precompile hook is nil" do
194-
allow(mock_config).to receive(:send).with(:data).and_return({ "hooks" => {} })
211+
allow(mock_config).to receive(:send).with(:data).and_return({ hooks: {} })
195212

196213
expect(described_class.shakapacker_precompile_hook_configured?).to be(false)
197214
end
@@ -238,3 +255,4 @@ module ReactOnRails
238255
end
239256
end
240257
end
258+
# rubocop:enable Metrics/ModuleLength

0 commit comments

Comments
 (0)