Skip to content

Commit a3c3391

Browse files
justin808claude
andcommitted
Fix Shakapacker version requirement from 8.2.0 to 8.0.0 for basic compatibility
This change separates basic React on Rails functionality from feature-specific requirements to eliminate unnecessary breaking changes. ## Problem React on Rails 16 artificially required Shakapacker >= 8.2.0 for ALL functionality, causing the `:clean` task error when using Shakapacker 8.0.0. The 8.2.0 requirement is only needed for async script loading, not basic compatibility. ## Solution 1. **Reduced basic compatibility requirement** from 8.2.0 to 8.0.0 in `using_shakapacker_const?` 2. **Added feature-specific methods**: - `supports_async_loading?` - checks Shakapacker >= 8.2.0 for async script loading - `supports_auto_registration?` - checks Shakapacker >= 7.0.0 + nested_entries support 3. **Updated configuration logic** to use feature-specific detection 4. **Updated system checker** to use proper feature detection methods ## Impact - ✅ Fixes Docker build failure: `Don't know how to build task ':clean'` - ✅ Allows React on Rails 16 to work with Shakapacker 8.0.0 for basic functionality - ✅ Shows helpful warnings instead of fatal errors for missing features - ✅ Maintains backward compatibility while enabling gradual upgrades ## Testing - All existing tests pass - Added comprehensive test coverage for new methods - Verified assets:precompile works with Shakapacker 8.0.0 - Confirmed proper `shakapacker:clean` task invocation Fixes the artificial breaking change that forced unnecessary Shakapacker upgrades. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 32d4665 commit a3c3391

File tree

4 files changed

+100
-6
lines changed

4 files changed

+100
-6
lines changed

lib/react_on_rails/configuration.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ def validate_generated_component_packs_loading_strategy
180180
1. Use :sync or :defer loading strategy instead of :async
181181
2. Upgrade to Shakapacker v8.2.0 or above to enable async script loading
182182
MSG
183-
if PackerUtils.shakapacker_version_requirement_met?("8.2.0")
183+
if PackerUtils.supports_async_loading?
184184
self.generated_component_packs_loading_strategy ||= :async
185185
elsif generated_component_packs_loading_strategy.nil?
186186
Rails.logger.warn("**WARNING** #{msg}")

lib/react_on_rails/packer_utils.rb

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ def self.using_shakapacker_const?
1010
return @using_shakapacker_const if defined?(@using_shakapacker_const)
1111

1212
@using_shakapacker_const = ReactOnRails::Utils.gem_available?("shakapacker") &&
13-
shakapacker_version_requirement_met?("8.2.0")
13+
shakapacker_version_requirement_met?("8.0.0")
1414
end
1515

1616
def self.packer_type
@@ -56,6 +56,16 @@ def self.shakapacker_version_requirement_met?(required_version)
5656
Gem::Version.new(shakapacker_version) >= Gem::Version.new(required_version)
5757
end
5858

59+
def self.supports_async_loading?
60+
using_shakapacker_const? && shakapacker_version_requirement_met?("8.2.0")
61+
end
62+
63+
def self.supports_auto_registration?
64+
using_shakapacker_const? &&
65+
packer.config.respond_to?(:nested_entries?) &&
66+
shakapacker_version_requirement_met?("7.0.0")
67+
end
68+
5969
# This returns either a URL for the webpack-dev-server, non-server bundle or
6070
# the hashed server bundle if using the same bundle for the client.
6171
# Otherwise returns a file path.

lib/react_on_rails/system_checker.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -619,13 +619,13 @@ def report_shakapacker_version_with_threshold
619619

620620
begin
621621
# Use proper semantic version comparison
622-
version_obj = Gem::Version.new(version)
623-
threshold_version = Gem::Version.new("8.2")
622+
Gem::Version.new(version)
623+
Gem::Version.new("8.2")
624624

625-
if version_obj >= threshold_version
625+
if ReactOnRails::PackerUtils.supports_auto_registration?
626626
add_success("✅ Shakapacker #{version} (supports React on Rails auto-registration)")
627627
else
628-
add_warning("⚠️ Shakapacker #{version} - Version 8.2+ needed for React on Rails auto-registration")
628+
add_warning("⚠️ Shakapacker #{version} - Version 7.0+ needed for React on Rails auto-registration")
629629
end
630630
rescue ArgumentError
631631
# Fallback for invalid version strings

