[Fresh] Retry failed roots on refresh (#15966)
* Retry failed roots on refresh * Don't prevent retry after error -> render(null) special case The check wasn't very resilient because in Concurrent Mode it looks like we can get further follow-up commits even if we captured an error. So we can't reliably distinguish the case where after an error you _manually_ rendered null. Retrying on an edit after a tree failed _and_ you rendered null in the same tree seems fine. It's also very unlikely a pattern like this actually exists in the wild.
Dan Abramov committed
Jun 24, 2019 at 16:54 UTC
d420d2ccb6223a66d5e8fe824ac0d31ed5bf87a1
5 files changed
+165
-6
packages/react-reconciler/src/ReactFiberDevToolsHook.js
+4
-2
@@ -15,6 +15,7 @@ import type {Fiber} from './ReactFiber';
15
import type {FiberRoot} from './ReactFiberRoot';
16
import type {ExpirationTime} from './ReactFiberExpirationTime';
17
18
+import {DidCapture} from 'shared/ReactSideEffectTags';
19
import warningWithoutStack from 'shared/warningWithoutStack';
20
21
declare var __REACT_DEVTOOLS_GLOBAL_HOOK__: Object | void;
@@ -55,15 +56,16 @@ export function injectInternals(internals: Object): boolean {
56
// We have successfully injected, so now it is safe to set up hooks.
57
onCommitFiberRoot = (root, expirationTime) => {
58
try {
59
+ const didError = (root.current.effectTag & DidCapture) === DidCapture;
60
if (enableProfilerTimer) {
61
const currentTime = requestCurrentTime();
62
const priorityLevel = inferPriorityFromExpirationTime(
63
currentTime,
64
expirationTime,
65
);
64
- hook.onCommitFiberRoot(rendererID, root, priorityLevel);
66
+ hook.onCommitFiberRoot(rendererID, root, priorityLevel, didError);
67
} else {
66
- hook.onCommitFiberRoot(rendererID, root);
68
+ hook.onCommitFiberRoot(rendererID, root, undefined, didError);
69
}
70
} catch (err) {
71
if (__DEV__ && !hasLoggedError) {
packages/react-reconciler/src/ReactFiberHotReloading.js
+20
@@ -11,12 +11,15 @@ import type {ReactElement} from 'shared/ReactElementType';
11
import type {Fiber} from './ReactFiber';
12
import type {FiberRoot} from './ReactFiberRoot';
13
import type {Instance} from './ReactFiberHostConfig';
14
+import type {ReactNodeList} from 'shared/ReactTypes';
15
16
import {
17
flushSync,
18
scheduleWork,
19
flushPassiveEffects,
20
} from './ReactFiberWorkLoop';
21
+import {updateContainerAtExpirationTime} from './ReactFiberReconciler';
22
+import {emptyContextObject} from './ReactFiberContext';
23
import {Sync} from './ReactFiberExpirationTime';
24
import {
25
ClassComponent,
@@ -49,6 +52,7 @@ type RefreshHandler = any => Family | void;
52
// Used by React Refresh runtime through DevTools Global Hook.
53
export type SetRefreshHandler = (handler: RefreshHandler | null) => void;
54
export type ScheduleRefresh = (root: FiberRoot, update: RefreshUpdate) => void;
55
+export type ScheduleRoot = (root: FiberRoot, element: ReactNodeList) => void;
56
export type FindHostInstancesForRefresh = (
57
root: FiberRoot,
58
families: Array<Family>,
@@ -242,6 +246,22 @@ export let scheduleRefresh: ScheduleRefresh = (
246
}
247
};
248
249
+export let scheduleRoot: ScheduleRoot = (
250
+ root: FiberRoot,
251
+ element: ReactNodeList,
252
+): void => {
253
+ if (__DEV__) {
254
+ if (root.context !== emptyContextObject) {
255
+ // Super edge case: root has a legacy _renderSubtree context
256
+ // but we don't know the parentComponent so we can't pass it.
257
+ // Just ignore. We'll delete this with _renderSubtree code path later.
258
+ return;
259
+ }
260
+ flushPassiveEffects();
261
+ updateContainerAtExpirationTime(element, root, null, Sync, null);
262
+ }
263
+};
264
+
265
function scheduleFibersWithFamiliesRecursively(
266
fiber: Fiber,
267
updatedFamilies: Set<Family>,
packages/react-reconciler/src/ReactFiberReconciler.js
+2
@@ -72,6 +72,7 @@ import {revertPassiveEffectsChange} from 'shared/ReactFeatureFlags';
72
import {requestCurrentSuspenseConfig} from './ReactFiberSuspenseConfig';
73
import {
74
scheduleRefresh,
75
+ scheduleRoot,
76
setRefreshHandler,
77
findHostInstancesForRefresh,
78
} from './ReactFiberHotReloading';
@@ -498,6 +499,7 @@ export function injectIntoDevTools(devToolsConfig: DevToolsConfig): boolean {
499
// React Refresh
500
findHostInstancesForRefresh: __DEV__ ? findHostInstancesForRefresh : null,
501
scheduleRefresh: __DEV__ ? scheduleRefresh : null,
502
+ scheduleRoot: __DEV__ ? scheduleRoot : null,
503
setRefreshHandler: __DEV__ ? setRefreshHandler : null,
504
});
505
}
packages/react-refresh/src/ReactFreshRuntime.js
+49
-4
@@ -13,9 +13,11 @@ import type {
13
Family,
14
RefreshUpdate,
15
ScheduleRefresh,
16
+ ScheduleRoot,
17
FindHostInstancesForRefresh,
18
SetRefreshHandler,
19
} from 'react-reconciler/src/ReactFiberHotReloading';
20
+import type {ReactNodeList} from 'shared/ReactTypes';
21
22
import {REACT_MEMO_TYPE, REACT_FORWARD_REF_TYPE} from 'shared/ReactSymbols';
23
import warningWithoutStack from 'shared/warningWithoutStack';
@@ -57,9 +59,13 @@ let pendingUpdates: Array<[Family, any]> = [];
59
// This is injected by the renderer via DevTools global hook.
60
let setRefreshHandler: null | SetRefreshHandler = null;
61
let scheduleRefresh: null | ScheduleRefresh = null;
62
+let scheduleRoot: null | ScheduleRoot = null;
63
let findHostInstancesForRefresh: null | FindHostInstancesForRefresh = null;
64
62
-let mountedRoots = new Set();
65
+// We keep track of mounted roots so we can schedule updates.
66
+let mountedRoots: Set<FiberRoot> = new Set();
67
+// If a root captures an error, we add its element to this Map so we can retry on edit.
68
+let failedRoots: Map<FiberRoot, ReactNodeList> = new Map();
69
70
function computeFullKey(signature: Signature): string {
71
if (signature.fullKey !== null) {
@@ -196,7 +202,18 @@ export function performReactRefresh(): RefreshUpdate | null {
202
);
203
return null;
204
}
205
+ if (typeof scheduleRoot !== 'function') {
206
+ warningWithoutStack(
207
+ false,
208
+ 'Could not find the scheduleRoot() implementation. ' +
209
+ 'This likely means that injectIntoGlobalHook() was either ' +
210
+ 'called before the global DevTools hook was set up, or after the ' +
211
+ 'renderer has already initialized. Please file an issue with a reproducing case.',
212
+ );
213
+ return null;
214
+ }
215
const scheduleRefreshForRoot = scheduleRefresh;
216
+ const scheduleRenderForRoot = scheduleRoot;
217
218
// Even if there are no roots, set the handler on first update.
219
// This ensures that if *new* roots are mounted, they'll use the resolve handler.
@@ -204,6 +221,17 @@ export function performReactRefresh(): RefreshUpdate | null {
221
222
let didError = false;
223
let firstError = null;
224
+ failedRoots.forEach((element, root) => {
225
+ try {
226
+ scheduleRenderForRoot(root, element);
227
+ } catch (err) {
228
+ if (!didError) {
229
+ didError = true;
230
+ firstError = err;
231
+ }
232
+ // Keep trying other roots.
233
+ }
234
+ });
235
mountedRoots.forEach(root => {
236
try {
237
scheduleRefreshForRoot(root, update);
@@ -245,7 +273,7 @@ export function register(type: any, id: string): void {
273
274
// Create family or remember to update it.
275
// None of this bookkeeping affects reconciliation
248
- // until the first prepareUpdate() call above.
276
+ // until the first performReactRefresh() call above.
277
let family = allFamiliesByID.get(id);
278
if (family === undefined) {
279
family = {current: type};
@@ -362,7 +390,12 @@ export function injectIntoGlobalHook(globalObject: any): void {
390
globalObject.__REACT_DEVTOOLS_GLOBAL_HOOK__ = hook = {
391
supportsFiber: true,
392
inject() {},
365
- onCommitFiberRoot(id: mixed, root: FiberRoot) {},
393
+ onCommitFiberRoot(
394
+ id: mixed,
395
+ root: FiberRoot,
396
+ maybePriorityLevel: mixed,
397
+ didError: boolean,
398
+ ) {},
399
onCommitFiberUnmount() {},
400
};
401
}
@@ -373,6 +406,7 @@ export function injectIntoGlobalHook(globalObject: any): void {
406
findHostInstancesForRefresh = ((injected: any)
407
.findHostInstancesForRefresh: FindHostInstancesForRefresh);
408
scheduleRefresh = ((injected: any).scheduleRefresh: ScheduleRefresh);
409
+ scheduleRoot = ((injected: any).scheduleRoot: ScheduleRoot);
410
setRefreshHandler = ((injected: any)
411
.setRefreshHandler: SetRefreshHandler);
412
return oldInject.apply(this, arguments);
@@ -380,7 +414,12 @@ export function injectIntoGlobalHook(globalObject: any): void {
414
415
// We also want to track currently mounted roots.
416
const oldOnCommitFiberRoot = hook.onCommitFiberRoot;
383
- hook.onCommitFiberRoot = function(id: mixed, root: FiberRoot) {
417
+ hook.onCommitFiberRoot = function(
418
+ id: mixed,
419
+ root: FiberRoot,
420
+ maybePriorityLevel: mixed,
421
+ didError: boolean,
422
+ ) {
423
const current = root.current;
424
const alternate = current.alternate;
425
@@ -399,12 +438,18 @@ export function injectIntoGlobalHook(globalObject: any): void {
438
if (!wasMounted && isMounted) {
439
// Mount a new root.
440
mountedRoots.add(root);
441
+ failedRoots.delete(root);
442
} else if (wasMounted && isMounted) {
443
// Update an existing root.
444
// This doesn't affect our mounted root Set.
445
} else if (wasMounted && !isMounted) {
446
// Unmount an existing root.
447
mountedRoots.delete(root);
448
+ if (didError) {
449
+ // We'll remount it on future edits.
450
+ // Remember what was rendered so we can restore it.
451
+ failedRoots.set(root, alternate.memoizedState.element);
452
+ }
453
}
454
} else {
455
// Mount a new root.
packages/react-refresh/src/__tests__/ReactFresh-test.js
+90
@@ -2712,6 +2712,96 @@ describe('ReactFresh', () => {
2712
}
2713
});
2714
2715
+ it('remounts a failed root on update', () => {
2716
+ if (__DEV__) {
2717
+ render(() => {
2718
+ function Hello() {
2719
+ return <h1>Hi</h1>;
2720
+ }
2721
+ $RefreshReg$(Hello, 'Hello');
2722
+
2723
+ return Hello;
2724
+ });
2725
+ expect(container.innerHTML).toBe('<h1>Hi</h1>');
2726
+
2727
+ // Perform a hot update that fails.
2728
+ // This removes the root.
2729
+ expect(() => {
2730
+ patch(() => {
2731
+ function Hello() {
2732
+ throw new Error('No');
2733
+ }
2734
+ $RefreshReg$(Hello, 'Hello');
2735
+ });
2736
+ }).toThrow('No');
2737
+ expect(container.innerHTML).toBe('');
2738
+
2739
+ // A bad retry
2740
+ expect(() => {
2741
+ patch(() => {
2742
+ function Hello() {
2743
+ throw new Error('Not yet');
2744
+ }
2745
+ $RefreshReg$(Hello, 'Hello');
2746
+ });
2747
+ }).toThrow('Not yet');
2748
+ expect(container.innerHTML).toBe('');
2749
+
2750
+ // Perform a hot update that fixes the error.
2751
+ patch(() => {
2752
+ function Hello() {
2753
+ return <h1>Fixed!</h1>;
2754
+ }
2755
+ $RefreshReg$(Hello, 'Hello');
2756
+ });
2757
+ // This should remount the root.
2758
+ expect(container.innerHTML).toBe('<h1>Fixed!</h1>');
2759
+
2760
+ // Verify next hot reload doesn't remount anything.
2761
+ let helloNode = container.firstChild;
2762
+ patch(() => {
2763
+ function Hello() {
2764
+ return <h1>Nice.</h1>;
2765
+ }
2766
+ $RefreshReg$(Hello, 'Hello');
2767
+ });
2768
+ expect(container.firstChild).toBe(helloNode);
2769
+ expect(helloNode.textContent).toBe('Nice.');
2770
+
2771
+ // Break again.
2772
+ expect(() => {
2773
+ patch(() => {
2774
+ function Hello() {
2775
+ throw new Error('Oops');
2776
+ }
2777
+ $RefreshReg$(Hello, 'Hello');
2778
+ });
2779
+ }).toThrow('Oops');
2780
+ expect(container.innerHTML).toBe('');
2781
+
2782
+ // Perform a hot update that fixes the error.
2783
+ patch(() => {
2784
+ function Hello() {
2785
+ return <h1>At last.</h1>;
2786
+ }
2787
+ $RefreshReg$(Hello, 'Hello');
2788
+ });
2789
+ // This should remount the root.
2790
+ expect(container.innerHTML).toBe('<h1>At last.</h1>');
2791
+
2792
+ // Check we don't attempt to reverse an intentional unmount.
2793
+ ReactDOM.unmountComponentAtNode(container);
2794
+ expect(container.innerHTML).toBe('');
2795
+ patch(() => {
2796
+ function Hello() {
2797
+ return <h1>Never mind me!</h1>;
2798
+ }
2799
+ $RefreshReg$(Hello, 'Hello');
2800
+ });
2801
+ expect(container.innerHTML).toBe('');
2802
+ }
2803
+ });
2804
+
2805
it('remounts classes on every edit', () => {
2806
if (__DEV__) {
2807
let HelloV1 = render(() => {