Skip to content

Commit d9169a6

Browse files
justin808claude
andcommitted
Address PR #3933 review: dev-default reporting, merge semantics, shared isThenable
Review fixes from the Claude Code review pass and Cursor Bugbot on PR #3933: 1. Dev-mode hydration logger no longer swallows React's default reporting or the component stack. The error is default-reported exactly once (reportError, falling back to console.error — mirroring React's own default) so window-'error' tooling keeps working, and the branded line now includes errorInfo.componentStack. 2. New buildRootErrorCallbackOptions option defaultReportingHandledInternally: Pro's RSC-wrapped hydrate paths pass it so the chained internal handler remains the single default reporter and the dev logger emits only its supplemental branded line (no second full error dump in development). Added a Pro test covering the chained dev path with railsEnv development. 3. getRootErrorHandlers now returns a snapshot copy so callers cannot mutate the internal registration and bypass validation. 4. Extracted the three byte-identical isThenable copies into packages/react-on-rails/src/isThenable.ts (exported as react-on-rails/@internal/isThenable) and removed the MUST-SYNC thenable-guard duplication note from the Pro renderer. 5. Partial rootErrorHandlers updates now merge per key (consistent with the other setOptions keys): setting only one callback keeps the others; an explicit undefined clears just that key; resetOptions still clears all. setOptions stores the merged result so option('rootErrorHandlers') reflects the effective registration. Docs (guide, JS API reference, type JSDoc) and the changelog entry updated to describe the preserved default reporting, component stack, and merge semantics. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 9dbbf98 commit d9169a6

15 files changed

Lines changed: 302 additions & 59 deletions

File tree

CHANGELOG.md

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

2727
#### Added
2828

29-
- **React 19 root error callbacks**: `ReactOnRails.setOptions({ rootErrorHandlers: { onRecoverableError, onCaughtError, onUncaughtError } })` registers React's root error callbacks globally; React on Rails applies them to every `hydrateRoot`/`createRoot` call it makes and invokes them with an extra context argument containing the component name and DOM id. In development, recoverable hydration errors now log an actionable React on Rails message (component name, dom id, and a link to the new [Debugging Hydration Mismatches guide](https://reactonrails.com/docs/building-features/debugging-hydration-mismatches)) instead of React's bare console line. On React <19 (and <18 for `onRecoverableError`), registration is a graceful no-op with a console warning. On React on Rails Pro RSC/streaming hydration paths, user callbacks chain with (never replace) Pro's internal recoverable-error handler. Addresses [Issue 3892](https://github.com/shakacode/react_on_rails/issues/3892). [PR 3933](https://github.com/shakacode/react_on_rails/pull/3933) by [justin808](https://github.com/justin808).
29+
- **React 19 root error callbacks**: `ReactOnRails.setOptions({ rootErrorHandlers: { onRecoverableError, onCaughtError, onUncaughtError } })` registers React's root error callbacks globally; React on Rails applies them to every `hydrateRoot`/`createRoot` call it makes and invokes them with an extra context argument containing the component name and DOM id. In development, recoverable hydration errors now log an actionable React on Rails message (component name, dom id, component stack, and a link to the new [Debugging Hydration Mismatches guide](https://reactonrails.com/docs/building-features/debugging-hydration-mismatches)) alongside React's default error reporting, which stays intact so window-'error'-based tooling keeps working. Partial `rootErrorHandlers` updates merge per key, so registering one callback later does not drop the others. On React <19 (and <18 for `onRecoverableError`), registration is a graceful no-op with a console warning. On React on Rails Pro RSC/streaming hydration paths, user callbacks chain with (never replace) Pro's internal recoverable-error handler. Addresses [Issue 3892](https://github.com/shakacode/react_on_rails/issues/3892). [PR 3933](https://github.com/shakacode/react_on_rails/pull/3933) by [justin808](https://github.com/justin808).
3030

