From 51c07912ac047d076e15e730eda1f4634cfe144f Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Thu, 31 Jan 2019 13:56:48 +0000 Subject: [PATCH] Warn when second argument is passed to useCallback (#14729) --- ...DOMServerIntegrationHooks-test.internal.js | 151 +++++++++---- .../ReactNewContext-test.internal.js | 202 +++++++++++------- packages/react/src/ReactHooks.js | 17 +- 3 files changed, 246 insertions(+), 124 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js index 9885cc0e74bef..3ad64377e8138 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.internal.js @@ -613,8 +613,114 @@ describe('ReactDOMServerHooks', () => { }); describe('useContext', () => { + itThrowsWhenRendering( + 'if used inside a class component', + async render => { + const Context = React.createContext({}, () => {}); + class Counter extends React.Component { + render() { + let [count] = useContext(Context); + return ; + } + } + + return render(); + }, + 'Hooks can only be called inside the body of a function component.', + ); + }); + + itRenders( + 'can use the same context multiple times in the same function', + async render => { + const Context = React.createContext({foo: 0, bar: 0, baz: 0}); + + function Provider(props) { + return ( + + {props.children} + + ); + } + + function FooAndBar() { + const {foo} = useContext(Context); + const {bar} = useContext(Context); + return ; + } + + function Baz() { + const {baz} = useContext(Context); + return ; + } + + class Indirection extends React.Component { + render() { + return this.props.children; + } + } + + function App(props) { + return ( +
+ + + + + + + + + + +
+ ); + } + + const domNode = await render(); + expect(clearYields()).toEqual(['Foo: 1, Bar: 3', 'Baz: 5']); + expect(domNode.childNodes.length).toBe(2); + expect(domNode.firstChild.tagName).toEqual('SPAN'); + expect(domNode.firstChild.textContent).toEqual('Foo: 1, Bar: 3'); + expect(domNode.lastChild.tagName).toEqual('SPAN'); + expect(domNode.lastChild.textContent).toEqual('Baz: 5'); + }, + ); + + itRenders('warns when bitmask is passed to useContext', async render => { + let Context = React.createContext('Hi'); + + function Foo() { + return {useContext(Context, 1)}; + } + + const domNode = await render(, 1); + expect(domNode.textContent).toBe('Hi'); + }); + + describe('useDebugValue', () => { + itRenders('is a noop', async render => { + function Counter(props) { + const debugValue = useDebugValue(123); + return ; + } + + const domNode = await render(); + expect(domNode.textContent).toEqual('undefined'); + }); + }); + + describe('readContext', () => { + function readContext(Context, observedBits) { + const dispatcher = + React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED + .ReactCurrentDispatcher.current; + return dispatcher.readContext(Context, observedBits); + } + itRenders( - 'can use the same context multiple times in the same function', + 'can read the same context multiple times in the same function', async render => { const Context = React.createContext( {foo: 0, bar: 0, baz: 0}, @@ -643,13 +749,13 @@ describe('ReactDOMServerHooks', () => { } function FooAndBar() { - const {foo} = useContext(Context, 0b001); - const {bar} = useContext(Context, 0b010); + const {foo} = readContext(Context, 0b001); + const {bar} = readContext(Context, 0b010); return ; } function Baz() { - const {baz} = useContext(Context, 0b100); + const {baz} = readContext(Context, 0b100); return ; } @@ -689,43 +795,6 @@ describe('ReactDOMServerHooks', () => { }, ); - itThrowsWhenRendering( - 'if used inside a class component', - async render => { - const Context = React.createContext({}, () => {}); - class Counter extends React.Component { - render() { - let [count] = useContext(Context); - return ; - } - } - - return render(); - }, - 'Hooks can only be called inside the body of a function component.', - ); - }); - - describe('useDebugValue', () => { - itRenders('is a noop', async render => { - function Counter(props) { - const debugValue = useDebugValue(123); - return ; - } - - const domNode = await render(); - expect(domNode.textContent).toEqual('undefined'); - }); - }); - - describe('readContext', () => { - function readContext(Context, observedBits) { - const dispatcher = - React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED - .ReactCurrentDispatcher.current; - return dispatcher.readContext(Context, observedBits); - } - itRenders('with a warning inside useMemo and useReducer', async render => { const Context = React.createContext(42); diff --git a/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js b/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js index 9a4408598f020..38de5af0fbd4a 100644 --- a/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js @@ -51,7 +51,15 @@ describe('ReactNewContext', () => { Context => function Consumer(props) { const observedBits = props.unstable_observedBits; - const contextValue = useContext(Context, observedBits); + let contextValue; + expect(() => { + contextValue = useContext(Context, observedBits); + }).toWarnDev( + observedBits !== undefined + ? 'useContext() second argument is reserved for future use in React. ' + + `Passing it is not supported. You passed: ${observedBits}.` + : [], + ); const render = props.children; return render(contextValue); }, @@ -59,7 +67,15 @@ describe('ReactNewContext', () => { sharedContextTests('useContext inside forwardRef component', Context => React.forwardRef(function Consumer(props, ref) { const observedBits = props.unstable_observedBits; - const contextValue = useContext(Context, observedBits); + let contextValue; + expect(() => { + contextValue = useContext(Context, observedBits); + }).toWarnDev( + observedBits !== undefined + ? 'useContext() second argument is reserved for future use in React. ' + + `Passing it is not supported. You passed: ${observedBits}.` + : [], + ); const render = props.children; return render(contextValue); }), @@ -67,7 +83,15 @@ describe('ReactNewContext', () => { sharedContextTests('useContext inside memoized function component', Context => React.memo(function Consumer(props) { const observedBits = props.unstable_observedBits; - const contextValue = useContext(Context, observedBits); + let contextValue; + expect(() => { + contextValue = useContext(Context, observedBits); + }).toWarnDev( + observedBits !== undefined + ? 'useContext() second argument is reserved for future use in React. ' + + `Passing it is not supported. You passed: ${observedBits}.` + : [], + ); const render = props.children; return render(contextValue); }), @@ -1300,81 +1324,7 @@ describe('ReactNewContext', () => { }); describe('readContext', () => { - // Context consumer bails out on propagating "deep" updates when `value` hasn't changed. - // However, it doesn't bail out from rendering if the component above it re-rendered anyway. - // If we bailed out on referential equality, it would be confusing that you - // can call this.setState(), but an autobound render callback "blocked" the update. - // https://github.com/facebook/react/pull/12470#issuecomment-376917711 - it('does not bail out if there were no bailouts above it', () => { - const Context = React.createContext(0); - - class Consumer extends React.Component { - render() { - const contextValue = readContext(Context); - return this.props.children(contextValue); - } - } - - class App extends React.Component { - state = { - text: 'hello', - }; - - renderConsumer = context => { - ReactNoop.yield('App#renderConsumer'); - return ; - }; - - render() { - ReactNoop.yield('App'); - return ( - - {this.renderConsumer} - - ); - } - } - - // Initial mount - let inst; - ReactNoop.render( (inst = ref)} />); - expect(ReactNoop.flush()).toEqual(['App', 'App#renderConsumer']); - expect(ReactNoop.getChildren()).toEqual([span('hello')]); - - // Update - inst.setState({text: 'goodbye'}); - expect(ReactNoop.flush()).toEqual(['App', 'App#renderConsumer']); - expect(ReactNoop.getChildren()).toEqual([span('goodbye')]); - }); - - it('warns when reading context inside render phase class setState updater', () => { - const ThemeContext = React.createContext('light'); - - class Cls extends React.Component { - state = {}; - render() { - this.setState(() => { - readContext(ThemeContext); - }); - return null; - } - } - - ReactNoop.render(); - expect(ReactNoop.flush).toWarnDev( - [ - 'Context can only be read while React is rendering', - 'Cannot update during an existing state transition', - ], - { - withoutStack: 1, - }, - ); - }); - }); - - describe('useContext', () => { - it('can use the same context multiple times in the same function', () => { + it('can read the same context multiple times in the same function', () => { const Context = React.createContext({foo: 0, bar: 0, baz: 0}, (a, b) => { let result = 0; if (a.foo !== b.foo) { @@ -1399,13 +1349,13 @@ describe('ReactNewContext', () => { } function FooAndBar() { - const {foo} = useContext(Context, 0b001); - const {bar} = useContext(Context, 0b010); + const {foo} = readContext(Context, 0b001); + const {bar} = readContext(Context, 0b010); return ; } function Baz() { - const {baz} = useContext(Context, 0b100); + const {baz} = readContext(Context, 0b100); return ; } @@ -1465,6 +1415,96 @@ describe('ReactNewContext', () => { ]); }); + // Context consumer bails out on propagating "deep" updates when `value` hasn't changed. + // However, it doesn't bail out from rendering if the component above it re-rendered anyway. + // If we bailed out on referential equality, it would be confusing that you + // can call this.setState(), but an autobound render callback "blocked" the update. + // https://github.com/facebook/react/pull/12470#issuecomment-376917711 + it('does not bail out if there were no bailouts above it', () => { + const Context = React.createContext(0); + + class Consumer extends React.Component { + render() { + const contextValue = readContext(Context); + return this.props.children(contextValue); + } + } + + class App extends React.Component { + state = { + text: 'hello', + }; + + renderConsumer = context => { + ReactNoop.yield('App#renderConsumer'); + return ; + }; + + render() { + ReactNoop.yield('App'); + return ( + + {this.renderConsumer} + + ); + } + } + + // Initial mount + let inst; + ReactNoop.render( (inst = ref)} />); + expect(ReactNoop.flush()).toEqual(['App', 'App#renderConsumer']); + expect(ReactNoop.getChildren()).toEqual([span('hello')]); + + // Update + inst.setState({text: 'goodbye'}); + expect(ReactNoop.flush()).toEqual(['App', 'App#renderConsumer']); + expect(ReactNoop.getChildren()).toEqual([span('goodbye')]); + }); + + it('warns when reading context inside render phase class setState updater', () => { + const ThemeContext = React.createContext('light'); + + class Cls extends React.Component { + state = {}; + render() { + this.setState(() => { + readContext(ThemeContext); + }); + return null; + } + } + + ReactNoop.render(); + expect(ReactNoop.flush).toWarnDev( + [ + 'Context can only be read while React is rendering', + 'Cannot update during an existing state transition', + ], + { + withoutStack: 1, + }, + ); + }); + }); + + describe('useContext', () => { + it('warns on array.map(useContext)', () => { + const Context = React.createContext(0); + function Foo() { + return [Context].map(useContext); + } + ReactNoop.render(); + expect(ReactNoop.flush).toWarnDev( + 'useContext() second argument is reserved for future ' + + 'use in React. Passing it is not supported. ' + + 'You passed: 0.\n\n' + + 'Did you call array.map(useContext)? ' + + 'Calling Hooks inside a loop is not supported. ' + + 'Learn more at https://fb.me/rules-of-hooks', + ); + }); + it('throws when used in a class component', () => { const Context = React.createContext(0); class Foo extends React.Component { diff --git a/packages/react/src/ReactHooks.js b/packages/react/src/ReactHooks.js index 07098fc740a7f..b7ba16ba5dd3a 100644 --- a/packages/react/src/ReactHooks.js +++ b/packages/react/src/ReactHooks.js @@ -24,10 +24,23 @@ function resolveDispatcher() { export function useContext( Context: ReactContext, - observedBits: number | boolean | void, + unstable_observedBits: number | boolean | void, ) { const dispatcher = resolveDispatcher(); if (__DEV__) { + warning( + unstable_observedBits === undefined, + 'useContext() second argument is reserved for future ' + + 'use in React. Passing it is not supported. ' + + 'You passed: %s.%s', + unstable_observedBits, + typeof unstable_observedBits === 'number' && Array.isArray(arguments[2]) + ? '\n\nDid you call array.map(useContext)? ' + + 'Calling Hooks inside a loop is not supported. ' + + 'Learn more at https://fb.me/rules-of-hooks' + : '', + ); + // TODO: add a more generic warning for invalid values. if ((Context: any)._context !== undefined) { const realContext = (Context: any)._context; @@ -48,7 +61,7 @@ export function useContext( } } } - return dispatcher.useContext(Context, observedBits); + return dispatcher.useContext(Context, unstable_observedBits); } export function useState(initialState: (() => S) | S) {