Skip to content

Commit 6470e0f

Browse files
authored
[Fresh] Make all errors recoverable (#17438)
* [Fresh] Detect root updates more reliably * [Fresh] Use WeakMap for root elements * [Fresh] Make initial failures recoverable too * Fix DevTools check * Fix wrong flow type
1 parent 54f6673 commit 6470e0f

File tree

5 files changed

+195
-80
lines changed

5 files changed

+195
-80
lines changed

packages/react-reconciler/src/ReactFiberDevToolsHook.js

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,14 @@ import {inferPriorityFromExpirationTime} from './ReactFiberExpirationTime';
1414
import type {Fiber} from './ReactFiber';
1515
import type {FiberRoot} from './ReactFiberRoot';
1616
import type {ExpirationTime} from './ReactFiberExpirationTime';
17+
import type {ReactNodeList} from 'shared/ReactTypes';
1718

1819
import {DidCapture} from 'shared/ReactSideEffectTags';
1920
import warningWithoutStack from 'shared/warningWithoutStack';
2021

2122
declare var __REACT_DEVTOOLS_GLOBAL_HOOK__: Object | void;
2223

24+
let onScheduleFiberRoot = null;
2325
let onCommitFiberRoot = null;
2426
let onCommitFiberUnmount = null;
2527
let hasLoggedError = false;
@@ -54,6 +56,25 @@ export function injectInternals(internals: Object): boolean {
5456
try {
5557
const rendererID = hook.inject(internals);
5658
// We have successfully injected, so now it is safe to set up hooks.
59+
if (__DEV__) {
60+
// Only used by Fast Refresh
61+
if (typeof hook.onScheduleFiberRoot === 'function') {
62+
onScheduleFiberRoot = (root, children) => {
63+
try {
64+
hook.onScheduleFiberRoot(rendererID, root, children);
65+
} catch (err) {
66+
if (__DEV__ && !hasLoggedError) {
67+
hasLoggedError = true;
68+
warningWithoutStack(
69+
false,
70+
'React DevTools encountered an error: %s',
71+
err,
72+
);
73+
}
74+
}
75+
};
76+
}
77+
}
5778
onCommitFiberRoot = (root, expirationTime) => {
5879
try {
5980
const didError = (root.current.effectTag & DidCapture) === DidCapture;
@@ -106,6 +127,12 @@ export function injectInternals(internals: Object): boolean {
106127
return true;
107128
}
108129

130+
export function onScheduleRoot(root: FiberRoot, children: ReactNodeList) {
131+
if (typeof onScheduleFiberRoot === 'function') {
132+
onScheduleFiberRoot(root, children);
133+
}
134+
}
135+
109136
export function onCommitRoot(root: FiberRoot, expirationTime: ExpirationTime) {
110137
if (typeof onCommitFiberRoot === 'function') {
111138
onCommitFiberRoot(root, expirationTime);

packages/react-reconciler/src/ReactFiberInstrumentation.js

Lines changed: 0 additions & 18 deletions
This file was deleted.

packages/react-reconciler/src/ReactFiberReconciler.js

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ import {
4848
isContextProvider as isLegacyContextProvider,
4949
} from './ReactFiberContext';
5050
import {createFiberRoot} from './ReactFiberRoot';
51-
import {injectInternals} from './ReactFiberDevToolsHook';
51+
import {injectInternals, onScheduleRoot} from './ReactFiberDevToolsHook';
5252
import {
5353
requestCurrentTimeForUpdate,
5454
computeExpirationForFiber,
@@ -69,7 +69,6 @@ import {
6969
IsThisRendererActing,
7070
} from './ReactFiberWorkLoop';
7171
import {createUpdate, enqueueUpdate} from './ReactUpdateQueue';
72-
import ReactFiberInstrumentation from './ReactFiberInstrumentation';
7372
import {
7473
getStackByFiberInDevAndProd,
7574
phase as ReactCurrentFiberPhase,
@@ -230,6 +229,9 @@ export function updateContainer(
230229
parentComponent: ?React$Component<any, any>,
231230
callback: ?Function,
232231
): ExpirationTime {
232+
if (__DEV__) {
233+
onScheduleRoot(container, element);
234+
}
233235
const current = container.current;
234236
const currentTime = requestCurrentTimeForUpdate();
235237
if (__DEV__) {
@@ -246,18 +248,6 @@ export function updateContainer(
246248
suspenseConfig,
247249
);
248250

249-
if (__DEV__) {
250-
if (ReactFiberInstrumentation.debugTool) {
251-
if (current.alternate === null) {
252-
ReactFiberInstrumentation.debugTool.onMountContainer(container);
253-
} else if (element === null) {
254-
ReactFiberInstrumentation.debugTool.onUnmountContainer(container);
255-
} else {
256-
ReactFiberInstrumentation.debugTool.onUpdateContainer(container);
257-
}
258-
}
259-
}
260-
261251
const context = getContextForSubtree(parentComponent);
262252
if (container.context === null) {
263253
container.context = context;

packages/react-refresh/src/ReactFreshRuntime.js

Lines changed: 68 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,17 @@ let helpersByRoot: Map<FiberRoot, RendererHelpers> = new Map();
6868

6969
// We keep track of mounted roots so we can schedule updates.
7070
let mountedRoots: Set<FiberRoot> = new Set();
71-
// If a root captures an error, we add its element to this Map so we can retry on edit.
72-
let failedRoots: Map<FiberRoot, ReactNodeList> = new Map();
73-
let didSomeRootFailOnMount = false;
71+
// If a root captures an error, we remember it so we can retry on edit.
72+
let failedRoots: Set<FiberRoot> = new Set();
73+
74+
// In environments that support WeakMap, we also remember the last element for every root.
75+
// It needs to be weak because we do this even for roots that failed to mount.
76+
// If there is no WeakMap, we won't attempt to do retrying.
77+
// $FlowIssue
78+
let rootElements: WeakMap<any, ReactNodeList> | null = // $FlowIssue
79+
typeof WeakMap === 'function' ? new WeakMap() : null;
80+
81+
let isPerformingRefresh = false;
7482

7583
function computeFullKey(signature: Signature): string {
7684
if (signature.fullKey !== null) {
@@ -171,11 +179,20 @@ function cloneSet<T>(set: Set<T>): Set<T> {
171179
}
172180

173181
export function performReactRefresh(): RefreshUpdate | null {
174-
if (__DEV__) {
175-
if (pendingUpdates.length === 0) {
176-
return null;
177-
}
182+
if (!__DEV__) {
183+
throw new Error(
184+
'Unexpected call to React Refresh in a production environment.',
185+
);
186+
}
187+
if (pendingUpdates.length === 0) {
188+
return null;
189+
}
190+
if (isPerformingRefresh) {
191+
return null;
192+
}
178193

194+
isPerformingRefresh = true;
195+
try {
179196
const staleFamilies = new Set();
180197
const updatedFamilies = new Set();
181198

@@ -216,17 +233,27 @@ export function performReactRefresh(): RefreshUpdate | null {
216233
// If we don't do this, there is a risk they will be mutated while
217234
// we iterate over them. For example, trying to recover a failed root
218235
// may cause another root to be added to the failed list -- an infinite loop.
219-
let failedRootsSnapshot = cloneMap(failedRoots);
236+
let failedRootsSnapshot = cloneSet(failedRoots);
220237
let mountedRootsSnapshot = cloneSet(mountedRoots);
221238
let helpersByRootSnapshot = cloneMap(helpersByRoot);
222239

223-
failedRootsSnapshot.forEach((element, root) => {
240+
failedRootsSnapshot.forEach(root => {
224241
const helpers = helpersByRootSnapshot.get(root);
225242
if (helpers === undefined) {
226243
throw new Error(
227244
'Could not find helpers for a root. This is a bug in React Refresh.',
228245
);
229246
}
247+
if (!failedRoots.has(root)) {
248+
// No longer failed.
249+
}
250+
if (rootElements === null) {
251+
return;
252+
}
253+
if (!rootElements.has(root)) {
254+
return;
255+
}
256+
const element = rootElements.get(root);
230257
try {
231258
helpers.scheduleRoot(root, element);
232259
} catch (err) {
@@ -244,6 +271,9 @@ export function performReactRefresh(): RefreshUpdate | null {
244271
'Could not find helpers for a root. This is a bug in React Refresh.',
245272
);
246273
}
274+
if (!mountedRoots.has(root)) {
275+
// No longer mounted.
276+
}
247277
try {
248278
helpers.scheduleRefresh(root, update);
249279
} catch (err) {
@@ -258,10 +288,8 @@ export function performReactRefresh(): RefreshUpdate | null {
258288
throw firstError;
259289
}
260290
return update;
261-
} else {
262-
throw new Error(
263-
'Unexpected call to React Refresh in a production environment.',
264-
);
291+
} finally {
292+
isPerformingRefresh = false;
265293
}
266294
}
267295

@@ -411,6 +439,11 @@ export function injectIntoGlobalHook(globalObject: any): void {
411439
inject(injected) {
412440
return nextID++;
413441
},
442+
onScheduleFiberRoot(
443+
id: number,
444+
root: FiberRoot,
445+
children: ReactNodeList,
446+
) {},
414447
onCommitFiberRoot(
415448
id: number,
416449
root: FiberRoot,
@@ -437,6 +470,22 @@ export function injectIntoGlobalHook(globalObject: any): void {
437470

438471
// We also want to track currently mounted roots.
439472
const oldOnCommitFiberRoot = hook.onCommitFiberRoot;
473+
const oldOnScheduleFiberRoot = hook.onScheduleFiberRoot || (() => {});
474+
hook.onScheduleFiberRoot = function(
475+
id: number,
476+
root: FiberRoot,
477+
children: mixed,
478+
) {
479+
if (!isPerformingRefresh) {
480+
// If it was intentionally scheduled, don't attempt to restore.
481+
// This includes intentionally scheduled unmounts.
482+
failedRoots.delete(root);
483+
if (rootElements !== null) {
484+
rootElements.set(root, children);
485+
}
486+
}
487+
return oldOnScheduleFiberRoot.apply(this, arguments);
488+
};
440489
hook.onCommitFiberRoot = function(
441490
id: number,
442491
root: FiberRoot,
@@ -476,27 +525,14 @@ export function injectIntoGlobalHook(globalObject: any): void {
476525
mountedRoots.delete(root);
477526
if (didError) {
478527
// We'll remount it on future edits.
479-
// Remember what was rendered so we can restore it.
480-
failedRoots.set(root, alternate.memoizedState.element);
528+
failedRoots.add(root);
481529
} else {
482530
helpersByRoot.delete(root);
483531
}
484532
} else if (!wasMounted && !isMounted) {
485-
if (didError && !failedRoots.has(root)) {
486-
// The root had an error during the initial mount.
487-
// We can't read its last element from the memoized state
488-
// because there was no previously committed alternate.
489-
// Ideally, it would be nice if we had a way to extract
490-
// the last attempted rendered element, but accessing the update queue
491-
// would tie this package too closely to the reconciler version.
492-
// So instead, we just set a flag.
493-
// TODO: Maybe we could fix this as the same time as when we fix
494-
// DevTools to not depend on `alternate.memoizedState.element`.
495-
didSomeRootFailOnMount = true;
496-
} else if (!didError && failedRoots.has(root)) {
497-
// The error is fixed but the component is still unmounted.
498-
// This means that the unmount was not caused by a failed refresh.
499-
failedRoots.delete(root);
533+
if (didError) {
534+
// We'll remount it on future edits.
535+
failedRoots.add(root);
500536
}
501537
}
502538
} else {
@@ -514,7 +550,8 @@ export function injectIntoGlobalHook(globalObject: any): void {
514550
}
515551

516552
export function hasUnrecoverableErrors() {
517-
return didSomeRootFailOnMount;
553+
// TODO: delete this after removing dependency in RN.
554+
return false;
518555
}
519556

520557
// Exposed for testing.

0 commit comments

Comments
 (0)