3131
### [17.0.0.rc.2] - 2026-06-09
3232

docs/oss/api-reference/javascript-api.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ reactHydrateOrRender(domNode, reactElement, hydrate);
8282
* `rootErrorHandlers: { onRecoverableError, onCaughtError, onUncaughtError }` React root error
8383
* callbacks applied to every React root created by React on Rails. Each callback receives
8484
* React's (error, errorInfo) plus a context object with the componentName and domNodeId.
85+
* Partial updates merge per key (setting one callback later keeps the others).
8586
* onRecoverableError requires React 18+; onCaughtError/onUncaughtError require React 19.
8687
* See https://reactonrails.com/docs/building-features/debugging-hydration-mismatches
8788
*/

docs/oss/building-features/debugging-hydration-mismatches.md

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,16 @@ Rails apps hit a set of systematic causes that JavaScript-only frameworks rarely
88

99
### Development default
1010

11-
When `Rails.env.development?`, React on Rails automatically logs every recoverable hydration error with the component name, the DOM id, and a link to this guide — replacing React's bare console line:
11+
When `Rails.env.development?`, React on Rails automatically logs every recoverable hydration error with the component name, the DOM id, the component stack (when React provides one), and a link to this guide:
1212

1313
```text
1414
[ReactOnRails] Recoverable hydration error in component "MyComponent" (dom id: "MyComponent-react-component-0"). ...
15+
Component stack:
16+
at MyComponent
1517
```
1618

19+
React's default error reporting is preserved: the error itself is still reported once via `reportError` (falling back to `console.error`), so dev overlays and other window-`'error'`-based tooling keep working. The branded line is supplemental context. On React on Rails Pro RSC hydration paths, Pro's internal handler already reports the error, so only the supplemental line is added — each error is reported exactly once either way.
20+
1721
No setup is required. The default logger runs in addition to any callback you register.
1822

1923
### Registering root error callbacks (React 19)
@@ -51,7 +55,8 @@ Each callback receives React's original `(error, errorInfo)` arguments plus a `c
5155

5256
Notes:
5357

54-
- **Register before rendering.** Callbacks apply to roots created after registration; register them in the same pack file where you call `ReactOnRails.register`.
58+
- **Register before rendering.** Each root captures the callbacks registered at the moment it is created; register them in the same pack file where you call `ReactOnRails.register`.
59+
- **Partial updates merge per key.** A later `setOptions({ rootErrorHandlers: { onCaughtError } })` keeps a previously registered `onRecoverableError`/`onUncaughtError`. Pass an explicit `undefined` for a key to clear just that callback; `ReactOnRails.resetOptions()` clears all of them.
5560
- **React version support:** `onRecoverableError` requires React 18+; `onCaughtError`/`onUncaughtError` require React 19. On older React versions, registration is a graceful no-op and React on Rails logs a one-time `console.warn`.
5661
- **React on Rails Pro:** on RSC/streaming hydration paths, Pro installs an internal `onRecoverableError` for its own bookkeeping. Your callback is chained after it — both always run, and Pro's internal control-flow signals (such as the `RSCRoute` `ssr: false` bailout) are filtered out of both so they never reach your error reporter.
5762
- **Per-component overrides** are not currently supported; the global registration above is the blessed route.

packages/react-on-rails-pro/src/ClientSideRenderer.ts

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import { supportsHydrate, supportsRootApi, unmountComponentAtNode } from 'react-
3333
import reactHydrateOrRender from 'react-on-rails/reactHydrateOrRender';
3434
import { debugTurbolinks } from 'react-on-rails/turbolinksUtils';
3535
import { buildRootErrorCallbackOptions } from 'react-on-rails/@internal/rootErrorHandlers';
36+
import { isThenable } from 'react-on-rails/@internal/isThenable';
3637
import { maybeWrapWithDefaultRSCProviderWithStatus } from './defaultRSCProviderRegistry.ts';
3738
import { chainRecoverableErrorHandlers } from './handleRecoverableError.client.ts';
3839

