From 6707b4b4473b46b194ef731ce7ae38ab51ebbbbb Mon Sep 17 00:00:00 2001 From: Anton Korzunov Date: Thu, 14 Nov 2019 23:35:31 +1100 Subject: [PATCH 1/4] fix: (regression) hook order change is causing React error, fixes #1393 --- src/internal/getReactStack.js | 5 +++-- src/webpack/patch.js | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/internal/getReactStack.js b/src/internal/getReactStack.js index 71819a7ab..27ba0602b 100644 --- a/src/internal/getReactStack.js +++ b/src/internal/getReactStack.js @@ -34,7 +34,8 @@ const markUpdate = ({ fiber }) => { const mostResentType = resolveType(fiber.type) || fiber.type; if (fiber.elementType === fiber.type) { - fiber.elementType = mostResentType; + // DO NOT update elementType - or will not be able to catch un update + // fiber.elementType = mostResentType; } fiber.type = mostResentType; @@ -44,7 +45,7 @@ const markUpdate = ({ fiber }) => { fiber.alternate.type = fiber.type; // elementType might not exists in older react versions if ('elementType' in fiber.alternate) { - fiber.alternate.elementType = fiber.elementType; + // fiber.alternate.elementType = fiber.elementType; } } diff --git a/src/webpack/patch.js b/src/webpack/patch.js index f83d33e09..ee5624dc5 100644 --- a/src/webpack/patch.js +++ b/src/webpack/patch.js @@ -26,12 +26,13 @@ const injectionStart = { }; const additional = { + '16.10-update': [ - '( // Keep this check inline so it only runs on the false path:\n isCompatibleFamilyForHotReloading(current$$1, element)))', + 'current$$1.elementType === element.type || ( // Keep this check inline so it only runs on the false path:\n isCompatibleFamilyForHotReloading(current$$1, element)))', '(hotCompareElements(current$$1.elementType, element.type, hotUpdateChild(current$$1), current$$1.type)))' ], '16.9-update': [ - '(\n // Keep this check inline so it only runs on the false path:\n isCompatibleFamilyForHotReloading(current$$1, element)))', + 'current$$1.elementType === element.type || (\n // Keep this check inline so it only runs on the false path:\n isCompatibleFamilyForHotReloading(current$$1, element)))', '(hotCompareElements(current$$1.elementType, element.type, hotUpdateChild(current$$1), current$$1.type)))' ], '16.6-update': [ From b94adb31e556bf1e379030591beb0999e30c33e3 Mon Sep 17 00:00:00 2001 From: Anton Korzunov Date: Fri, 15 Nov 2019 15:10:40 +1100 Subject: [PATCH 2/4] fix: production babel plugin might perform eager replacement, fixes #1388 --- src/babel.prod.js | 1 + test/__babel_fixtures__/drop-hot-half.prod.js | 23 +++++++++++++ test/__snapshots__/babel.test.js.snap | 32 +++++++++++++++++++ 3 files changed, 56 insertions(+) create mode 100644 test/__babel_fixtures__/drop-hot-half.prod.js diff --git a/src/babel.prod.js b/src/babel.prod.js index cdfa8587a..d0f92ce24 100644 --- a/src/babel.prod.js +++ b/src/babel.prod.js @@ -74,6 +74,7 @@ export default function plugin() { // ensure that this is `hot` from RHL isImportedFromRHL(path, specifier.local) && path.parent.type === 'CallExpression' && + path.parent.arguments.length === 1 && path.parent.arguments[0] && path.parent.arguments[0].type === 'Identifier' ) { diff --git a/test/__babel_fixtures__/drop-hot-half.prod.js b/test/__babel_fixtures__/drop-hot-half.prod.js new file mode 100644 index 000000000..44b231fb3 --- /dev/null +++ b/test/__babel_fixtures__/drop-hot-half.prod.js @@ -0,0 +1,23 @@ +import { hot } from 'react-hot-loader'; +import { hot as rootHot } from 'react-hot-loader/root'; + +const control = compose( + withDebug, + withDebug, +)(App); + +const targetCase1 = compose( + withDebug, + withDebug, + hot(module), +)(App); + +const targetCase2 = compose( + withDebug, + withDebug, + rootHot, +)(App); + +const removeHot1 = hot(control); +const removeHot2 = hot(module)(control); +const removeHot3 = rootHot(control); \ No newline at end of file diff --git a/test/__snapshots__/babel.test.js.snap b/test/__snapshots__/babel.test.js.snap index 1bfd1531d..a835fb508 100644 --- a/test/__snapshots__/babel.test.js.snap +++ b/test/__snapshots__/babel.test.js.snap @@ -1069,6 +1069,24 @@ exports.default = _default; })();" `; +exports[`babel Targetting "es2015" tags potential React components drop hot half.prod.js 1`] = ` +"'use strict'; + +var _reactHotLoader = require('react-hot-loader'); + +var _root = require('react-hot-loader/root'); + +var control = compose(withDebug, withDebug)(App); + +var targetCase1 = compose(withDebug, withDebug, (0, _reactHotLoader.hot)(module))(App); + +var targetCase2 = compose(withDebug, withDebug, _root.hot)(App); + +var removeHot1 = (0, _reactHotLoader.hot)(control); +var removeHot2 = control; +var removeHot3 = control;" +`; + exports[`babel Targetting "es2015" tags potential React components drop hot.prod.js 1`] = ` "'use strict'; @@ -2179,6 +2197,20 @@ exports.default = _default; })();" `; +exports[`babel Targetting "modern" tags potential React components drop hot half.prod.js 1`] = ` +"'use strict'; + +Object.defineProperty(exports, \\"__esModule\\", { + value: true +}); + +var _reactHotLoader = require('react-hot-loader'); + +var _root = require('react-hot-loader/root'); + +exports.default = withDebug(App);" +`; + exports[`babel Targetting "modern" tags potential React components drop hot.prod.js 1`] = ` "'use strict'; From 300a819f8b65d61ea82e4cf904b7df711b86689a Mon Sep 17 00:00:00 2001 From: Anton Korzunov Date: Sat, 16 Nov 2019 14:02:26 +1100 Subject: [PATCH 3/4] add tests for hook reload --- src/internal/getReactStack.js | 9 +-- test/__snapshots__/babel.test.js.snap | 14 +++-- test/hot/react-dom.integration.spec.js | 78 +++++++++++++++++++++++++- 3 files changed, 86 insertions(+), 15 deletions(-) diff --git a/src/internal/getReactStack.js b/src/internal/getReactStack.js index 27ba0602b..35d4fdd7b 100644 --- a/src/internal/getReactStack.js +++ b/src/internal/getReactStack.js @@ -33,20 +33,13 @@ const markUpdate = ({ fiber }) => { } const mostResentType = resolveType(fiber.type) || fiber.type; - if (fiber.elementType === fiber.type) { - // DO NOT update elementType - or will not be able to catch un update - // fiber.elementType = mostResentType; - } fiber.type = mostResentType; + // do not change fiber.elementType to keep old information for the hot-update fiber.expirationTime = 1; if (fiber.alternate) { fiber.alternate.expirationTime = 1; fiber.alternate.type = fiber.type; - // elementType might not exists in older react versions - if ('elementType' in fiber.alternate) { - // fiber.alternate.elementType = fiber.elementType; - } } if (fiber.memoizedProps && typeof fiber.memoizedProps === 'object') { diff --git a/test/__snapshots__/babel.test.js.snap b/test/__snapshots__/babel.test.js.snap index a835fb508..ec039c205 100644 --- a/test/__snapshots__/babel.test.js.snap +++ b/test/__snapshots__/babel.test.js.snap @@ -2200,15 +2200,19 @@ exports.default = _default; exports[`babel Targetting "modern" tags potential React components drop hot half.prod.js 1`] = ` "'use strict'; -Object.defineProperty(exports, \\"__esModule\\", { - value: true -}); - var _reactHotLoader = require('react-hot-loader'); var _root = require('react-hot-loader/root'); -exports.default = withDebug(App);" +const control = compose(withDebug, withDebug)(App); + +const targetCase1 = compose(withDebug, withDebug, (0, _reactHotLoader.hot)(module))(App); + +const targetCase2 = compose(withDebug, withDebug, _root.hot)(App); + +const removeHot1 = (0, _reactHotLoader.hot)(control); +const removeHot2 = control; +const removeHot3 = control;" `; exports[`babel Targetting "modern" tags potential React components drop hot.prod.js 1`] = ` diff --git a/test/hot/react-dom.integration.spec.js b/test/hot/react-dom.integration.spec.js index f574475c1..c1d105bbc 100644 --- a/test/hot/react-dom.integration.spec.js +++ b/test/hot/react-dom.integration.spec.js @@ -5,9 +5,11 @@ import React, { Suspense } from 'react'; import ReactDom from 'react-dom'; // import TestRenderer from 'react-test-renderer'; import ReactHotLoader, { setConfig } from '../../src/index.dev'; -import { configureGeneration, incrementHotGeneration } from '../../src/global/generation'; +import { configureGeneration, enterHotUpdate, incrementHotGeneration } from '../../src/global/generation'; import configuration from '../../src/configuration'; -import { AppContainer } from '../../index'; +import AppContainer from '../../src/AppContainer.dev'; +import reactHotLoader from '../../src/reactHotLoader'; +import reconcileHotReplacement from '../../src/reconciler'; jest.mock('react-dom', () => { const reactDom = require('./react-dom'); @@ -21,6 +23,7 @@ describe(`🔥-dom`, () => { ignoreSFC: true, }); configureGeneration(1, 1); + reactHotLoader.register(AppContainer, 'AppContainer', 'test'); }); const tick = () => new Promise(resolve => setTimeout(resolve, 10)); @@ -213,6 +216,77 @@ describe(`🔥-dom`, () => { expect(mount).toHaveBeenCalledWith('test2'); }); + it('should fail on hook order change', async () => { + const Fun1 = () => { + const [state, setState] = React.useState('test0'); + React.useEffect(() => setState('test1'), []); + return state; + }; + + const el = document.createElement('div'); + ReactDom.render(, el); + + expect(el.innerHTML).toEqual('test0'); + + incrementHotGeneration(); + { + const Fun1 = () => { + React.useState('anotherstate'); + const [state, setState] = React.useState('test0'); + React.useEffect(() => setState('test1'), []); + return state; + }; + expect(() => ReactDom.render(, el)).toThrow(); + } + }); + + it('should set on hook order change if signature provided', async () => { + const ref = React.createRef(); + const App = ({ children }) => {children}; + const Fun1 = () => { + const [state, setState] = React.useState('test0'); + React.useEffect(() => setState('test1'), []); + return state; + }; + reactHotLoader.signature(Fun1, 'fun1-key1'); + reactHotLoader.register(Fun1, 'Fun1', 'test'); + + const el = document.createElement('div'); + ReactDom.render( + + + , + el, + ); + + expect(el.innerHTML).toEqual('test0'); + + { + const Fun1 = () => { + React.useState('anotherstate'); + const [state, setState] = React.useState('test-new'); + React.useEffect(() => setState('test1'), []); + return state; + }; + reactHotLoader.signature(Fun1, 'fun1-key2'); + reactHotLoader.register(Fun1, 'Fun1', 'test'); + + incrementHotGeneration(); + enterHotUpdate(); + reconcileHotReplacement(ref.current); + + expect(() => + ReactDom.render( + + + , + el, + ), + ).not.toThrow(); + expect(el.innerHTML).toEqual('test-new'); + } + }); + it('should reset hook comparator', async () => { const Fun1 = () => { const [state, setState] = React.useState('test0'); From 29ef0a0f270c8521b8debd84f37c7bfd8bae2fff Mon Sep 17 00:00:00 2001 From: Anton Korzunov Date: Sat, 16 Nov 2019 19:25:33 +1100 Subject: [PATCH 4/4] more rich test scenario fro hooks reload --- examples/styled-components/src/App.js | 3 +++ test/hot/react-dom.integration.spec.js | 21 ++++++++++++++++++--- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/examples/styled-components/src/App.js b/examples/styled-components/src/App.js index 74d24a343..732b15fdc 100644 --- a/examples/styled-components/src/App.js +++ b/examples/styled-components/src/App.js @@ -41,6 +41,9 @@ const Context = React.createContext(); const Hook = () => { const [state, setState] = React.useState({ x: 4 }); + + React.useState(0); + React.useEffect(() => { console.log('mount effected 1'); setState(state => ({ diff --git a/test/hot/react-dom.integration.spec.js b/test/hot/react-dom.integration.spec.js index c1d105bbc..6b4bb2a57 100644 --- a/test/hot/react-dom.integration.spec.js +++ b/test/hot/react-dom.integration.spec.js @@ -242,12 +242,23 @@ describe(`🔥-dom`, () => { it('should set on hook order change if signature provided', async () => { const ref = React.createRef(); - const App = ({ children }) => {children}; + const App = ({ children }) => ( + + {children} + + ); const Fun1 = () => { const [state, setState] = React.useState('test0'); React.useEffect(() => setState('test1'), []); return state; }; + + const Fun2 = () => { + const [state, setState] = React.useState('step1'); + React.useEffect(() => setState('step2'), []); + return state; + }; + reactHotLoader.signature(Fun1, 'fun1-key1'); reactHotLoader.register(Fun1, 'Fun1', 'test'); @@ -255,11 +266,14 @@ describe(`🔥-dom`, () => { ReactDom.render( + , el, ); - expect(el.innerHTML).toEqual('test0'); + expect(el.innerHTML).toEqual('test0step1'); + await tick(); + expect(el.innerHTML).toEqual('test1step2'); { const Fun1 = () => { @@ -279,11 +293,12 @@ describe(`🔥-dom`, () => { ReactDom.render( + , el, ), ).not.toThrow(); - expect(el.innerHTML).toEqual('test-new'); + expect(el.innerHTML).toEqual('test-newstep2'); } });