Skip to content

Commit 4eb8364

Browse files
ihabadhamclaude
andauthored
Fix initial page startup race for late-loading client bundles (#3151)
### Summary Fixes a client lifecycle gap where React on Rails can miss the initial page startup if the bundle begins executing after `DOMContentLoaded` has already fired but before the document reaches `complete`. Fixes #3150 ### Problem Core startup currently initializes immediately only when `document.readyState === "complete"`, and otherwise waits for `DOMContentLoaded`. That avoids starting too early during `interactive`, when deferred or module scripts may still be registering components. But there is another browser timing window: - the bundle starts while `document.readyState === "interactive"` - `DOMContentLoaded` has already fired - the document is not yet `"complete"` In that case, the `DOMContentLoaded` listener is attached too late, so the initial page-load callbacks never run. ### What changed Adds a `readystatechange → complete` fallback listener when registration happens during `interactive`. Both listeners share a single handler that removes both on first fire, so initialization runs exactly once in real browsers. ### Test plan - Added regression coverage for the missed-DOMContentLoaded-during-interactive scenario. Verified by temporarily reverting the fix and running the new tests against main's `pageLifecycle.ts` — exactly the new tests failed. - Verified initial startup now runs when the client bundle begins during `interactive` after `DOMContentLoaded` has already fired. --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent ad45e4a commit 4eb8364

3 files changed

Lines changed: 47 additions & 27 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ After a release, run `/update-changelog` in Claude Code to analyze commits, writ
2626

2727
#### Fixed
2828

29+
- **Client startup now recovers if initialization begins during `interactive` after `DOMContentLoaded` already fired**: React on Rails now still initializes the page when the client bundle starts in the browser timing window after `DOMContentLoaded` but before the document reaches `complete`. Fixes [Issue 3150](https://github.com/shakacode/react_on_rails/issues/3150). [PR 3151](https://github.com/shakacode/react_on_rails/pull/3151) by [ihabadham](https://github.com/ihabadham).
2930
- **Doctor accepts TypeScript server bundle entrypoints**: `react_on_rails:doctor` now resolves common source entrypoint suffixes (`.js`, `.jsx`, `.ts`, `.tsx`, `.mjs`, `.cjs`) before warning that the server bundle is missing, preventing false positives when apps use `server-bundle.ts`. [PR 3111](https://github.com/shakacode/react_on_rails/pull/3111) by [justin808](https://github.com/justin808).
3031
- **Doctor no longer fails custom projects for a missing generated `bin/dev`**: `react_on_rails:doctor` now downgrades a missing official React on Rails `bin/dev` launcher from an error to a warning and adds explicit guidance when a custom `./dev` script is detected, so custom projects can pass diagnostics when their development setup is intentional. Fixes [Issue 3103](https://github.com/shakacode/react_on_rails/issues/3103). [PR 3117](https://github.com/shakacode/react_on_rails/pull/3117) by [justin808](https://github.com/justin808).
3132

packages/react-on-rails/src/pageLifecycle.ts

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -67,20 +67,33 @@ function initializePageEventListeners(): void {
6767
}
6868
isPageLifecycleInitialized = true;
6969

70-
// Important: replacing this condition with `document.readyState !== 'loading'` is not valid
71-
// As the core ReactOnRails needs to ensure that all component bundles are loaded and executed before hydrating them
72-
// If the `document.readyState === 'interactive'`, it doesn't guarantee that deferred scripts are executed
73-
// the `readyState` can be `'interactive'` while the deferred scripts are still being executed
74-
// Which will lead to the error `"Could not find component registered with name <component name>"`
75-
// It will happen if this line is reached before the component chunk is executed on browser and reached the line
76-
// ReactOnRails.register({ Component });
77-
// ReactOnRailsPro is resellient against that type of race conditions, but it won't wait for that state anyway
78-
// As it immediately hydrates the components at the page as soon as its html and bundle is loaded on the browser
79-
// See pageLifecycle.test.js for unit tests validating this logic
70+
// Important: replacing this condition with `document.readyState !== 'loading'` is not valid for
71+
// the core page-load sweep. During `interactive`, deferred/module scripts may still be executing,
72+
// and a component chunk may not yet have reached `ReactOnRails.register({ Component })`.
73+
// Starting hydration too early can trigger "Could not find component registered" errors.
74+
//
75+
// However, async or dynamically-injected scripts can start after DOMContentLoaded has already fired
76+
// while the document is still `interactive`. In that case, waiting only for DOMContentLoaded can miss
77+
// initialization entirely, so we recover on the later `complete` transition.
78+
//
79+
// ReactOnRailsPro's early hydration path is more resilient to the registration race because it can
80+
// hydrate components as their HTML and bundles arrive, but this page lifecycle still powers the
81+
// fallback page-load sweep. See pageLifecycle.test.js for regression coverage of both cases.
8082
if (document.readyState === 'complete') {
8183
setupPageNavigationListeners();
8284
} else {
83-
document.addEventListener('DOMContentLoaded', setupPageNavigationListeners);
85+
const initialPageLoadHandler = (event: Event): void => {
86+
if (event.type === 'readystatechange' && document.readyState !== 'complete') return;
87+
document.removeEventListener('DOMContentLoaded', initialPageLoadHandler);
88+
document.removeEventListener('readystatechange', initialPageLoadHandler);
89+
setupPageNavigationListeners();
90+
};
91+
92+
document.addEventListener('DOMContentLoaded', initialPageLoadHandler);
93+
94+
if (document.readyState === 'interactive') {
95+
document.addEventListener('readystatechange', initialPageLoadHandler);
96+
}
8497
}
8598
}
8699

packages/react-on-rails/tests/pageLifecycle.test.js

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ describe('pageLifecycle', () => {
3131
// eslint-disable-next-line global-require
3232
const importPageLifecycle = () => require('../src/pageLifecycle.ts');
3333

34+
const getEventHandler = (eventName) =>
35+
addEventListenerSpy.mock.calls.find((call) => call[0] === eventName)?.[1];
36+
3437
// Helper function to create navigation library mock
3538
const createNavigationMock = (overrides = {}) => ({
3639
debugTurbolinks: jest.fn(),
@@ -81,7 +84,7 @@ describe('pageLifecycle', () => {
8184
expect(addEventListenerSpy).not.toHaveBeenCalledWith('DOMContentLoaded', expect.any(Function));
8285
});
8386

84-
it('should wait for DOMContentLoaded when when document.readyState is "interactive"', () => {
87+
it('should wait for DOMContentLoaded when document.readyState is "interactive"', () => {
8588
setReadyState('interactive');
8689
const callback = jest.fn();
8790
const { onPageLoaded } = importPageLifecycle();
@@ -92,6 +95,7 @@ describe('pageLifecycle', () => {
9295
expect(callback).not.toHaveBeenCalled();
9396
// Verify that a DOMContentLoaded listener was added when readyState is 'interactive'
9497
expect(addEventListenerSpy).toHaveBeenCalledWith('DOMContentLoaded', expect.any(Function));
98+
expect(addEventListenerSpy).toHaveBeenCalledWith('readystatechange', expect.any(Function));
9599
});
96100

97101
it('should wait for DOMContentLoaded when document.readyState is "loading"', () => {
@@ -272,8 +276,10 @@ describe('pageLifecycle', () => {
272276
// - It tries to hydrate components BEFORE step 5 completes
273277
// - ComponentRegistry.get() throws "Could not find component registered with name"
274278
//
275-
// The fix: Use `readyState === 'complete'` to ensure we wait for DOMContentLoaded
276-
// (which fires AFTER deferred scripts execute)
279+
// The fix: do not initialize immediately during `interactive`.
280+
// Wait for DOMContentLoaded (which fires AFTER deferred scripts execute),
281+
// and separately recover on the later `complete` transition only if
282+
// DOMContentLoaded was already missed before startup ran.
277283

278284
it('should NOT call callbacks immediately when readyState is "interactive" because deferred scripts may not have executed', () => {
279285
// Simulate the state right after HTML parsing completes but before deferred scripts run
@@ -285,9 +291,9 @@ describe('pageLifecycle', () => {
285291
const hydrateComponentCallback = jest.fn();
286292
onPageLoaded(hydrateComponentCallback);
287293

288-
// CRITICAL: With the correct implementation (readyState === 'complete'),
289-
// the callback should NOT be called immediately when readyState is 'interactive'.
290-
// This gives deferred scripts time to execute and register components.
294+
// CRITICAL: With the correct implementation, the callback should NOT be
295+
// called immediately when readyState is 'interactive'. This gives deferred
296+
// scripts time to execute and register components before hydration runs.
291297
expect(hydrateComponentCallback).not.toHaveBeenCalled();
292298

293299
// Instead, a DOMContentLoaded listener should be added
@@ -324,7 +330,7 @@ describe('pageLifecycle', () => {
324330

325331
onPageLoaded(attemptHydration);
326332

327-
// With correct implementation: callback not called yet, so no error
333+
// With the correct implementation: callback not called yet, so no error
328334
expect(attemptHydration).not.toHaveBeenCalled();
329335

330336
// Simulate deferred script executing (this happens before DOMContentLoaded)
@@ -339,12 +345,12 @@ describe('pageLifecycle', () => {
339345

340346
// Now when DOMContentLoaded fires, the component is registered
341347
// and hydration succeeds
342-
domContentLoadedHandler();
348+
domContentLoadedHandler(new Event('DOMContentLoaded'));
343349
expect(attemptHydration).toHaveBeenCalled();
344350
// No error thrown because componentRegistered is now true
345351
});
346352

347-
it('should handle the case where readyState transitions from interactive to complete', () => {
353+
it('should recover when DOMContentLoaded was already missed during interactive by waiting for complete', () => {
348354
// Start in 'interactive' state (HTML parsed, deferred scripts may be running)
349355
setReadyState('interactive');
350356

@@ -356,17 +362,17 @@ describe('pageLifecycle', () => {
356362
// Callback should not be called immediately at 'interactive'
357363
expect(callback).not.toHaveBeenCalled();
358364
expect(addEventListenerSpy).toHaveBeenCalledWith('DOMContentLoaded', expect.any(Function));
365+
expect(addEventListenerSpy).toHaveBeenCalledWith('readystatechange', expect.any(Function));
359366

360-
// Get the DOMContentLoaded handler
361-
const domContentLoadedHandler = addEventListenerSpy.mock.calls.find(
362-
(call) => call[0] === 'DOMContentLoaded',
363-
)?.[1];
367+
// Simulate the case where DOMContentLoaded already fired before startup installed its listener.
368+
// The later transition to complete should still recover initialization.
369+
const readyStateChangeHandler = getEventHandler('readystatechange');
370+
expect(readyStateChangeHandler).toBeDefined();
364371

365-
// Simulate state transition: deferred scripts complete, then DOMContentLoaded fires
366372
setReadyState('complete');
367-
domContentLoadedHandler();
373+
readyStateChangeHandler(new Event('readystatechange'));
368374

369-
// Now callback should have been called
375+
// Now callback should have been called through the complete fallback
370376
expect(callback).toHaveBeenCalledTimes(1);
371377
});
372378
});

0 commit comments

Comments
 (0)