Skip to content
Prev Previous commit
Next Next commit
address more feedback
  • Loading branch information
trueadm committed Oct 12, 2018
commit 26d6d2c203dece291f8a776c8bf799d89a1e5e5a
Original file line number Diff line number Diff line change
Expand Up @@ -1635,4 +1635,28 @@ Context fuzz tester error! Copy and paste the following line into the test suite
'a future major release. Did you mean to render Context.unstable_read() instead?',
);
});

it('should warn with an error message when using Context.Consumer.Provider DEV', () => {
const BarContext = React.createContext({value: 'bar-initial'});

function Component() {
return (
<React.Fragment>
<BarContext.Consumer.Provider value={{value: 'bar-updated'}}>
<BarContext.Consumer>
{({value}) => <div actual={value} expected="bar-updated" />}
</BarContext.Consumer>
</BarContext.Consumer.Provider>
</React.Fragment>
);
}

expect(() => {
ReactNoop.render(<Component />);
ReactNoop.flush();
}).toWarnDev(
'Rendering <Context.Consumer.Provider> is not supported and will be removed in ' +
'a future major release. Did you mean to render <Context.Provider> instead?',
);
});
});
18 changes: 17 additions & 1 deletion packages/react/src/ReactContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ export function createContext<T>(

let hasWarnedAboutUsingNestedContextConsumers = false;
let hasWarnedAboutUsingConsumerUnstableRead = false;
let hasWarnedAboutUsingConsumerProvider = false;

if (__DEV__) {
// A separate object, but proxies back to the original context object for
Expand All @@ -82,7 +83,6 @@ export function createContext<T>(
$$typeof: REACT_CONTEXT_TYPE,
_context: context,
_calculateChangedBits: context._calculateChangedBits,
Provider: context.Provider,
unstable_read() {
if (!hasWarnedAboutUsingConsumerUnstableRead) {
hasWarnedAboutUsingConsumerUnstableRead = true;
Expand All @@ -97,6 +97,22 @@ export function createContext<T>(
};
// $FlowFixMe: Flow complains about not setting a value, which is intentional here
Object.defineProperties(consumer, {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it better to have consumer proxy to context, or the other way around? Why?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whatever we read/write from/to most. I’m actually not sure which one that is.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked and the context gets written to the most in our tests.

Provider: {
get() {
if (!hasWarnedAboutUsingConsumerProvider) {
hasWarnedAboutUsingConsumerProvider = true;
warning(
false,
'Rendering <Context.Consumer.Provider> is not supported and will be removed in ' +
'a future major release. Did you mean to render <Context.Provider> instead?',
);
}
return context.Provider;
},
set(_Provider) {
context.Provider = _Provider;
},
},
_currentValue: {
get() {
return context._currentValue;
Expand Down