@@ -41,24 +42,14 @@ import * as ComponentRegistry from './ComponentRegistry.ts';
4142

4243
const REACT_ON_RAILS_STORE_ATTRIBUTE = 'data-js-react-on-rails-store';
4344

44-
/** Narrows an unknown value to a thenable (has a callable `.then`) without assuming a native Promise. */
45-
function isThenable(value: unknown): value is PromiseLike<unknown> {
46-
return (
47-
value != null &&
48-
(typeof value === 'object' || typeof value === 'function') &&
49-
typeof (value as { then?: unknown }).then === 'function'
50-
);
51-
}
52-
5345
/**
5446
* Invokes a renderer teardown, swallowing async rejections so a failing teardown cannot produce an
5547
* unhandled promise rejection. Synchronous throws propagate to the caller's try/catch.
5648
*
5749
* Intentionally re-implemented (not imported) from the OSS `react-on-rails` `invokeRendererTeardown`:
5850
* the OSS module does not export it, so re-implementing keeps the Pro client renderer decoupled from
5951
* OSS internals (no reliance on a non-public export) instead of widening the OSS public API just to
60-
* share it. Keep the local thenable guard in sync with the OSS helper so non-native thenables are
61-
* handled the same way in both packages. The shared `RendererFunction`/`RendererTeardown`/
52+
* share it. The thenable guard (`isThenable`) and the shared `RendererFunction`/`RendererTeardown`/
6253
* `RendererTeardownResult` *types* are imported, so only this small runtime helper is duplicated.
6354
* MUST SYNC: A sibling helper exists in packages/react-on-rails/src/ClientRenderer.ts. If you
6455
* change the error-handling logic or log format here, update that copy too.
@@ -259,10 +250,12 @@ You should return a React.Component always for the client side entry point.`);
259250
// component name and dom id. Applied to every root; on the RSC-wrapped hydrate path the
260251
// user onRecoverableError is CHAINED after Pro's internal recoverable-error handler so
261252
// both run (the internal handler must never be clobbered, and the user callback must
262-
// never be dropped).
253+
// never be dropped). On that chained path the internal handler already default-reports
254+
// the error, so the dev-mode logger must emit only its supplemental branded line.
263255
const userErrorCallbackOptions = buildRootErrorCallbackOptions(
264256
{ componentName: name, domNodeId },
265257
shouldHydrate,
258+
{ defaultReportingHandledInternally: wrappedByDefaultRSCProvider && shouldHydrate },
266259
);
267260
let renderOptions: Parameters<typeof reactHydrateOrRender>[3] = userErrorCallbackOptions;
268261
if (wrappedByDefaultRSCProvider) {

packages/react-on-rails-pro/src/wrapServerComponentRenderer/client.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,11 +124,16 @@ const wrapServerComponentRenderer = (
124124

125125
// User-registered root error callbacks (rootErrorHandlers), wrapped with this mount's
126126
// component name and dom id. On the hydrate path the user onRecoverableError is CHAINED after
127-
// Pro's internal recoverable-error handler so both run.
127+
// Pro's internal recoverable-error handler so both run; the internal handler already
128+
// default-reports the error there, so the dev-mode logger emits only its supplemental branded
129+
// line (the flag is moot on the createRoot path, where the dev logger is not attached).
128130
const shouldHydrate = !!domNode.innerHTML;
129131
const userErrorCallbackOptions = buildRootErrorCallbackOptions(
130132
{ componentName, domNodeId },
131133
shouldHydrate,
134+
{
135+
defaultReportingHandledInternally: shouldHydrate,
136+
},
132137
);
133138
const reactRoot = shouldHydrate
134139
? ReactDOMClient.hydrateRoot(domNode, rootElement, {

packages/react-on-rails-pro/tests/ClientSideRenderer.test.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,58 @@ describe('ClientSideRenderer', () => {
396396
}
397397
});
398398

399+
it('emits one internal report plus the supplemental branded dev line on the chained dev hydrate path', async () => {
400+
// railsEnv 'development' + RSC-wrapped hydrate: the chained handler must default-report the
401+
// error exactly once (Pro's internal handler), with the dev logger adding only its branded
402+
// supplemental line — never a second full error dump.
403+
const TestComponent = ({ greeting }: { greeting: string }) => React.createElement('div', null, greeting);
404+
const defaultProviderFactory = jest.fn(({ reactElement }: DefaultRSCProviderFactoryArgs) => reactElement);
405+
const globalWithReportError = globalThis as typeof globalThis & {
406+
reportError?: (error: unknown) => void;
407+
};
408+
const originalReportError = globalWithReportError.reportError;
409+
const reportErrorSpy = jest.fn();
410+
globalWithReportError.reportError = reportErrorSpy;
411+
const userOnRecoverableError = jest.fn();
412+
setRootErrorHandlers({ onRecoverableError: userOnRecoverableError });
413+
ComponentRegistry.register({ TestComponent });
414+
const componentSpec = setupTestComponentDom('dom-id-dev', '<div>Server fallback</div>');
415+
addRailsContext({ railsEnv: 'development', rscPayloadGenerationUrlPath: '/rsc_payload' });
416+
setDefaultRSCProviderFactory(defaultProviderFactory);
417+
418+
try {
419+
await renderOrHydrateComponent(componentSpec);
420+
421+
const recoverableError = new Error('dev chained recoverable error');
422+
const errorInfo = { componentStack: '\n at TestComponent' };
423+
const onRecoverableError = expectRecoverableHandlerFromLastRender();
424+
(onRecoverableError as (error: unknown, errorInfo: unknown) => void)(recoverableError, errorInfo);
425+
426+
// Exactly one default report — Pro's internal handler.
427+
expect(reportErrorSpy).toHaveBeenCalledTimes(1);
428+
expect(reportErrorSpy).toHaveBeenCalledWith(recoverableError);
429+
430+
// Exactly one console.error: the branded supplemental line (context + stack + guide link),
431+
// not a second full error dump.
432+
const consoleErrorCalls = (console.error as jest.Mock).mock.calls;
433+
expect(consoleErrorCalls).toHaveLength(1);
434+
const [message] = consoleErrorCalls[0] as [unknown];
435+
expect(message).toEqual(expect.stringContaining('[ReactOnRails] Recoverable hydration error'));
436+
expect(message).toEqual(expect.stringContaining('"TestComponent"'));
437+
expect(message).toEqual(expect.stringContaining('dom-id-dev'));
438+
expect(message).toEqual(expect.stringContaining('Component stack:'));
439+
expect(consoleErrorCalls[0]).toHaveLength(1);
440+
441+
// The user callback still runs with the enriched context.
442+
expect(userOnRecoverableError).toHaveBeenCalledWith(recoverableError, errorInfo, {
443+
componentName: 'TestComponent',
444+
domNodeId: 'dom-id-dev',
445+
});
446+
} finally {
447+
globalWithReportError.reportError = originalReportError;
448+
}
449+
});
450+
399451
it('passes user root error callbacks on non-RSC-wrapped hydrate paths without the internal handler', async () => {
400452
const TestComponent = ({ greeting }: { greeting: string }) => React.createElement('div', null, greeting);
401453
const globalWithReportError = globalThis as typeof globalThis & {

packages/react-on-rails/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
"./@internal/sanitizeNonce": "./lib/sanitizeNonce.js",
5555
"./@internal/rendererTeardown": "./lib/rendererTeardown.js",
5656
"./@internal/rootErrorHandlers": "./lib/rootErrorHandlers.js",
57+
"./@internal/isThenable": "./lib/isThenable.js",
5758
"./@internal/base/client": "./lib/base/client.js",
5859
"./@internal/base/full": {
5960
"react-server": "./lib/base/full.rsc.js",

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

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { onPageUnloaded } from './pageLifecycle.ts';
1717
import { supportsRootApi, unmountComponentAtNode } from './reactApis.cts';
1818
import { isRendererTeardownResult } from './rendererTeardown.ts';
1919
import { buildRootErrorCallbackOptions } from './rootErrorHandlers.ts';
20+
import { isThenable } from './isThenable.ts';
2021

2122
const REACT_ON_RAILS_STORE_ATTRIBUTE = 'data-js-react-on-rails-store';
2223

@@ -35,15 +36,6 @@ type RenderedEntry =
3536
| { kind: 'react'; root: RenderReturnType; domNode: Element }
3637
| { kind: 'renderer'; teardown?: RendererTeardown; domNode: Element };
3738

38-
/** Narrows an unknown value to a thenable (has a callable `.then`) without assuming a native Promise. */
39-
function isThenable(value: unknown): value is PromiseLike<unknown> {
40-
return (
41-
value != null &&
42-
(typeof value === 'object' || typeof value === 'function') &&
43-
typeof (value as { then?: unknown }).then === 'function'
44-
);
45-
}
46-
4739
// Track all rendered roots for cleanup
4840
const renderedRoots = new Map<string, RenderedEntry>();
4941