spec/react_on_rails/packer_utils_spec.rb

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,5 +76,89 @@ module ReactOnRails
7676
end
7777
end
7878
end
79+
80+
describe ".supports_async_loading?" do
81+
it "returns true when using Shakapacker >= 8.2.0" do
82+
allow(described_class).to receive(:using_shakapacker_const?).and_return(true)
83+
allow(described_class).to receive(:shakapacker_version_requirement_met?).with("8.2.0").and_return(true)
84+
85+
expect(described_class.supports_async_loading?).to be(true)
86+
end
87+
88+
it "returns false when using Shakapacker < 8.2.0" do
89+
allow(described_class).to receive(:using_shakapacker_const?).and_return(true)
90+
allow(described_class).to receive(:shakapacker_version_requirement_met?).with("8.2.0").and_return(false)
91+
92+
expect(described_class.supports_async_loading?).to be(false)
93+
end
94+
95+
it "returns false when not using Shakapacker" do
96+
allow(described_class).to receive(:using_shakapacker_const?).and_return(false)
97+
98+
expect(described_class.supports_async_loading?).to be(false)
99+
end
100+
end
101+
102+
describe ".supports_auto_registration?" do
103+
let(:mock_config) { instance_double(Config) }
104+
let(:mock_packer) { instance_double(Packer, config: mock_config) }
105+
106+
before do
107+
allow(described_class).to receive(:packer).and_return(mock_packer)
108+
end
109+
110+
it "returns true when using Shakapacker >= 7.0.0 with nested_entries support" do
111+
allow(described_class).to receive(:using_shakapacker_const?).and_return(true)
112+
allow(mock_config).to receive(:respond_to?).with(:nested_entries?).and_return(true)
113+
allow(described_class).to receive(:shakapacker_version_requirement_met?).with("7.0.0").and_return(true)
114+
115+
expect(described_class.supports_auto_registration?).to be(true)
116+
end
117+
118+
it "returns false when using Shakapacker < 7.0.0" do
119+
allow(described_class).to receive(:using_shakapacker_const?).and_return(true)
120+
allow(mock_config).to receive(:respond_to?).with(:nested_entries?).and_return(true)
121+
allow(described_class).to receive(:shakapacker_version_requirement_met?).with("7.0.0").and_return(false)
122+
123+
expect(described_class.supports_auto_registration?).to be(false)
124+
end
125+
126+
it "returns false when nested_entries method is not available" do
127+
allow(described_class).to receive(:using_shakapacker_const?).and_return(true)
128+
allow(mock_config).to receive(:respond_to?).with(:nested_entries?).and_return(false)
129+
allow(described_class).to receive(:shakapacker_version_requirement_met?).with("7.0.0").and_return(true)
130+
131+
expect(described_class.supports_auto_registration?).to be(false)
132+
end
133+
134+
it "returns false when not using Shakapacker" do
135+
allow(described_class).to receive(:using_shakapacker_const?).and_return(false)
136+
137+
expect(described_class.supports_auto_registration?).to be(false)
138+
end
139+
end
140+
141+
describe ".using_shakapacker_const?" do
142+
before do
143+
# Clear memoized value to ensure tests are isolated
144+
if described_class.instance_variable_defined?(:@using_shakapacker_const)
145+
described_class.remove_instance_variable(:@using_shakapacker_const)
146+
end
147+
end
148+
149+
it "requires Shakapacker >= 8.0.0 instead of 8.2.0" do
150+
allow(ReactOnRails::Utils).to receive(:gem_available?).with("shakapacker").and_return(true)
151+
allow(described_class).to receive(:shakapacker_version_requirement_met?).with("8.0.0").and_return(true)
152+
153+
expect(described_class.using_shakapacker_const?).to be(true)
154+
end
155+
156+
it "returns false when Shakapacker < 8.0.0" do
157+
allow(ReactOnRails::Utils).to receive(:gem_available?).with("shakapacker").and_return(true)
158+
allow(described_class).to receive(:shakapacker_version_requirement_met?).with("8.0.0").and_return(false)
159+
160+
expect(described_class.using_shakapacker_const?).to be(false)
161+
end
162+
end
79163
end
80164
end

0 commit comments

Comments
 (0)