Skip to content

Commit

Permalink
Remove unstable_read() in favor of direct dispatcher call (facebook#1…
Browse files Browse the repository at this point in the history 
…3861)

* Remove unstable_read() in favor of direct dispatcher call

* This no longer throws immediately
  • Loading branch information
@gaearon @jetoneza
gaearon authored and jetoneza committed Jan 23, 2019
1 parent 183d127 commit 7e0b045
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 78 deletions.
18 changes: 13 additions & 5 deletions packages/react-reconciler/src/ReactFiberClassComponent.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
import ReactStrictModeWarnings from './ReactStrictModeWarnings';
import {isMounted} from 'react-reconciler/reflection';
import * as ReactInstanceMap from 'shared/ReactInstanceMap';
import ReactSharedInternals from 'shared/ReactSharedInternals';
import shallowEqual from 'shared/shallowEqual';
import getComponentName from 'shared/getComponentName';
import invariant from 'shared/invariant';
Expand Down Expand Up @@ -50,6 +51,13 @@ import {
scheduleWork,
} from './ReactFiberScheduler';

const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner;

function readContext(contextType: any): any {
const dispatcher = ReactCurrentOwner.currentDispatcher;
return dispatcher.readContext(contextType);
}

const fakeInternalInstance = {};
const isArray = Array.isArray;

