diff --git a/packages/react-refresh/package.json b/packages/react-refresh/package.json index 36b5102dc1e3e..84b3d704ee9d4 100644 --- a/packages/react-refresh/package.json +++ b/packages/react-refresh/package.json @@ -25,5 +25,9 @@ }, "engines": { "node": ">=0.10.0" + }, + "devDependencies": { + "react-16-8": "npm:react@16.8.0", + "react-dom-16-8": "npm:react-dom@16.8.0" } -} +} \ No newline at end of file diff --git a/packages/react-refresh/src/ReactFreshRuntime.js b/packages/react-refresh/src/ReactFreshRuntime.js index cd5d5a6716bf8..9cf8af21182b9 100644 --- a/packages/react-refresh/src/ReactFreshRuntime.js +++ b/packages/react-refresh/src/ReactFreshRuntime.js @@ -517,53 +517,53 @@ export function injectIntoGlobalHook(globalObject: any): void { didError: boolean, ) { const helpers = helpersByRendererID.get(id); - if (helpers === undefined) { - return; - } - helpersByRoot.set(root, helpers); - - const current = root.current; - const alternate = current.alternate; - - // We need to determine whether this root has just (un)mounted. - // This logic is copy-pasted from similar logic in the DevTools backend. - // If this breaks with some refactoring, you'll want to update DevTools too. - - if (alternate !== null) { - const wasMounted = - alternate.memoizedState != null && - alternate.memoizedState.element != null; - const isMounted = - current.memoizedState != null && - current.memoizedState.element != null; - - if (!wasMounted && isMounted) { + if (helpers !== undefined) { + helpersByRoot.set(root, helpers); + + const current = root.current; + const alternate = current.alternate; + + // We need to determine whether this root has just (un)mounted. + // This logic is copy-pasted from similar logic in the DevTools backend. + // If this breaks with some refactoring, you'll want to update DevTools too. + + if (alternate !== null) { + const wasMounted = + alternate.memoizedState != null && + alternate.memoizedState.element != null; + const isMounted = + current.memoizedState != null && + current.memoizedState.element != null; + + if (!wasMounted && isMounted) { + // Mount a new root. + mountedRoots.add(root); + failedRoots.delete(root); + } else if (wasMounted && isMounted) { + // Update an existing root. + // This doesn't affect our mounted root Set. + } else if (wasMounted && !isMounted) { + // Unmount an existing root. + mountedRoots.delete(root); + if (didError) { + // We'll remount it on future edits. + failedRoots.add(root); + } else { + helpersByRoot.delete(root); + } + } else if (!wasMounted && !isMounted) { + if (didError) { + // We'll remount it on future edits. + failedRoots.add(root); + } + } + } else { // Mount a new root. mountedRoots.add(root); - failedRoots.delete(root); - } else if (wasMounted && isMounted) { - // Update an existing root. - // This doesn't affect our mounted root Set. - } else if (wasMounted && !isMounted) { - // Unmount an existing root. - mountedRoots.delete(root); - if (didError) { - // We'll remount it on future edits. - failedRoots.add(root); - } else { - helpersByRoot.delete(root); - } - } else if (!wasMounted && !isMounted) { - if (didError) { - // We'll remount it on future edits. - failedRoots.add(root); - } } - } else { - // Mount a new root. - mountedRoots.add(root); } + // Always call the decorated DevTools hook. return oldOnCommitFiberRoot.apply(this, arguments); }; } else { diff --git a/packages/react-refresh/src/__tests__/ReactFresh-test.js b/packages/react-refresh/src/__tests__/ReactFresh-test.js index da73032520c64..5e4a766640ad2 100644 --- a/packages/react-refresh/src/__tests__/ReactFresh-test.js +++ b/packages/react-refresh/src/__tests__/ReactFresh-test.js @@ -3745,24 +3745,32 @@ describe('ReactFresh', () => { } }); - // This simulates the scenario in https://github.com/facebook/react/issues/17626. + function initFauxDevToolsHook() { + const onCommitFiberRoot = jest.fn(); + const onCommitFiberUnmount = jest.fn(); + + let idCounter = 0; + const renderers = new Map(); + + // This is a minimal shim for the global hook installed by DevTools. + // The real one is in packages/react-devtools-shared/src/hook.js. + global.__REACT_DEVTOOLS_GLOBAL_HOOK__ = { + renderers, + supportsFiber: true, + inject(renderer) { + const id = ++idCounter; + renderers.set(id, renderer); + return id; + }, + onCommitFiberRoot, + onCommitFiberUnmount, + }; + } + + // This simulates the scenario in https://github.com/facebook/react/issues/17626 it('can inject the runtime after the renderer executes', () => { if (__DEV__) { - // This is a minimal shim for the global hook installed by DevTools. - // The real one is in packages/react-devtools-shared/src/hook.js. - let idCounter = 0; - const renderers = new Map(); - global.__REACT_DEVTOOLS_GLOBAL_HOOK__ = { - renderers, - supportsFiber: true, - inject(renderer) { - const id = ++idCounter; - renderers.set(id, renderer); - return id; - }, - onCommitFiberRoot() {}, - onCommitFiberUnmount() {}, - }; + initFauxDevToolsHook(); // Load these first, as if they're coming from a CDN. jest.resetModules(); @@ -3820,4 +3828,39 @@ describe('ReactFresh', () => { expect(el.style.color).toBe('red'); } }); + + // This simulates the scenario in https://github.com/facebook/react/issues/20100 + it('does not block DevTools when an unsupported renderer is injected', () => { + if (__DEV__) { + initFauxDevToolsHook(); + + const onCommitFiberRoot = + global.__REACT_DEVTOOLS_GLOBAL_HOOK__.onCommitFiberRoot; + + // Redirect all React/ReactDOM requires to v16.8.0 + // This version predates Fast Refresh support. + jest.mock('react', () => jest.requireActual('react-16-8')); + jest.mock('react-dom', () => jest.requireActual('react-dom-16-8')); + + // Load React and company. + jest.resetModules(); + React = require('react'); + ReactDOM = require('react-dom'); + Scheduler = require('scheduler'); + + // Important! Inject into the global hook *after* ReactDOM runs: + ReactFreshRuntime = require('react-refresh/runtime'); + ReactFreshRuntime.injectIntoGlobalHook(global); + + render(() => { + function Hello() { + return
Hi!
; + } + $RefreshReg$(Hello, 'Hello'); + return Hello; + }); + + expect(onCommitFiberRoot).toHaveBeenCalled(); + } + }); }); diff --git a/yarn.lock b/yarn.lock index 52febebc3db8a..fb413bf61246b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -11499,6 +11499,16 @@ rc@^1.0.1, rc@^1.1.6, rc@^1.2.8: object-assign "^4.1.0" prop-types "^15.5.10" +"react-16-8@npm:react@16.8.0": + version "16.8.0" + resolved "https://registry.yarnpkg.com/react/-/react-16.8.0.tgz#8533f0e4af818f448a276eae71681d09e8dd970a" + integrity sha512-g+nikW2D48kqgWSPwNo0NH9tIGG3DsQFlrtrQ1kj6W77z5ahyIHG0w8kPpz4Sdj6gyLnz0lEd/xsjOoGge2MYQ== + dependencies: + loose-envify "^1.1.0" + object-assign "^4.1.1" + prop-types "^15.6.2" + scheduler "^0.13.0" + "react-dom-15@npm:react-dom@^15": version "15.6.2" 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: object-assign "^4.1.0" prop-types "^15.5.10" +"react-dom-16-8@npm:react-dom@16.8.0": + version "16.8.0" + resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-16.8.0.tgz#18f28d4be3571ed206672a267c66dd083145a9c4" + integrity sha512-dBzoAGYZpW9Yggp+CzBPC7q1HmWSeRc93DWrwbskmG1eHJWznZB/p0l/Sm+69leIGUS91AXPB/qB3WcPnKx8Sw== + dependencies: + loose-envify "^1.1.0" + object-assign "^4.1.1" + prop-types "^15.6.2" + scheduler "^0.13.0" + react-is@^16.12.0, react-is@^16.8.1: version "16.13.1" resolved "https://registry.yarnpkg.com/react-is/-/react-is-16.13.1.tgz#789729a4dc36de2999dc156dd6c1d9c18cea56a4" @@ -12315,6 +12335,14 @@ scheduler@^0.11.0: loose-envify "^1.1.0" object-assign "^4.1.1" +scheduler@^0.13.0: + version "0.13.6" + resolved "https://registry.yarnpkg.com/scheduler/-/scheduler-0.13.6.tgz#466a4ec332467b31a91b9bf74e5347072e4cd889" + integrity sha512-IWnObHt413ucAYKsD9J1QShUKkbKLQQHdxRyw73sw4FN26iWr3DY/H34xGPe4nmL1DwXyWmSWmMrA9TfQbE/XQ== + dependencies: + loose-envify "^1.1.0" + object-assign "^4.1.1" + schema-utils@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/schema-utils/-/schema-utils-1.0.0.tgz#0b79a93204d7b600d4b2850d1f66c2a34951c770"