Fast Refresh: Don't block DevTools commit hook (#20129)
In some scenarios (either timing dependent, or pre-FR compatible React versions) FR blocked calling the React DevTools commit hook. This PR adds a test and a fix for that.
Brian Vaughn committed
Oct 29, 2020 at 13:23 UTC
343d7a4a7eb621a31ba946309b557e0988be3a63
4 files changed
+134
-59
packages/react-refresh/package.json
+5
-1
@@ -25,5 +25,9 @@
25
},
26
"engines": {
27
"node": ">=0.10.0"
28
+ },
29
+ "devDependencies": {
30
+ "react-16-8": "npm:react@16.8.0",
31
+ "react-dom-16-8": "npm:react-dom@16.8.0"
32
}
29
-}
33
+}
\ No newline at end of file
packages/react-refresh/src/ReactFreshRuntime.js
+42
-42
@@ -517,53 +517,53 @@ export function injectIntoGlobalHook(globalObject: any): void {
517
didError: boolean,
518
) {
519
const helpers = helpersByRendererID.get(id);
520
- if (helpers === undefined) {
521
- return;
522
- }
523
- helpersByRoot.set(root, helpers);
524
-
525
- const current = root.current;
526
- const alternate = current.alternate;
527
-
528
- // We need to determine whether this root has just (un)mounted.
529
- // This logic is copy-pasted from similar logic in the DevTools backend.
530
- // If this breaks with some refactoring, you'll want to update DevTools too.
531
-
532
- if (alternate !== null) {
533
- const wasMounted =
534
- alternate.memoizedState != null &&
535
- alternate.memoizedState.element != null;
536
- const isMounted =
537
- current.memoizedState != null &&
538
- current.memoizedState.element != null;
539
-
540
- if (!wasMounted && isMounted) {
520
+ if (helpers !== undefined) {
521
+ helpersByRoot.set(root, helpers);
522
+
523
+ const current = root.current;
524
+ const alternate = current.alternate;
525
+
526
+ // We need to determine whether this root has just (un)mounted.
527
+ // This logic is copy-pasted from similar logic in the DevTools backend.
528
+ // If this breaks with some refactoring, you'll want to update DevTools too.
529
+
530
+ if (alternate !== null) {
531
+ const wasMounted =
532
+ alternate.memoizedState != null &&
533
+ alternate.memoizedState.element != null;
534
+ const isMounted =
535
+ current.memoizedState != null &&
536
+ current.memoizedState.element != null;
537
+
538
+ if (!wasMounted && isMounted) {
539
+ // Mount a new root.
540
+ mountedRoots.add(root);
541
+ failedRoots.delete(root);
542
+ } else if (wasMounted && isMounted) {
543
+ // Update an existing root.
544
+ // This doesn't affect our mounted root Set.
545
+ } else if (wasMounted && !isMounted) {
546
+ // Unmount an existing root.
547
+ mountedRoots.delete(root);
548
+ if (didError) {
549
+ // We'll remount it on future edits.
550
+ failedRoots.add(root);
551
+ } else {
552
+ helpersByRoot.delete(root);
553
+ }
554
+ } else if (!wasMounted && !isMounted) {
555
+ if (didError) {
556
+ // We'll remount it on future edits.
557
+ failedRoots.add(root);
558
+ }
559
+ }
560
+ } else {
561
// Mount a new root.
562
mountedRoots.add(root);
543
- failedRoots.delete(root);
544
- } else if (wasMounted && isMounted) {
545
- // Update an existing root.
546
- // This doesn't affect our mounted root Set.
547
- } else if (wasMounted && !isMounted) {
548
- // Unmount an existing root.
549
- mountedRoots.delete(root);
550
- if (didError) {
551
- // We'll remount it on future edits.
552
- failedRoots.add(root);
553
- } else {
554
- helpersByRoot.delete(root);
555
- }
556
- } else if (!wasMounted && !isMounted) {
557
- if (didError) {
558
- // We'll remount it on future edits.
559
- failedRoots.add(root);
560
- }
563
}
562
- } else {
563
- // Mount a new root.
564
- mountedRoots.add(root);
564
}
565
566
+ // Always call the decorated DevTools hook.
567
return oldOnCommitFiberRoot.apply(this, arguments);
568
};
569
} else {
packages/react-refresh/src/__tests__/ReactFresh-test.js
+59
-16
@@ -3745,24 +3745,32 @@ describe('ReactFresh', () => {
3745
}
3746
});
3747
3748
- // This simulates the scenario in https://github.com/facebook/react/issues/17626.
3748
+ function initFauxDevToolsHook() {
3749
+ const onCommitFiberRoot = jest.fn();
3750
+ const onCommitFiberUnmount = jest.fn();
3751
+
3752
+ let idCounter = 0;
3753
+ const renderers = new Map();
3754
+
3755
+ // This is a minimal shim for the global hook installed by DevTools.
3756
+ // The real one is in packages/react-devtools-shared/src/hook.js.
3757
+ global.__REACT_DEVTOOLS_GLOBAL_HOOK__ = {
3758
+ renderers,
3759
+ supportsFiber: true,
3760
+ inject(renderer) {
3761
+ const id = ++idCounter;
3762
+ renderers.set(id, renderer);
3763
+ return id;
3764
+ },
3765
+ onCommitFiberRoot,
3766
+ onCommitFiberUnmount,
3767
+ };
3768
+ }
3769
+
3770
+ // This simulates the scenario in https://github.com/facebook/react/issues/17626
3771
it('can inject the runtime after the renderer executes', () => {
3772
if (__DEV__) {
3751
- // This is a minimal shim for the global hook installed by DevTools.
3752
- // The real one is in packages/react-devtools-shared/src/hook.js.
3753
- let idCounter = 0;
3754
- const renderers = new Map();
3755
- global.__REACT_DEVTOOLS_GLOBAL_HOOK__ = {
3756
- renderers,
3757
- supportsFiber: true,
3758
- inject(renderer) {
3759
- const id = ++idCounter;
3760
- renderers.set(id, renderer);
3761
- return id;
3762
- },
3763
- onCommitFiberRoot() {},
3764
- onCommitFiberUnmount() {},
3765
- };
3773
+ initFauxDevToolsHook();
3774
3775
// Load these first, as if they're coming from a CDN.
3776
jest.resetModules();
@@ -3820,4 +3828,39 @@ describe('ReactFresh', () => {
3828
expect(el.style.color).toBe('red');
3829
}
3830
});
3831
+
3832
+ // This simulates the scenario in https://github.com/facebook/react/issues/20100
3833
+ it('does not block DevTools when an unsupported renderer is injected', () => {
3834
+ if (__DEV__) {
3835
+ initFauxDevToolsHook();
3836
+
3837
+ const onCommitFiberRoot =
3838
+ global.__REACT_DEVTOOLS_GLOBAL_HOOK__.onCommitFiberRoot;
3839
+
3840
+ // Redirect all React/ReactDOM requires to v16.8.0
3841
+ // This version predates Fast Refresh support.
3842
+ jest.mock('react', () => jest.requireActual('react-16-8'));
3843
+ jest.mock('react-dom', () => jest.requireActual('react-dom-16-8'));
3844
+
3845
+ // Load React and company.
3846
+ jest.resetModules();
3847
+ React = require('react');
3848
+ ReactDOM = require('react-dom');
3849
+ Scheduler = require('scheduler');
3850
+
3851
+ // Important! Inject into the global hook *after* ReactDOM runs:
3852
+ ReactFreshRuntime = require('react-refresh/runtime');
3853
+ ReactFreshRuntime.injectIntoGlobalHook(global);
3854
+
3855
+ render(() => {
3856
+ function Hello() {
3857
+ return <div>Hi!</div>;
3858
+ }
3859
+ $RefreshReg$(Hello, 'Hello');
3860
+ return Hello;
3861
+ });
3862
+
3863
+ expect(onCommitFiberRoot).toHaveBeenCalled();
3864
+ }
3865
+ });
3866
});
yarn.lock
+28
@@ -11499,6 +11499,16 @@ rc@^1.0.1, rc@^1.1.6, rc@^1.2.8:
11499
object-assign "^4.1.0"
11500
prop-types "^15.5.10"
11501
11502
+"react-16-8@npm:react@16.8.0":
11503
+ version "16.8.0"
11504
+ resolved "https://registry.yarnpkg.com/react/-/react-16.8.0.tgz#8533f0e4af818f448a276eae71681d09e8dd970a"
11505
+ integrity sha512-g+nikW2D48kqgWSPwNo0NH9tIGG3DsQFlrtrQ1kj6W77z5ahyIHG0w8kPpz4Sdj6gyLnz0lEd/xsjOoGge2MYQ==
11506
+ dependencies:
11507
+ loose-envify "^1.1.0"
11508
+ object-assign "^4.1.1"
11509
+ prop-types "^15.6.2"
11510
+ scheduler "^0.13.0"
11511
+
11512
"react-dom-15@npm:react-dom@^15":
11513
version "15.6.2"
11514
resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-15.6.2.tgz#41cfadf693b757faf2708443a1d1fd5a02bef730"
@@ -11509,6 +11519,16 @@ rc@^1.0.1, rc@^1.1.6, rc@^1.2.8:
11519
object-assign "^4.1.0"
11520
prop-types "^15.5.10"
11521
11522
+"react-dom-16-8@npm:react-dom@16.8.0":
11523
+ version "16.8.0"
11524
+ resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-16.8.0.tgz#18f28d4be3571ed206672a267c66dd083145a9c4"
11525
+ integrity sha512-dBzoAGYZpW9Yggp+CzBPC7q1HmWSeRc93DWrwbskmG1eHJWznZB/p0l/Sm+69leIGUS91AXPB/qB3WcPnKx8Sw==
11526
+ dependencies:
11527
+ loose-envify "^1.1.0"
11528
+ object-assign "^4.1.1"
11529
+ prop-types "^15.6.2"
11530
+ scheduler "^0.13.0"
11531
+
11532
react-is@^16.12.0, react-is@^16.8.1:
11533
version "16.13.1"
11534
resolved "https://registry.yarnpkg.com/react-is/-/react-is-16.13.1.tgz#789729a4dc36de2999dc156dd6c1d9c18cea56a4"
@@ -12315,6 +12335,14 @@ scheduler@^0.11.0:
12335
loose-envify "^1.1.0"
12336
object-assign "^4.1.1"
12337
12338
+scheduler@^0.13.0:
12339
+ version "0.13.6"
12340
+ resolved "https://registry.yarnpkg.com/scheduler/-/scheduler-0.13.6.tgz#466a4ec332467b31a91b9bf74e5347072e4cd889"
12341
+ integrity sha512-IWnObHt413ucAYKsD9J1QShUKkbKLQQHdxRyw73sw4FN26iWr3DY/H34xGPe4nmL1DwXyWmSWmMrA9TfQbE/XQ==
12342
+ dependencies:
12343
+ loose-envify "^1.1.0"
12344
+ object-assign "^4.1.1"
12345
+
12346
schema-utils@^1.0.0:
12347
version "1.0.0"
12348
resolved "https://registry.yarnpkg.com/schema-utils/-/schema-utils-1.0.0.tgz#0b79a93204d7b600d4b2850d1f66c2a34951c770"