Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(regression) hook order change is causing React error #1394

Merged
merged 4 commits into from Nov 16, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions examples/styled-components/src/App.js
Expand Up @@ -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 => ({
Expand Down
1 change: 1 addition & 0 deletions src/babel.prod.js
Expand Up @@ -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'
) {
Expand Down
8 changes: 1 addition & 7 deletions src/internal/getReactStack.js
Expand Up @@ -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') {
Expand Down
5 changes: 3 additions & 2 deletions src/webpack/patch.js
Expand Up @@ -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': [
Expand Down
23 changes: 23 additions & 0 deletions 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);
36 changes: 36 additions & 0 deletions test/__snapshots__/babel.test.js.snap
Expand Up @@ -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';

Expand Down Expand Up @@ -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';

Expand Down
93 changes: 91 additions & 2 deletions test/hot/react-dom.integration.spec.js
Expand Up @@ -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');
Expand All @@ -21,6 +23,7 @@ describe(`🔥-dom`, () => {
ignoreSFC: true,
});
configureGeneration(1, 1);
reactHotLoader.register(AppContainer, 'AppContainer', 'test');
});

const tick = () => new Promise(resolve => setTimeout(resolve, 10));
Expand Down Expand Up @@ -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');
Expand Down