@samitouri / QOS-React / commits / 6470e0f169

[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

Dan Abramov committed Nov 25, 2019 at 17:25 UTC 6470e0f169b4cf0416132aa66221bf20e264c618
5 files changed +195 -80
packages/react-reconciler/src/ReactFiberDevToolsHook.js
+27
@@ -14,12 +14,14 @@ import {inferPriorityFromExpirationTime} from './ReactFiberExpirationTime';
14 import type {Fiber} from './ReactFiber';
15 import type {FiberRoot} from './ReactFiberRoot';
16 import type {ExpirationTime} from './ReactFiberExpirationTime';
17 +import type {ReactNodeList} from 'shared/ReactTypes';
18
19 import {DidCapture} from 'shared/ReactSideEffectTags';
20 import warningWithoutStack from 'shared/warningWithoutStack';
21
22 declare var __REACT_DEVTOOLS_GLOBAL_HOOK__: Object | void;
23
24 +let onScheduleFiberRoot = null;
25 let onCommitFiberRoot = null;
26 let onCommitFiberUnmount = null;
27 let hasLoggedError = false;
@@ -54,6 +56,25 @@ export function injectInternals(internals: Object): boolean {
56 try {
57 const rendererID = hook.inject(internals);
58 // 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 + }
78 onCommitFiberRoot = (root, expirationTime) => {
79 try {
80 const didError = (root.current.effectTag & DidCapture) === DidCapture;
@@ -106,6 +127,12 @@ export function injectInternals(internals: Object): boolean {
127 return true;
128 }
129
130 +export function onScheduleRoot(root: FiberRoot, children: ReactNodeList) {
131 + if (typeof onScheduleFiberRoot === 'function') {
132 + onScheduleFiberRoot(root, children);
133 + }
134 +}
135 +
136 export function onCommitRoot(root: FiberRoot, expirationTime: ExpirationTime) {
137 if (typeof onCommitFiberRoot === 'function') {
138 onCommitFiberRoot(root, expirationTime);
packages/react-reconciler/src/ReactFiberInstrumentation.js deleted
-18
@@ -1,18 +0,0 @@
1 -/**
2 - * Copyright (c) Facebook, Inc. and its affiliates.
3 - *
4 - * This source code is licensed under the MIT license found in the
5 - * LICENSE file in the root directory of this source tree.
6 - *
7 - * @flow
8 - */
9 -
10 -// This lets us hook into Fiber to debug what it's doing.
11 -// See https://github.com/facebook/react/pull/8033.
12 -// This is not part of the public API, not even for React DevTools.
13 -// You may only inject a debugTool if you work on React Fiber itself.
14 -const ReactFiberInstrumentation = {
15 - debugTool: null,
16 -};
17 -
18 -module.exports = ReactFiberInstrumentation;
packages/react-reconciler/src/ReactFiberReconciler.js
+4 -14
@@ -48,7 +48,7 @@ import {
48 isContextProvider as isLegacyContextProvider,
49 } from './ReactFiberContext';
50 import {createFiberRoot} from './ReactFiberRoot';
51 -import {injectInternals} from './ReactFiberDevToolsHook';
51 +import {injectInternals, onScheduleRoot} from './ReactFiberDevToolsHook';
52 import {
53 requestCurrentTimeForUpdate,
54 computeExpirationForFiber,
@@ -69,7 +69,6 @@ import {
69 IsThisRendererActing,
70 } from './ReactFiberWorkLoop';
71 import {createUpdate, enqueueUpdate} from './ReactUpdateQueue';
72 -import ReactFiberInstrumentation from './ReactFiberInstrumentation';
72 import {
73 getStackByFiberInDevAndProd,
74 phase as ReactCurrentFiberPhase,
@@ -230,6 +229,9 @@ export function updateContainer(
229 parentComponent: ?React$Component<any, any>,
230 callback: ?Function,
231 ): ExpirationTime {
232 + if (__DEV__) {
233 + onScheduleRoot(container, element);
234 + }
235 const current = container.current;
236 const currentTime = requestCurrentTimeForUpdate();
237 if (__DEV__) {
@@ -246,18 +248,6 @@ export function updateContainer(
248 suspenseConfig,
249 );
250
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 -
251 const context = getContextForSubtree(parentComponent);
252 if (container.context === null) {
253 container.context = context;
packages/react-refresh/src/ReactFreshRuntime.js
+68 -31
@@ -68,9 +68,17 @@ let helpersByRoot: Map<FiberRoot, RendererHelpers> = new Map();
68
69 // We keep track of mounted roots so we can schedule updates.
70 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;
82
83 function computeFullKey(signature: Signature): string {
84 if (signature.fullKey !== null) {
@@ -171,11 +179,20 @@ function cloneSet<T>(set: Set<T>): Set<T> {
179 }
180
181 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 + }
193
194 + isPerformingRefresh = true;
195 + try {
196 const staleFamilies = new Set();
197 const updatedFamilies = new Set();
198
@@ -216,17 +233,27 @@ export function performReactRefresh(): RefreshUpdate | null {
233 // If we don't do this, there is a risk they will be mutated while
234 // we iterate over them. For example, trying to recover a failed root
235 // may cause another root to be added to the failed list -- an infinite loop.
219 - let failedRootsSnapshot = cloneMap(failedRoots);
236 + let failedRootsSnapshot = cloneSet(failedRoots);
237 let mountedRootsSnapshot = cloneSet(mountedRoots);
238 let helpersByRootSnapshot = cloneMap(helpersByRoot);
239
223 - failedRootsSnapshot.forEach((element, root) => {
240 + failedRootsSnapshot.forEach(root => {
241 const helpers = helpersByRootSnapshot.get(root);
242 if (helpers === undefined) {
243 throw new Error(
244 'Could not find helpers for a root. This is a bug in React Refresh.',
245 );
246 }
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);
257 try {
258 helpers.scheduleRoot(root, element);
259 } catch (err) {
@@ -244,6 +271,9 @@ export function performReactRefresh(): RefreshUpdate | null {
271 'Could not find helpers for a root. This is a bug in React Refresh.',
272 );
273 }
274 + if (!mountedRoots.has(root)) {
275 + // No longer mounted.
276 + }
277 try {
278 helpers.scheduleRefresh(root, update);
279 } catch (err) {
@@ -258,10 +288,8 @@ export function performReactRefresh(): RefreshUpdate | null {
288 throw firstError;
289 }
290 return update;
261 - } else {
262 - throw new Error(
263 - 'Unexpected call to React Refresh in a production environment.',
264 - );
291 + } finally {
292 + isPerformingRefresh = false;
293 }
294 }
295
@@ -411,6 +439,11 @@ export function injectIntoGlobalHook(globalObject: any): void {
439 inject(injected) {
440 return nextID++;
441 },
442 + onScheduleFiberRoot(
443 + id: number,
444 + root: FiberRoot,
445 + children: ReactNodeList,
446 + ) {},
447 onCommitFiberRoot(
448 id: number,
449 root: FiberRoot,
@@ -437,6 +470,22 @@ export function injectIntoGlobalHook(globalObject: any): void {
470
471 // We also want to track currently mounted roots.
472 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 + };
489 hook.onCommitFiberRoot = function(
490 id: number,
491 root: FiberRoot,
@@ -476,27 +525,14 @@ export function injectIntoGlobalHook(globalObject: any): void {
525 mountedRoots.delete(root);
526 if (didError) {
527 // 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);
529 } else {
530 helpersByRoot.delete(root);
531 }
532 } 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);
536 }
537 }
538 } else {
@@ -514,7 +550,8 @@ export function injectIntoGlobalHook(globalObject: any): void {
550 }
551
552 export function hasUnrecoverableErrors() {
517 - return didSomeRootFailOnMount;
553 + // TODO: delete this after removing dependency in RN.
554 + return false;
555 }
556
557 // Exposed for testing.
packages/react-refresh/src/__tests__/ReactFresh-test.js
+96 -17
@@ -2755,11 +2755,8 @@ describe('ReactFresh', () => {
2755 }
2756 });
2757
2758 - // TODO: we can make this recoverable in the future
2759 - // if we add a way to track the last attempted element.
2760 - it('records an unrecoverable error if a root fails on mount', () => {
2758 + it('remounts a failed root on mount', () => {
2759 if (__DEV__) {
2762 - expect(ReactFreshRuntime.hasUnrecoverableErrors()).toBe(false);
2760 expect(() => {
2761 render(() => {
2762 function Hello() {
@@ -2770,13 +2767,106 @@ describe('ReactFresh', () => {
2767 return Hello;
2768 });
2769 }).toThrow('No');
2773 - expect(ReactFreshRuntime.hasUnrecoverableErrors()).toBe(true);
2770 + expect(container.innerHTML).toBe('');
2771 +
2772 + // A bad retry
2773 + expect(() => {
2774 + patch(() => {
2775 + function Hello() {
2776 + throw new Error('Not yet');
2777 + }
2778 + $RefreshReg$(Hello, 'Hello');
2779 + });
2780 + }).toThrow('Not yet');
2781 + expect(container.innerHTML).toBe('');
2782 +
2783 + // Perform a hot update that fixes the error.
2784 + patch(() => {
2785 + function Hello() {
2786 + return <h1>Fixed!</h1>;
2787 + }
2788 + $RefreshReg$(Hello, 'Hello');
2789 + });
2790 + // This should mount the root.
2791 + expect(container.innerHTML).toBe('<h1>Fixed!</h1>');
2792 +
2793 + // Ensure we can keep failing and recovering later.
2794 + expect(() => {
2795 + patch(() => {
2796 + function Hello() {
2797 + throw new Error('No 2');
2798 + }
2799 + $RefreshReg$(Hello, 'Hello');
2800 + });
2801 + }).toThrow('No 2');
2802 + expect(container.innerHTML).toBe('');
2803 + expect(() => {
2804 + patch(() => {
2805 + function Hello() {
2806 + throw new Error('Not yet 2');
2807 + }
2808 + $RefreshReg$(Hello, 'Hello');
2809 + });
2810 + }).toThrow('Not yet 2');
2811 + expect(container.innerHTML).toBe('');
2812 + patch(() => {
2813 + function Hello() {
2814 + return <h1>Fixed 2!</h1>;
2815 + }
2816 + $RefreshReg$(Hello, 'Hello');
2817 + });
2818 + expect(container.innerHTML).toBe('<h1>Fixed 2!</h1>');
2819 +
2820 + // Updates after intentional unmount are ignored.
2821 + ReactDOM.unmountComponentAtNode(container);
2822 + patch(() => {
2823 + function Hello() {
2824 + throw new Error('Ignored');
2825 + }
2826 + $RefreshReg$(Hello, 'Hello');
2827 + });
2828 + expect(container.innerHTML).toBe('');
2829 + patch(() => {
2830 + function Hello() {
2831 + return <h1>Ignored</h1>;
2832 + }
2833 + $RefreshReg$(Hello, 'Hello');
2834 + });
2835 + expect(container.innerHTML).toBe('');
2836 + }
2837 + });
2838 +
2839 + it('does not retry an intentionally unmounted failed root', () => {
2840 + if (__DEV__) {
2841 + expect(() => {
2842 + render(() => {
2843 + function Hello() {
2844 + throw new Error('No');
2845 + }
2846 + $RefreshReg$(Hello, 'Hello');
2847 +
2848 + return Hello;
2849 + });
2850 + }).toThrow('No');
2851 + expect(container.innerHTML).toBe('');
2852 +
2853 + // Intentional unmount.
2854 + ReactDOM.unmountComponentAtNode(container);
2855 +
2856 + // Perform a hot update that fixes the error.
2857 + patch(() => {
2858 + function Hello() {
2859 + return <h1>Fixed!</h1>;
2860 + }
2861 + $RefreshReg$(Hello, 'Hello');
2862 + });
2863 + // This should stay unmounted.
2864 + expect(container.innerHTML).toBe('');
2865 }
2866 });
2867
2868 it('remounts a failed root on update', () => {
2869 if (__DEV__) {
2779 - expect(ReactFreshRuntime.hasUnrecoverableErrors()).toBe(false);
2870 render(() => {
2871 function Hello() {
2872 return <h1>Hi</h1>;
@@ -2786,7 +2876,6 @@ describe('ReactFresh', () => {
2876 return Hello;
2877 });
2878 expect(container.innerHTML).toBe('<h1>Hi</h1>');
2789 - expect(ReactFreshRuntime.hasUnrecoverableErrors()).toBe(false);
2879
2880 // Perform a hot update that fails.
2881 // This removes the root.
@@ -2799,7 +2888,6 @@ describe('ReactFresh', () => {
2888 });
2889 }).toThrow('No');
2890 expect(container.innerHTML).toBe('');
2802 - expect(ReactFreshRuntime.hasUnrecoverableErrors()).toBe(false);
2891
2892 // A bad retry
2893 expect(() => {
@@ -2811,7 +2899,6 @@ describe('ReactFresh', () => {
2899 });
2900 }).toThrow('Not yet');
2901 expect(container.innerHTML).toBe('');
2814 - expect(ReactFreshRuntime.hasUnrecoverableErrors()).toBe(false);
2902
2903 // Perform a hot update that fixes the error.
2904 patch(() => {
@@ -2822,7 +2909,6 @@ describe('ReactFresh', () => {
2909 });
2910 // This should remount the root.
2911 expect(container.innerHTML).toBe('<h1>Fixed!</h1>');
2825 - expect(ReactFreshRuntime.hasUnrecoverableErrors()).toBe(false);
2912
2913 // Verify next hot reload doesn't remount anything.
2914 let helloNode = container.firstChild;
@@ -2834,7 +2920,6 @@ describe('ReactFresh', () => {
2920 });
2921 expect(container.firstChild).toBe(helloNode);
2922 expect(helloNode.textContent).toBe('Nice.');
2837 - expect(ReactFreshRuntime.hasUnrecoverableErrors()).toBe(false);
2923
2924 // Break again.
2925 expect(() => {
@@ -2846,7 +2931,6 @@ describe('ReactFresh', () => {
2931 });
2932 }).toThrow('Oops');
2933 expect(container.innerHTML).toBe('');
2849 - expect(ReactFreshRuntime.hasUnrecoverableErrors()).toBe(false);
2934
2935 // Perform a hot update that fixes the error.
2936 patch(() => {
@@ -2857,7 +2941,6 @@ describe('ReactFresh', () => {
2941 });
2942 // This should remount the root.
2943 expect(container.innerHTML).toBe('<h1>At last.</h1>');
2860 - expect(ReactFreshRuntime.hasUnrecoverableErrors()).toBe(false);
2944
2945 // Check we don't attempt to reverse an intentional unmount.
2946 ReactDOM.unmountComponentAtNode(container);
@@ -2869,7 +2952,6 @@ describe('ReactFresh', () => {
2952 $RefreshReg$(Hello, 'Hello');
2953 });
2954 expect(container.innerHTML).toBe('');
2872 - expect(ReactFreshRuntime.hasUnrecoverableErrors()).toBe(false);
2955
2956 // Mount a new container.
2957 render(() => {
@@ -2881,7 +2963,6 @@ describe('ReactFresh', () => {
2963 return Hello;
2964 });
2965 expect(container.innerHTML).toBe('<h1>Hi</h1>');
2884 - expect(ReactFreshRuntime.hasUnrecoverableErrors()).toBe(false);
2966
2967 // Break again.
2968 expect(() => {
@@ -2893,7 +2974,6 @@ describe('ReactFresh', () => {
2974 });
2975 }).toThrow('Oops');
2976 expect(container.innerHTML).toBe('');
2896 - expect(ReactFreshRuntime.hasUnrecoverableErrors()).toBe(false);
2977
2978 // Check we don't attempt to reverse an intentional unmount, even after an error.
2979 ReactDOM.unmountComponentAtNode(container);
@@ -2905,7 +2985,6 @@ describe('ReactFresh', () => {
2985 $RefreshReg$(Hello, 'Hello');
2986 });
2987 expect(container.innerHTML).toBe('');
2908 - expect(ReactFreshRuntime.hasUnrecoverableErrors()).toBe(false);
2988 }
2989 });
2990