Expand Down Expand Up @@ -508,7 +516,7 @@ function constructClassInstance(
if (typeof contextType === 'object' && contextType !== null) {
if (__DEV__) {
if (
typeof contextType.unstable_read !== 'function' &&
contextType.Consumer === undefined &&
!didWarnAboutInvalidateContextType.has(ctor)
) {
didWarnAboutInvalidateContextType.add(ctor);
Expand All @@ -522,7 +530,7 @@ function constructClassInstance(
}
}

context = (contextType: any).unstable_read();
context = readContext((contextType: any));
} else {
unmaskedContext = getUnmaskedContext(workInProgress, ctor, true);
const contextTypes = ctor.contextTypes;
Expand Down Expand Up @@ -725,7 +733,7 @@ function mountClassInstance(

const contextType = ctor.contextType;
if (typeof contextType === 'object' && contextType !== null) {
instance.context = (contextType: any).unstable_read();
instance.context = readContext(contextType);
} else {
const unmaskedContext = getUnmaskedContext(workInProgress, ctor, true);
instance.context = getMaskedContext(workInProgress, unmaskedContext);
Expand Down Expand Up @@ -833,7 +841,7 @@ function resumeMountClassInstance(
const contextType = ctor.contextType;
let nextContext;
if (typeof contextType === 'object' && contextType !== null) {
nextContext = (contextType: any).unstable_read();
nextContext = readContext(contextType);
} else {
const nextLegacyUnmaskedContext = getUnmaskedContext(
workInProgress,
Expand Down Expand Up @@ -979,7 +987,7 @@ function updateClassInstance(
const contextType = ctor.contextType;
let nextContext;
if (typeof contextType === 'object' && contextType !== null) {
nextContext = (contextType: any).unstable_read();
nextContext = readContext(contextType);
} else {
const nextUnmaskedContext = getUnmaskedContext(workInProgress, ctor, true);
nextContext = getMaskedContext(workInProgress, nextUnmaskedContext);
Expand Down
2 changes: 1 addition & 1 deletion packages/react-reconciler/src/ReactFiberNewContext.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ export function readContext<T>(
if (lastContextDependency === null) {
invariant(
currentlyRenderingFiber !== null,
'Context.unstable_read(): Context can only be read while React is ' +
'Context can only be read while React is ' +
'rendering, e.g. inside the render method or getDerivedStateFromProps.',
);
// This is the first dependency in the list
Expand Down
52 changes: 16 additions & 36 deletions packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,26 +39,33 @@ describe('ReactNewContext', () => {
return {type: 'span', children: [], prop};
}

function readContext(Context, observedBits) {
const dispatcher =
React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.ReactCurrentOwner
.currentDispatcher;
return dispatcher.readContext(Context, observedBits);
}

// We have several ways of reading from context. sharedContextTests runs
// a suite of tests for a given context consumer implementation.
sharedContextTests('Context.Consumer', Context => Context.Consumer);
sharedContextTests(
'Context.unstable_read inside function component',
'readContext(Context) inside function component',
Context =>
function Consumer(props) {
const observedBits = props.unstable_observedBits;
const contextValue = Context.unstable_read(observedBits);
const contextValue = readContext(Context, observedBits);
const render = props.children;
return render(contextValue);
},
);
sharedContextTests(
'Context.unstable_read inside class component',
'readContext(Context) inside class component',
Context =>
class Consumer extends React.Component {
render() {
const observedBits = this.props.unstable_observedBits;
const contextValue = Context.unstable_read(observedBits);
const contextValue = readContext(Context, observedBits);
const render = this.props.children;
return render(contextValue);
}
Expand Down Expand Up @@ -1192,7 +1199,7 @@ describe('ReactNewContext', () => {
return (
<FooContext.Consumer>
{foo => {
const bar = BarContext.unstable_read();
const bar = readContext(BarContext);
return <Text text={`Foo: ${foo}, Bar: ${bar}`} />;
}}
</FooContext.Consumer>
Expand Down Expand Up @@ -1236,7 +1243,7 @@ describe('ReactNewContext', () => {
});
});

describe('unstable_readContext', () => {
describe('readContext', () => {
it('can use the same context multiple times in the same function', () => {
const Context = React.createContext({foo: 0, bar: 0, baz: 0}, (a, b) => {
let result = 0;
Expand All @@ -1262,13 +1269,13 @@ describe('ReactNewContext', () => {
}

function FooAndBar() {
const {foo} = Context.unstable_read(0b001);
const {bar} = Context.unstable_read(0b010);
const {foo} = readContext(Context, 0b001);
const {bar} = readContext(Context, 0b010);
return <Text text={`Foo: ${foo}, Bar: ${bar}`} />;
}

function Baz() {
const {baz} = Context.unstable_read(0b100);
const {baz} = readContext(Context, 0b100);
return <Text text={'Baz: ' + baz} />;
}

Expand Down Expand Up @@ -1631,33 +1638,6 @@ Context fuzz tester error! Copy and paste the following line into the test suite
);
});

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

function Child() {
let value = BarContext.Consumer.unstable_read();
return <div actual={value} expected="bar-updated" />;
}

function Component() {
return (
<React.Fragment>
<BarContext.Provider value={{value: 'bar-updated'}}>
<Child />
</BarContext.Provider>
</React.Fragment>
);
}

expect(() => {
ReactNoop.render(<Component />);
ReactNoop.flush();
}).toWarnDev(
'Calling Context.Consumer.unstable_read() is not supported and will be removed in ' +
'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'});

Expand Down
9 changes: 8 additions & 1 deletion packages/react-reconciler/src/__tests__/ReactPure-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,15 @@ describe('pure', () => {

const CountContext = React.createContext(0);

function readContext(Context) {
const dispatcher =
React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED
.ReactCurrentOwner.currentDispatcher;
return dispatcher.readContext(Context);
}

function Counter(props) {
const count = CountContext.unstable_read();
const count = readContext(CountContext);
return <Text text={`${props.label}: ${count}`} />;
}
Counter = pure(Counter);
Expand Down
31 changes: 0 additions & 31 deletions packages/react/src/ReactContext.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,25 +11,9 @@ import {REACT_PROVIDER_TYPE, REACT_CONTEXT_TYPE} from 'shared/ReactSymbols';

import type {ReactContext} from 'shared/ReactTypes';

import invariant from 'shared/invariant';
import warningWithoutStack from 'shared/warningWithoutStack';
import warning from 'shared/warning';

import ReactCurrentOwner from './ReactCurrentOwner';

export function readContext<T>(
context: ReactContext<T>,
observedBits: void | number | boolean,
): T {
const dispatcher = ReactCurrentOwner.currentDispatcher;
invariant(
dispatcher !== null,
'Context.unstable_read(): Context can only be read while React is ' +
'rendering, e.g. inside the render method or getDerivedStateFromProps.',
);
return dispatcher.readContext(context, observedBits);
}

export function createContext<T>(
defaultValue: T,
calculateChangedBits: ?(a: T, b: T) => number,
Expand Down Expand Up @@ -61,18 +45,14 @@ export function createContext<T>(
// These are circular
Provider: (null: any),
Consumer: (null: any),
unstable_read: (null: any),
};

context.Provider = {
$$typeof: REACT_PROVIDER_TYPE,
_context: context,
};

context.unstable_read = readContext.bind(null, context);

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

if (__DEV__) {
Expand All @@ -83,17 +63,6 @@ export function createContext<T>(
$$typeof: REACT_CONTEXT_TYPE,
_context: context,
_calculateChangedBits: context._calculateChangedBits,
unstable_read() {
if (!hasWarnedAboutUsingConsumerUnstableRead) {
hasWarnedAboutUsingConsumerUnstableRead = true;
warning(
false,
'Calling Context.Consumer.unstable_read() is not supported and will be removed in ' +
'a future major release. Did you mean to render Context.unstable_read() instead?',
);
}
return context.unstable_read();
},
};
// $FlowFixMe: Flow complains about not setting a value, which is intentional here
Object.defineProperties(Consumer, {
Expand Down
6 changes: 3 additions & 3 deletions packages/react/src/__tests__/ReactContextValidator-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ describe('ReactContextValidator', () => {
}

expect(() => {
expect(() => ReactTestUtils.renderIntoDocument(<ComponentA />)).toThrow();
ReactTestUtils.renderIntoDocument(<ComponentA />);
}).toWarnDev(
'Warning: ComponentA defines an invalid contextType. ' +
'contextType should point to the Context object returned by React.createContext(). ' +
Expand All @@ -565,10 +565,10 @@ describe('ReactContextValidator', () => {
);

// Warnings should be deduped by component type
expect(() => ReactTestUtils.renderIntoDocument(<ComponentA />)).toThrow();
ReactTestUtils.renderIntoDocument(<ComponentA />);

expect(() => {
expect(() => ReactTestUtils.renderIntoDocument(<ComponentB />)).toThrow();
ReactTestUtils.renderIntoDocument(<ComponentB />);
}).toWarnDev(
'Warning: ComponentB defines an invalid contextType. ' +
'contextType should point to the Context object returned by React.createContext(). ' +
Expand Down
1 change: 0 additions & 1 deletion packages/shared/ReactTypes.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ export type ReactContext<T> = {
$$typeof: Symbol | number,
Consumer: ReactContext<T>,
Provider: ReactProviderType<T>,
unstable_read: () => T,

_calculateChangedBits: ((a: T, b: T) => number) | null,

Expand Down

0 comments on commit 7e0b045

Please sign in to comment.