Skip to content

Commit d1a8a1a

Browse files
authored
Fix component registration race condition with defer loading strategy (#1972)
## Problem CI tests were failing intermittently with component registration errors: This was caused by a race condition where React hydration started before component registrations completed. ## Root Cause With script loading (default on Shakapacker >= 8.2.0): - Both and generated component packs used - Scripts execute as soon as downloaded, in **any order** - When main bundle executed first, React couldn't find registered components ## Solution Changed default loading strategy to for deterministic execution order: 1. **Configuration** - Default to (was ) 2. **Layout** - Use for (was conditional async/defer) 3. **Result** - Scripts execute in DOM order after HTML parsing With : - All scripts download in parallel (fast) - Execute in DOM order after parsing - Generated component packs execute before main bundle - Component registrations complete before React hydration ## Testing Fixes CI failures in: - spec/system/integration_spec.rb[1:1:6:1:2] - spec/system/integration_spec.rb[1:1:6:2:2] ## Files Changed - `lib/react_on_rails/configuration.rb` - Changed default strategy to :defer - `spec/dummy/app/views/layouts/application.html.erb` - Always use defer for client-bundle - `spec/dummy/config/initializers/react_on_rails.rb` - Updated comments --- 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> <!-- Reviewable:start --> - - - This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/shakacode/react_on_rails/1972) <!-- Reviewable:end --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Fixed component registration timing to ensure completion before hydration * **Refactor** * Simplified component loading strategy to consistently use defer behavior * Removed automatic async loading detection for more predictable configuration <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 9d2710a commit d1a8a1a

File tree

4 files changed

+17
-24
lines changed

4 files changed

+17
-24
lines changed

lib/react_on_rails/configuration.rb

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -154,9 +154,9 @@ def check_component_registry_timeout
154154
raise ReactOnRails::Error, "component_registry_timeout must be a positive integer"
155155
end
156156

157-
# rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
157+
# rubocop:disable Metrics/CyclomaticComplexity
158158
def validate_generated_component_packs_loading_strategy
159-
# rubocop:enable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
159+
# rubocop:enable Metrics/CyclomaticComplexity
160160

161161
if defer_generated_component_packs
162162
if %i[async sync].include?(generated_component_packs_loading_strategy)
@@ -176,12 +176,11 @@ def validate_generated_component_packs_loading_strategy
176176
1. Use :sync or :defer loading strategy instead of :async
177177
2. Upgrade to Shakapacker v8.2.0 or above to enable async script loading
178178
MSG
179-
if PackerUtils.supports_async_loading?
180-
self.generated_component_packs_loading_strategy ||= :async
181-
elsif generated_component_packs_loading_strategy.nil?
182-
Rails.logger.warn("**WARNING** #{msg}")
183-
self.generated_component_packs_loading_strategy = :sync
184-
elsif generated_component_packs_loading_strategy == :async
179+
if generated_component_packs_loading_strategy.nil?
180+
# Use defer as the default to ensure generated component packs load and register
181+
# components before main bundle executes, avoiding race conditions with async loading
182+
self.generated_component_packs_loading_strategy = :defer
183+
elsif generated_component_packs_loading_strategy == :async && !PackerUtils.supports_async_loading?
185184
raise ReactOnRails::Error, "**ERROR** #{msg}"
186185
end
187186

spec/dummy/app/views/layouts/application.html.erb

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,10 @@
99

1010
<%= yield :head %>
1111

12-
<%# Conditionally use defer: true for pages with Redux shared stores (inline registration).
13-
Modern apps should use async: true for optimal performance. See docs for details:
14-
docs/building-features/streaming-server-rendering.md %>
15-
<% if uses_redux_shared_store? %>
16-
<%# defer: true required for Redux shared stores with inline component registration %>
17-
<%= javascript_pack_tag('client-bundle', 'data-turbo-track': 'reload', defer: true) %>
18-
<% else %>
19-
<%# async: true is the recommended approach for modern apps (Shakapacker >= 8.2.0) %>
20-
<%= javascript_pack_tag('client-bundle', 'data-turbo-track': 'reload', async: true) %>
21-
<% end %>
12+
<%# Use defer: true to ensure proper script execution order.
13+
When using generated component packs (auto_load_bundle), defer ensures
14+
component registrations complete before React hydration begins. %>
15+
<%= javascript_pack_tag('client-bundle', 'data-turbo-track': 'reload', defer: true) %>
2216

2317
<%= csrf_meta_tags %>
2418
</head>

spec/dummy/config/initializers/react_on_rails.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,5 +42,6 @@ def self.adjust_props_for_client_side_hydration(component_name, props)
4242
config.components_subdirectory = "startup"
4343
config.auto_load_bundle = true
4444
config.immediate_hydration = false
45-
config.generated_component_packs_loading_strategy = :defer
45+
# Don't explicitly set generated_component_packs_loading_strategy - let it default to :defer
46+
# which ensures generated component packs load and register components before main bundle executes
4647
end

spec/react_on_rails/configuration_spec.rb

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -284,9 +284,9 @@ module ReactOnRails
284284
.with("8.2.0").and_return(true)
285285
end
286286

287-
it "defaults to :async" do
287+
it "defaults to :defer" do
288288
ReactOnRails.configure {} # rubocop:disable Lint/EmptyBlock
289-
expect(ReactOnRails.configuration.generated_component_packs_loading_strategy).to eq(:async)
289+
expect(ReactOnRails.configuration.generated_component_packs_loading_strategy).to eq(:defer)
290290
end
291291

292292
it "accepts :async value" do
@@ -332,10 +332,9 @@ module ReactOnRails
332332
allow(Rails.logger).to receive(:warn)
333333
end
334334

335-
it "defaults to :sync and logs a warning" do
335+
it "defaults to :defer" do
336336
ReactOnRails.configure {} # rubocop:disable Lint/EmptyBlock
337-
expect(ReactOnRails.configuration.generated_component_packs_loading_strategy).to eq(:sync)
338-
expect(Rails.logger).to have_received(:warn).with(/does not support async script loading/)
337+
expect(ReactOnRails.configuration.generated_component_packs_loading_strategy).to eq(:defer)
339338
end
340339

341340
it "accepts :defer value" do

0 commit comments

Comments
 (0)