Merge pull request #1394 from gaearon/hot-hook-fix

(regression) hook order change is causing React error

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 => ({

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'
         ) {

src/internal/getReactStack.js

@@ -33,19 +33,13 @@ const markUpdate = ({ fiber }) => {
   }
 
   const mostResentType = resolveType(fiber.type) || fiber.type;
-  if (fiber.elementType === fiber.type) {
-    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') {

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': [

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);

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,24 @@ exports.default = _default;
 })();"
 `;
 
+exports[`babel Targetting "modern" 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');
+
+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`] = `
 "'use strict';
 

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,92 @@ 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(<Fun1 />, 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(<Fun1 />, el)).toThrow();
+      }
+    });
+
+    it('should set on hook order change if signature provided', async () => {
+      const ref = React.createRef();
+      const App = ({ children }) => (
+        <AppContainer ref={ref}>
+          <React.Fragment>{children}</React.Fragment>
+        </AppContainer>
+      );
+      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');
+
+      const el = document.createElement('div');
+      ReactDom.render(
+        <App>
+          <Fun1 />
+          <Fun2 />
+        </App>,
+        el,
+      );
+
+      expect(el.innerHTML).toEqual('test0step1');
+      await tick();
+      expect(el.innerHTML).toEqual('test1step2');
+
+      {
+        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(
+            <App>
+              <Fun1 />
+              <Fun2 />
+            </App>,
+            el,
+          ),
+        ).not.toThrow();
+        expect(el.innerHTML).toEqual('test-newstep2');
+      }
+    });
+
     it('should reset hook comparator', async () => {
       const Fun1 = () => {
         const [state, setState] = React.useState('test0');