Warn about variable number of dependencies

We don't check this in prod, since best practice is to always pass
these inline. But we should still warn in dev.
This commit is contained in:
Andrew Clark
2018-10-29 13:38:54 -07:00
parent b92cdef641
commit e02086bfcc
3 changed files with 71 additions and 2 deletions
+13 -1
View File
@@ -37,6 +37,7 @@ import {
} from './ReactFiberScheduler';
import invariant from 'shared/invariant';
import warning from 'shared/warning';
import warningWithoutStack from 'shared/warningWithoutStack';
import {enableDispatchCallback_DEPRECATED} from 'shared/ReactFeatureFlags';
@@ -760,8 +761,19 @@ function dispatchAction<S, A>(
}
function inputsAreEqual(arr1, arr2) {
// Don't bother comparing lengths because these arrays are always
// Don't bother comparing lengths in prod because these arrays should be
// passed inline.
if (__DEV__) {
warning(
arr1.length === arr2.length,
'Detected a variable number of hook dependencies. The length of the ' +
'dependencies array should be constant between renders.\n\n' +
'Previous: %s\n' +
'Incoming: %s',
arr1.join(', '),
arr2.join(', '),
);
}
for (let i = 0; i < arr1.length; i++) {
// Inlined Object.is polyfill.
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/is
@@ -0,0 +1,54 @@
/**
* Copyright (c) 2013-present, Facebook, Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @emails react-core
* @jest-environment node
*/
/* eslint-disable no-func-assign */
'use strict';
let React;
let ReactFeatureFlags;
let ReactTestRenderer;
// Additional tests can be found in ReactHooksWithNoopRenderer. Plan is to
// gradually migrate those to this file.
describe('ReactHooks', () => {
beforeEach(() => {
jest.resetModules();
ReactFeatureFlags = require('shared/ReactFeatureFlags');
ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false;
ReactFeatureFlags.enableHooks = true;
React = require('react');
ReactTestRenderer = require('react-test-renderer');
});
it('warns about variable number of dependencies', () => {
const {useLayoutEffect} = React;
function App(props) {
useLayoutEffect(() => {
ReactTestRenderer.unstable_yield(
'Did commit: ' + props.dependencies.join(', '),
);
}, props.dependencies);
return props.dependencies;
}
const root = ReactTestRenderer.create(<App dependencies={['A']} />);
expect(ReactTestRenderer).toHaveYielded(['Did commit: A']);
expect(() => {
root.update(<App dependencies={['A', 'B']} />);
}).toWarnDev([
'Warning: Detected a variable number of hook dependencies. The length ' +
'of the dependencies array should be constant between renders.\n\n' +
'Previous: A, B\n' +
'Incoming: A',
]);
expect(ReactTestRenderer).toHaveYielded(['Did commit: A, B']);
});
});
@@ -29,7 +29,10 @@ let forwardRef;
let flushPassiveEffects;
let memo;
describe('ReactHooks', () => {
// These tests use React Noop Renderer. All new tests should use React Test
// Renderer and go in ReactHooks-test; plan is gradually migrate the noop tests
// to that file.
describe('ReactHooksWithNoopRenderer', () => {
beforeEach(() => {
jest.resetModules();