{"commit":{"authoredAt":"2018-10-15T18:31:14-04:00","shortRevision":"f7cb9d2b22","revision":"f7cb9d2b2291b2b4725786105ddd89316711161f","authorName":"Dan Abramov","authorEmail":"dan.abramov@gmail.com","subject":"Warn about useContext(Consumer|Provider)"},"files":[{"status":"M","path":"packages\/react-reconciler\/src\/__tests__\/ReactNewContext-test.internal.js"},{"status":"M","path":"packages\/react\/src\/ReactHooks.js"}],"parents":["63cc7d2b31b4e5ad17c07b383a328c598c91276e"],"patch":"diff --git a\/packages\/react-reconciler\/src\/__tests__\/ReactNewContext-test.internal.js b\/packages\/react-reconciler\/src\/__tests__\/ReactNewContext-test.internal.js\nindex 4ebcd1f84d..4cc8b4b15f 100644\n--- a\/packages\/react-reconciler\/src\/__tests__\/ReactNewContext-test.internal.js\n+++ b\/packages\/react-reconciler\/src\/__tests__\/ReactNewContext-test.internal.js\n@@ -65,16 +65,13 @@ describe('ReactNewContext', () => {\n return render(contextValue);\n }),\n );\n- sharedContextTests(\n- 'useContext inside memoized function component',\n- Context =>\n- React.memo(function Consumer(props) {\n- const observedBits = props.unstable_observedBits;\n- const contextValue = useContext(Context, observedBits);\n- const render = props.children;\n- return render(contextValue);\n- }),\n- true,\n+ sharedContextTests('useContext inside memoized function component', Context =>\n+ React.memo(function Consumer(props) {\n+ const observedBits = props.unstable_observedBits;\n+ const contextValue = useContext(Context, observedBits);\n+ const render = props.children;\n+ return render(contextValue);\n+ }),\n );\n sharedContextTests(\n 'readContext(Context) inside class component',\n@@ -88,8 +85,20 @@ describe('ReactNewContext', () => {\n }\n },\n );\n+ sharedContextTests(\n+ 'readContext(Context) inside pure class component',\n+ Context =>\n+ class Consumer extends React.PureComponent {\n+ render() {\n+ const observedBits = this.props.unstable_observedBits;\n+ const contextValue = readContext(Context, observedBits);\n+ const render = this.props.children;\n+ return render(contextValue);\n+ }\n+ },\n+ );\n \n- function sharedContextTests(label, getConsumer, isPure) {\n+ function sharedContextTests(label, getConsumer) {\n describe(`reading context with ${label}`, () => {\n it('simple mount and update', () => {\n const Context = React.createContext(1);\n@@ -874,49 +883,6 @@ describe('ReactNewContext', () => {\n expect(ReactNoop.getChildren()).toEqual([span(2), span(2)]);\n });\n \n- if (!isPure) {\n- \/\/ Context consumer bails out on propagating \"deep\" updates when `value` hasn't changed.\n- \/\/ However, it doesn't bail out from rendering if the component above it re-rendered anyway.\n- \/\/ If we bailed out on referential equality, it would be confusing that you\n- \/\/ can call this.setState(), but an autobound render callback \"blocked\" the update.\n- \/\/ https:\/\/github.com\/facebook\/react\/pull\/12470#issuecomment-376917711\n- it('consumer does not bail out if there were no bailouts above it', () => {\n- const Context = React.createContext(0);\n- const Consumer = getConsumer(Context);\n-\n- class App extends React.Component {\n- state = {\n- text: 'hello',\n- };\n-\n- renderConsumer = context => {\n- ReactNoop.yield('App#renderConsumer');\n- return ;\n- };\n-\n- render() {\n- ReactNoop.yield('App');\n- return (\n- \n- {this.renderConsumer}<\/Consumer>\n- <\/Context.Provider>\n- );\n- }\n- }\n-\n- \/\/ Initial mount\n- let inst;\n- ReactNoop.render( (inst = ref)} \/>);\n- expect(ReactNoop.flush()).toEqual(['App', 'App#renderConsumer']);\n- expect(ReactNoop.getChildren()).toEqual([span('hello')]);\n-\n- \/\/ Update\n- inst.setState({text: 'goodbye'});\n- expect(ReactNoop.flush()).toEqual(['App', 'App#renderConsumer']);\n- expect(ReactNoop.getChildren()).toEqual([span('goodbye')]);\n- });\n- }\n-\n \/\/ This is a regression case for https:\/\/github.com\/facebook\/react\/issues\/12389.\n it('does not run into an infinite loop', () => {\n const Context = React.createContext(null);\n@@ -1260,6 +1226,96 @@ describe('ReactNewContext', () => {\n expect(ReactNoop.flush()).toEqual(['Foo: 2, Bar: 2']);\n expect(ReactNoop.getChildren()).toEqual([span('Foo: 2, Bar: 2')]);\n });\n+\n+ \/\/ Context consumer bails out on propagating \"deep\" updates when `value` hasn't changed.\n+ \/\/ However, it doesn't bail out from rendering if the component above it re-rendered anyway.\n+ \/\/ If we bailed out on referential equality, it would be confusing that you\n+ \/\/ can call this.setState(), but an autobound render callback \"blocked\" the update.\n+ \/\/ https:\/\/github.com\/facebook\/react\/pull\/12470#issuecomment-376917711\n+ it('consumer does not bail out if there were no bailouts above it', () => {\n+ const Context = React.createContext(0);\n+ const Consumer = Context.Consumer;\n+\n+ class App extends React.Component {\n+ state = {\n+ text: 'hello',\n+ };\n+\n+ renderConsumer = context => {\n+ ReactNoop.yield('App#renderConsumer');\n+ return ;\n+ };\n+\n+ render() {\n+ ReactNoop.yield('App');\n+ return (\n+ \n+ {this.renderConsumer}<\/Consumer>\n+ <\/Context.Provider>\n+ );\n+ }\n+ }\n+\n+ \/\/ Initial mount\n+ let inst;\n+ ReactNoop.render( (inst = ref)} \/>);\n+ expect(ReactNoop.flush()).toEqual(['App', 'App#renderConsumer']);\n+ expect(ReactNoop.getChildren()).toEqual([span('hello')]);\n+\n+ \/\/ Update\n+ inst.setState({text: 'goodbye'});\n+ expect(ReactNoop.flush()).toEqual(['App', 'App#renderConsumer']);\n+ expect(ReactNoop.getChildren()).toEqual([span('goodbye')]);\n+ });\n+ });\n+\n+ describe('readContext', () => {\n+ \/\/ Context consumer bails out on propagating \"deep\" updates when `value` hasn't changed.\n+ \/\/ However, it doesn't bail out from rendering if the component above it re-rendered anyway.\n+ \/\/ If we bailed out on referential equality, it would be confusing that you\n+ \/\/ can call this.setState(), but an autobound render callback \"blocked\" the update.\n+ \/\/ https:\/\/github.com\/facebook\/react\/pull\/12470#issuecomment-376917711\n+ it('does not bail out if there were no bailouts above it', () => {\n+ const Context = React.createContext(0);\n+\n+ class Consumer extends React.Component {\n+ render() {\n+ const contextValue = readContext(Context);\n+ return this.props.children(contextValue);\n+ }\n+ }\n+\n+ class App extends React.Component {\n+ state = {\n+ text: 'hello',\n+ };\n+\n+ renderConsumer = context => {\n+ ReactNoop.yield('App#renderConsumer');\n+ return ;\n+ };\n+\n+ render() {\n+ ReactNoop.yield('App');\n+ return (\n+ \n+ {this.renderConsumer}<\/Consumer>\n+ <\/Context.Provider>\n+ );\n+ }\n+ }\n+\n+ \/\/ Initial mount\n+ let inst;\n+ ReactNoop.render( (inst = ref)} \/>);\n+ expect(ReactNoop.flush()).toEqual(['App', 'App#renderConsumer']);\n+ expect(ReactNoop.getChildren()).toEqual([span('hello')]);\n+\n+ \/\/ Update\n+ inst.setState({text: 'goodbye'});\n+ expect(ReactNoop.flush()).toEqual(['App', 'App#renderConsumer']);\n+ expect(ReactNoop.getChildren()).toEqual([span('goodbye')]);\n+ });\n });\n \n describe('useContext', () => {\n@@ -1366,6 +1422,77 @@ describe('ReactNewContext', () => {\n 'Hooks can only be called inside the body of a function component.',\n );\n });\n+\n+ it('warns when passed a consumer', () => {\n+ const Context = React.createContext(0);\n+ function Foo() {\n+ return useContext(Context.Consumer);\n+ }\n+ ReactNoop.render();\n+ expect(ReactNoop.flush).toWarnDev(\n+ 'Calling useContext(Context.Consumer) is not supported, may cause bugs, ' +\n+ 'and will be removed in a future major release. ' +\n+ 'Did you mean to call useContext(Context) instead?',\n+ );\n+ });\n+\n+ it('warns when passed a provider', () => {\n+ const Context = React.createContext(0);\n+ function Foo() {\n+ useContext(Context.Provider);\n+ return null;\n+ }\n+ ReactNoop.render();\n+ expect(ReactNoop.flush).toWarnDev(\n+ 'Calling useContext(Context.Provider) is not supported. ' +\n+ 'Did you mean to call useContext(Context) instead?',\n+ );\n+ });\n+\n+ \/\/ Context consumer bails out on propagating \"deep\" updates when `value` hasn't changed.\n+ \/\/ However, it doesn't bail out from rendering if the component above it re-rendered anyway.\n+ \/\/ If we bailed out on referential equality, it would be confusing that you\n+ \/\/ can call this.setState(), but an autobound render callback \"blocked\" the update.\n+ \/\/ https:\/\/github.com\/facebook\/react\/pull\/12470#issuecomment-376917711\n+ it('does not bail out if there were no bailouts above it', () => {\n+ const Context = React.createContext(0);\n+\n+ function Consumer({children}) {\n+ const contextValue = useContext(Context);\n+ return children(contextValue);\n+ }\n+\n+ class App extends React.Component {\n+ state = {\n+ text: 'hello',\n+ };\n+\n+ renderConsumer = context => {\n+ ReactNoop.yield('App#renderConsumer');\n+ return ;\n+ };\n+\n+ render() {\n+ ReactNoop.yield('App');\n+ return (\n+ \n+ {this.renderConsumer}<\/Consumer>\n+ <\/Context.Provider>\n+ );\n+ }\n+ }\n+\n+ \/\/ Initial mount\n+ let inst;\n+ ReactNoop.render( (inst = ref)} \/>);\n+ expect(ReactNoop.flush()).toEqual(['App', 'App#renderConsumer']);\n+ expect(ReactNoop.getChildren()).toEqual([span('hello')]);\n+\n+ \/\/ Update\n+ inst.setState({text: 'goodbye'});\n+ expect(ReactNoop.flush()).toEqual(['App', 'App#renderConsumer']);\n+ expect(ReactNoop.getChildren()).toEqual([span('goodbye')]);\n+ });\n });\n \n it('unwinds after errors in complete phase', () => {\ndiff --git a\/packages\/react\/src\/ReactHooks.js b\/packages\/react\/src\/ReactHooks.js\nindex e2ccc3a2a8..1cc8b79ac5 100644\n--- a\/packages\/react\/src\/ReactHooks.js\n+++ b\/packages\/react\/src\/ReactHooks.js\n@@ -9,6 +9,7 @@\n \n import type {ReactContext} from 'shared\/ReactTypes';\n import invariant from 'shared\/invariant';\n+import warning from 'shared\/warning';\n \n import ReactCurrentOwner from '.\/ReactCurrentOwner';\n \n@@ -26,6 +27,27 @@ export function useContext(\n observedBits: number | boolean | void,\n ) {\n const dispatcher = resolveDispatcher();\n+ if (__DEV__) {\n+ \/\/ TODO: add a more generic warning for invalid values.\n+ if ((Context: any)._context !== undefined) {\n+ const realContext = (Context: any)._context;\n+ \/\/ Don't deduplicate because this legitimately causes bugs\n+ \/\/ and nobody should be using this in existing code.\n+ if (realContext.Consumer === Context) {\n+ warning(\n+ false,\n+ 'Calling useContext(Context.Consumer) is not supported, may cause bugs, and will be ' +\n+ 'removed in a future major release. Did you mean to call useContext(Context) instead?',\n+ );\n+ } else if (realContext.Provider === Context) {\n+ warning(\n+ false,\n+ 'Calling useContext(Context.Provider) is not supported. ' +\n+ 'Did you mean to call useContext(Context) instead?',\n+ );\n+ }\n+ }\n+ }\n return dispatcher.useContext(Context, observedBits);\n }\n \n","body":""}

dillon1000/react

Files changed