diff --git a/PR_1972_INVESTIGATION.md b/PR_1972_INVESTIGATION.md
new file mode 100644
index 0000000000..cd01b8afaa
--- /dev/null
+++ b/PR_1972_INVESTIGATION.md
@@ -0,0 +1,324 @@
+# PR #1972 Investigation: Component Registration Race Condition
+
+## Executive Summary
+
+PR #1972 attempted to fix intermittent CI test failures by changing the default `generated_component_packs_loading_strategy` from `:async` to `:defer`. This document provides an in-depth analysis of the race condition, the proposed solution, and recommendations for a better approach.
+
+## The Problem
+
+### Test Failures
+
+The following tests were failing intermittently in CI:
+
+- `spec/system/integration_spec.rb[1:1:6:1:2]` - "2 react components, 1 store, client only, defer"
+- `spec/system/integration_spec.rb[1:1:6:2:2]` - "2 react components, 1 store, server side, defer"
+
+These tests check Redux shared store functionality where two components share the same store and typing in one component should update the other.
+
+### Root Cause Analysis
+
+#### The Race Condition
+
+With `async` script loading (default on Shakapacker >= 8.2.0):
+
+1. **Browser behavior**: When scripts have the `async` attribute, they:
+
+ - Download in parallel (good for performance)
+ - Execute immediately when download completes (unpredictable order)
+ - Do not block HTML parsing
+
+2. **The problem with generated component packs**:
+
+ ```html
+
+
+
+
+
+ ```
+
+3. **Race condition scenario**:
+ - If `client-bundle.js` finishes downloading first, it executes immediately
+ - React hydration starts before component registrations from generated packs
+ - Error: "Could not find component registered with name ComponentName"
+
+#### Why It's Intermittent
+
+The race condition depends on:
+
+- Network conditions
+- File sizes (smaller files download faster)
+- Browser caching
+- Server response times
+
+This makes it particularly difficult to reproduce locally but common in CI environments with varying network conditions.
+
+## PR #1972 Solution Analysis
+
+### What Changed
+
+1. **Configuration default** (`lib/react_on_rails/configuration.rb`):
+
+ ```ruby
+ # OLD: Defaulted to :async when Shakapacker >= 8.2.0, else :sync
+ # NEW: Always defaults to :defer
+ self.generated_component_packs_loading_strategy = :defer
+ ```
+
+2. **Layout file** (`spec/dummy/app/views/layouts/application.html.erb`):
+
+ ```erb
+
+
+ <%= javascript_pack_tag('client-bundle', defer: true) %>
+ ```
+
+3. **Test expectations updated** to expect `:defer` as default
+
+### How Defer "Fixes" It
+
+With `defer`:
+
+- Scripts still download in parallel (fast)
+- Scripts execute in DOM order after HTML parsing completes
+- Generated component packs execute before main bundle (predictable)
+- Component registrations complete before React hydration
+
+```html
+
+
+
+
+
+```
+
+## The Real Issue
+
+### Why This Solution Is Problematic
+
+1. **Performance Impact**:
+
+ - `async` provides better performance by executing scripts as soon as they're ready
+ - `defer` forces sequential execution, which can be slower
+ - Modern web apps benefit from async loading
+
+2. **Masks Architectural Problem**:
+
+ - The real issue is that React hydration shouldn't depend on script execution order
+ - Components should be registered before hydration attempts to use them
+ - This is a timing/coordination problem, not a loading strategy problem
+
+3. **Doesn't Address Root Cause**:
+ - The race condition still exists with generated component packs
+ - We're just forcing a particular execution order to avoid it
+ - Better solution: ensure component registry is ready before hydration
+
+### The `uses_redux_shared_store?` Helper
+
+Before PR #1972, there was conditional logic:
+
+```ruby
+# application_controller.rb
+def uses_redux_shared_store?
+ action_name.in?(%w[
+ index
+ server_side_redux_app
+ # ... other actions with shared stores
+ ])
+end
+```
+
+This recognized that **only certain pages need defer**. PR #1972 removed this nuance by forcing defer everywhere.
+
+## Recommended Approach
+
+### Option 1: Component Registry Timeout (Already Implemented!)
+
+React on Rails already has `component_registry_timeout` (default 5000ms):
+
+```ruby
+# configuration.rb
+component_registry_timeout: DEFAULT_COMPONENT_REGISTRY_TIMEOUT # 5000ms
+```
+
+This means the client-side code should **wait** for components to register before hydrating. The race condition might indicate:
+
+- The timeout isn't working correctly
+- There's a bug in the component registration check
+- The timeout is too short for CI environments
+
+**Investigation needed**:
+
+- Review `packages/react-on-rails/src/` for component registry logic
+- Check if hydration properly waits for registrations
+- Verify timeout is honored in all code paths
+
+### Option 2: Explicit Component Dependencies
+
+Make the main bundle explicitly wait for generated pack scripts:
+
+```javascript
+// In generated component packs:
+window.ReactOnRailsComponentsReady = window.ReactOnRailsComponentsReady || [];
+window.ReactOnRailsComponentsReady.push('ComponentName');
+
+// In client-bundle before hydration:
+function waitForComponents(required, timeout = 5000) {
+ return new Promise((resolve, reject) => {
+ const check = () => {
+ if (required.every((name) => window.ReactOnRailsComponentsReady.includes(name))) {
+ resolve();
+ }
+ };
+ // Poll until ready or timeout
+ });
+}
+```
+
+### Option 3: Module Dependencies
+
+Use ES modules with dynamic imports:
+
+```javascript
+// Instead of script tags, use:
+const component = await import(`./generated/${componentName}`);
+```
+
+This gives explicit control over load order without sacrificing async benefits.
+
+### Option 4: Smart Loading Strategy
+
+Keep async as default but fall back to defer only when needed:
+
+```ruby
+# Configuration that detects when defer is necessary
+def required_loading_strategy
+ if @rendered_components.any? { |c| needs_guaranteed_order?(c) }
+ :defer
+ else
+ :async
+ end
+end
+```
+
+## Test Analysis
+
+### The Failing Tests
+
+Looking at `spec/dummy/spec/system/integration_spec.rb:360-382`:
+
+```ruby
+describe "2 react components, 1 store, client only, defer", :js do
+ include_examples "React Component Shared Store", "/client_side_hello_world_shared_store_defer"
+end
+
+describe "2 react components, 1 store, server side, defer", :js do
+ include_examples "React Component Shared Store", "/server_side_hello_world_shared_store_defer"
+end
+```
+
+These tests **specifically test defer functionality**. The fact that they fail with async is expected behavior! The routes ending in `_defer` are explicitly testing defer mode.
+
+**Key insight**: The failures might not be a bug but tests failing because:
+
+1. Default was changed from async to defer
+2. Tests expected defer behavior
+3. When default was async, these defer-specific tests used async instead
+
+## Recommendations
+
+### Immediate Actions
+
+1. **Revert PR #1972** ✅ (Already done)
+
+2. **Investigate component registry timeout**:
+
+ - Review `packages/react-on-rails/src/ComponentRegistry.ts`
+ - Check `component_registry_timeout` implementation
+ - Add detailed logging to see when/why registrations fail
+
+3. **Reproduce the race condition locally**:
+
+ ```bash
+ # Throttle network to simulate CI conditions
+ # Use browser DevTools Network tab -> Throttling
+ # Run tests multiple times to catch intermittent failures
+ ```
+
+4. **Add instrumentation**:
+ ```javascript
+ console.log('[RoR] Component registered:', componentName, Date.now());
+ console.log('[RoR] Attempting hydration:', componentName, Date.now());
+ console.log('[RoR] Registry contents:', Object.keys(componentRegistry));
+ ```
+
+### Long-term Solutions
+
+1. **Fix the timing issue properly**:
+
+ - Ensure `component_registry_timeout` works correctly
+ - Make hydration explicitly wait for required components
+ - Add warnings when components aren't registered in time
+
+2. **Make loading strategy configurable per-component**:
+
+ ```ruby
+ react_component('ComponentName', props, loading_strategy: :defer)
+ ```
+
+3. **Document when defer is needed**:
+
+ - Update docs to explain async vs defer trade-offs
+ - Provide guidance on when to use each
+ - Explain the performance implications
+
+4. **Improve test reliability**:
+ - Add retries for tests with network dependencies
+ - Use `retry: 3` in RSpec for these specific tests
+ - Consider mocking/stubbing script loading in tests
+
+## Questions to Answer
+
+1. **Why does `component_registry_timeout` not prevent the race condition?**
+
+ - Is it being used correctly?
+ - Is there a code path that bypasses it?
+ - Are generated component packs registering correctly?
+
+2. **Why do defer-specific tests fail with async default?**
+
+ - Are the routes configured correctly?
+ - Should these tests explicitly set the loading strategy?
+ - Is there a bug in the configuration precedence?
+
+3. **Can we detect when defer is truly necessary?**
+ - Shared Redux stores?
+ - Inline component registration?
+ - Server-side rendering?
+
+## Conclusion
+
+PR #1972's solution works but treats the symptom rather than the disease. The real fix requires:
+
+1. Understanding why the component registry timeout doesn't prevent the race
+2. Fixing the underlying timing/coordination issue
+3. Keeping async as the default for performance
+4. Using defer only when truly necessary (documented cases)
+
+The intermittent nature of the failures suggests a real race condition that needs proper synchronization, not just forced execution order.
+
+## Next Steps
+
+1. ✅ Revert PR #1972
+2. ⏳ Deep dive into component registry timeout implementation
+3. ⏳ Reproduce failures locally with network throttling
+4. ⏳ Add instrumentation to understand timing
+5. ⏳ Implement proper synchronization fix
+6. ⏳ Update documentation with clear guidance
+7. ⏳ Create new PR with proper solution
+
+---
+
+**Author**: Claude Code
+**Date**: November 11, 2025
+**Status**: Investigation Complete, Awaiting Implementation
diff --git a/lib/react_on_rails/configuration.rb b/lib/react_on_rails/configuration.rb
index 82f76bb76b..6d89cd071e 100644
--- a/lib/react_on_rails/configuration.rb
+++ b/lib/react_on_rails/configuration.rb
@@ -154,9 +154,9 @@ def check_component_registry_timeout
raise ReactOnRails::Error, "component_registry_timeout must be a positive integer"
end
- # rubocop:disable Metrics/CyclomaticComplexity
+ # rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
def validate_generated_component_packs_loading_strategy
- # rubocop:enable Metrics/CyclomaticComplexity
+ # rubocop:enable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
if defer_generated_component_packs
if %i[async sync].include?(generated_component_packs_loading_strategy)
@@ -176,11 +176,12 @@ def validate_generated_component_packs_loading_strategy
1. Use :defer or :sync loading strategy instead of :async
2. Upgrade to Shakapacker v8.2.0 or above to enable async script loading
MSG
- if generated_component_packs_loading_strategy.nil?
- # Use defer as the default to ensure generated component packs load and register
- # components before main bundle executes, avoiding race conditions with async loading
- self.generated_component_packs_loading_strategy = :defer
- elsif generated_component_packs_loading_strategy == :async && !PackerUtils.supports_async_loading?
+ if PackerUtils.supports_async_loading?
+ self.generated_component_packs_loading_strategy ||= :async
+ elsif generated_component_packs_loading_strategy.nil?
+ Rails.logger.warn("**WARNING** #{msg}")
+ self.generated_component_packs_loading_strategy = :sync
+ elsif generated_component_packs_loading_strategy == :async
raise ReactOnRails::Error, "**ERROR** #{msg}"
end
diff --git a/spec/dummy/app/views/layouts/application.html.erb b/spec/dummy/app/views/layouts/application.html.erb
index 8c7bc06bb0..0e25174d52 100644
--- a/spec/dummy/app/views/layouts/application.html.erb
+++ b/spec/dummy/app/views/layouts/application.html.erb
@@ -9,10 +9,16 @@
<%= yield :head %>
- <%# Use defer: true to ensure proper script execution order.
- When using generated component packs (auto_load_bundle), defer ensures
- component registrations complete before React hydration begins. %>
- <%= javascript_pack_tag('client-bundle', 'data-turbo-track': 'reload', defer: true) %>
+ <%# Conditionally use defer: true for pages with Redux shared stores (inline registration).
+ Modern apps should use async: true for optimal performance. See docs for details:
+ docs/building-features/streaming-server-rendering.md %>
+ <% if uses_redux_shared_store? %>
+ <%# defer: true required for Redux shared stores with inline component registration %>
+ <%= javascript_pack_tag('client-bundle', 'data-turbo-track': 'reload', defer: true) %>
+ <% else %>
+ <%# async: true is the recommended approach for modern apps (Shakapacker >= 8.2.0) %>
+ <%= javascript_pack_tag('client-bundle', 'data-turbo-track': 'reload', async: true) %>
+ <% end %>
<%= csrf_meta_tags %>
diff --git a/spec/dummy/config/initializers/react_on_rails.rb b/spec/dummy/config/initializers/react_on_rails.rb
index 7578a995df..54c2f40d5c 100644
--- a/spec/dummy/config/initializers/react_on_rails.rb
+++ b/spec/dummy/config/initializers/react_on_rails.rb
@@ -42,6 +42,5 @@ def self.adjust_props_for_client_side_hydration(component_name, props)
config.components_subdirectory = "startup"
config.auto_load_bundle = true
config.immediate_hydration = false
- # Don't explicitly set generated_component_packs_loading_strategy - let it default to :defer
- # which ensures generated component packs load and register components before main bundle executes
+ config.generated_component_packs_loading_strategy = :defer
end
diff --git a/spec/react_on_rails/configuration_spec.rb b/spec/react_on_rails/configuration_spec.rb
index e4198b8d94..e3123fcdb0 100644
--- a/spec/react_on_rails/configuration_spec.rb
+++ b/spec/react_on_rails/configuration_spec.rb
@@ -284,9 +284,9 @@ module ReactOnRails
.with("8.2.0").and_return(true)
end
- it "defaults to :defer" do
+ it "defaults to :async" do
ReactOnRails.configure {} # rubocop:disable Lint/EmptyBlock
- expect(ReactOnRails.configuration.generated_component_packs_loading_strategy).to eq(:defer)
+ expect(ReactOnRails.configuration.generated_component_packs_loading_strategy).to eq(:async)
end
it "accepts :async value" do
@@ -332,9 +332,10 @@ module ReactOnRails
allow(Rails.logger).to receive(:warn)
end
- it "defaults to :defer" do
+ it "defaults to :sync and logs a warning" do
ReactOnRails.configure {} # rubocop:disable Lint/EmptyBlock
- expect(ReactOnRails.configuration.generated_component_packs_loading_strategy).to eq(:defer)
+ expect(ReactOnRails.configuration.generated_component_packs_loading_strategy).to eq(:sync)
+ expect(Rails.logger).to have_received(:warn).with(/does not support async script loading/)
end
it "accepts :defer value" do