packages/react-on-rails/src/base/client.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import createReactOutput from '../createReactOutput.ts';
2121
import componentRegistrationMetric from '../componentRegistrationMetric.ts';
2222
import {
2323
buildRootErrorCallbackOptions,
24+
getRootErrorHandlers,
2425
resetRootErrorHandlers,
2526
setRootErrorHandlers,
2627
} from '../rootErrorHandlers.ts';
@@ -180,9 +181,11 @@ Fix: Use only react-on-rails OR react-on-rails-pro, not both.`);
180181
}
181182

182183
if (typeof newOptions.rootErrorHandlers !== 'undefined') {
183-
// Validates and stores the handlers; warns when the React runtime cannot support them.
184+
// Validates and merges the handlers per key (partial updates keep previously registered
185+
// callbacks); warns when the React runtime cannot support them. Store the merged result so
186+
// `option('rootErrorHandlers')` reflects the effective registration.
184187
setRootErrorHandlers(newOptions.rootErrorHandlers);
185-
this.options.rootErrorHandlers = newOptions.rootErrorHandlers;
188+
this.options.rootErrorHandlers = getRootErrorHandlers();
186189
// eslint-disable-next-line no-param-reassign
187190
delete newOptions.rootErrorHandlers;
188191
}

packages/react-on-rails/src/capabilities/core.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import createReactOutput from '../createReactOutput.ts';
1515
import componentRegistrationMetric from '../componentRegistrationMetric.ts';
1616
import {
1717
buildRootErrorCallbackOptions,
18+
getRootErrorHandlers,
1819
resetRootErrorHandlers,
1920
setRootErrorHandlers,
2021
} from '../rootErrorHandlers.ts';
@@ -101,9 +102,11 @@ export function createCoreCapability(registries: Registries) {
101102
}
102103

103104
if (typeof rootErrorHandlers !== 'undefined') {
104-
// Validates and stores the handlers; warns when the React runtime cannot support them.
105+
// Validates and merges the handlers per key (partial updates keep previously registered
106+
// callbacks); warns when the React runtime cannot support them. Store the merged result so
107+
// `option('rootErrorHandlers')` reflects the effective registration.
105108
setRootErrorHandlers(rootErrorHandlers);
106-
this.options.rootErrorHandlers = rootErrorHandlers;
109+
this.options.rootErrorHandlers = getRootErrorHandlers();
107110
}
108111

109112
if (Object.keys(rest).length > 0) {

0 commit comments

Comments
 (0)