Skip to content

Commit 7afd66e

Browse files
authored
Revert: Fix component registration race condition (#1972) (#1981)
## Summary Reverts commit d1a8a1a which changed the default component loading strategy from :sync to :defer. This reverts the fix for the component registration race condition that was introduced in #1972. ## Changes - Reverts the defer loading strategy back to the original behavior - Restores async loading support when available - Updates tests to reflect the original expectations <!-- 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/1981) <!-- Reviewable:end -->
1 parent 93c15ef commit 7afd66e

File tree

4 files changed

+24
-17
lines changed

4 files changed

+24
-17
lines changed

lib/react_on_rails/configuration.rb

Lines changed: 8 additions & 7 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
157+
# rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
158158
def validate_generated_component_packs_loading_strategy
159-
# rubocop:enable Metrics/CyclomaticComplexity
159+
# rubocop:enable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
160160

161161
if defer_generated_component_packs
162162
if %i[async sync].include?(generated_component_packs_loading_strategy)
@@ -176,11 +176,12 @@ def validate_generated_component_packs_loading_strategy
176176
1. Use :defer or :sync loading strategy instead of :async
177177
2. Upgrade to Shakapacker v8.2.0 or above to enable async script loading
178178
MSG
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?
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
184185
raise ReactOnRails::Error, "**ERROR** #{msg}"
185186
end
186187

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

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

1010
<%= yield :head %>
1111

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) %>
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 %>
1622

1723
<%= csrf_meta_tags %>
1824
</head>

spec/dummy/config/initializers/react_on_rails.rb

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,5 @@ 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-
# 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
45+
config.generated_component_packs_loading_strategy = :defer
4746
end

spec/react_on_rails/configuration_spec.rb

Lines changed: 5 additions & 4 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 :defer" do
287+
it "defaults to :async" do
288288
ReactOnRails.configure {} # rubocop:disable Lint/EmptyBlock
289-
expect(ReactOnRails.configuration.generated_component_packs_loading_strategy).to eq(:defer)
289+
expect(ReactOnRails.configuration.generated_component_packs_loading_strategy).to eq(:async)
290290
end
291291

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

335-
it "defaults to :defer" do
335+
it "defaults to :sync and logs a warning" do
336336
ReactOnRails.configure {} # rubocop:disable Lint/EmptyBlock
337-
expect(ReactOnRails.configuration.generated_component_packs_loading_strategy).to eq(:defer)
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/)
338339
end
339340

340341
it "accepts :defer value" do

0 commit comments

Comments
 (0)