Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unstable_read() in favor of direct dispatcher call #13861

Merged
merged 2 commits into from Oct 16, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 13 additions & 5 deletions packages/react-reconciler/src/ReactFiberClassComponent.js
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
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
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
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
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
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
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