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

Disallow reading context during useMemo etc #14653

Merged
merged 6 commits into from
Jan 23, 2019
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 1 addition & 8 deletions packages/react-reconciler/src/ReactFiberClassComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import {
import ReactStrictModeWarnings from './ReactStrictModeWarnings';
import {isMounted} from 'react-reconciler/reflection';
import {get as getInstance, set as setInstance} 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 @@ -48,20 +47,14 @@ import {
hasContextChanged,
emptyContextObject,
} from './ReactFiberContext';
import {readContext} from './ReactFiberNewContext';
import {
requestCurrentTime,
computeExpirationForFiber,
scheduleWork,
flushPassiveEffects,
} from './ReactFiberScheduler';

const ReactCurrentDispatcher = ReactSharedInternals.ReactCurrentDispatcher;

function readContext(contextType: any): any {
const dispatcher = ReactCurrentDispatcher.current;
return dispatcher.readContext(contextType);
}

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

Expand Down
20 changes: 19 additions & 1 deletion packages/react-reconciler/src/ReactFiberHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@ import type {HookEffectTag} from './ReactHookEffectTags';

import {NoWork} from './ReactFiberExpirationTime';
import {enableHooks} from 'shared/ReactFeatureFlags';
import {readContext} from './ReactFiberNewContext';
import {
readContext,
stashContextDependencies,
unstashContextDependencies,
} from './ReactFiberNewContext';
import {
Update as UpdateEffect,
Passive as PassiveEffect,
Expand Down Expand Up @@ -443,8 +447,10 @@ export function useReducer<S, A>(
const action = update.action;
// Temporarily clear to forbid calling Hooks in a reducer.
currentlyRenderingFiber = null;
stashContextDependencies();
newState = reducer(newState, action);
currentlyRenderingFiber = fiber;
unstashContextDependencies();
update = update.next;
} while (update !== null);

Expand Down Expand Up @@ -515,8 +521,10 @@ export function useReducer<S, A>(
const action = update.action;
// Temporarily clear to forbid calling Hooks in a reducer.
currentlyRenderingFiber = null;
stashContextDependencies();
newState = reducer(newState, action);
currentlyRenderingFiber = fiber;
unstashContextDependencies();
}
}
prevUpdate = update;
Expand Down Expand Up @@ -547,6 +555,7 @@ export function useReducer<S, A>(
}
// Temporarily clear to forbid calling Hooks in a reducer.
currentlyRenderingFiber = null;
stashContextDependencies();
// There's no existing queue, so this is the initial render.
if (reducer === basicStateReducer) {
// Special case for `useState`.
Expand All @@ -557,6 +566,7 @@ export function useReducer<S, A>(
initialState = reducer(initialState, initialAction);
}
currentlyRenderingFiber = fiber;
unstashContextDependencies();
workInProgressHook.memoizedState = workInProgressHook.baseState = initialState;
queue = workInProgressHook.queue = {
last: null,
Expand Down Expand Up @@ -779,8 +789,10 @@ export function useMemo<T>(

// Temporarily clear to forbid calling Hooks.
currentlyRenderingFiber = null;
stashContextDependencies();
const nextValue = nextCreate();
currentlyRenderingFiber = fiber;
unstashContextDependencies();
workInProgressHook.memoizedState = [nextValue, nextDeps];
return nextValue;
}
Expand Down Expand Up @@ -875,7 +887,13 @@ function dispatchAction<S, A>(
if (eagerReducer !== null) {
try {
const currentState: S = (queue.eagerState: any);
// Temporarily clear to forbid calling Hooks in a reducer.
let maybeFiber = currentlyRenderingFiber; // Note: likely null now unlike `fiber`
currentlyRenderingFiber = null;
stashContextDependencies();
const eagerState = eagerReducer(currentState, action);
currentlyRenderingFiber = maybeFiber;
unstashContextDependencies();
// Stash the eagerly computed state, and the reducer used to compute
// it, on the update object. If the reducer hasn't changed by the
// time we enter the render phase, then the eager state can be used
Expand Down
25 changes: 25 additions & 0 deletions packages/react-reconciler/src/ReactFiberNewContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,37 @@ let currentlyRenderingFiber: Fiber | null = null;
let lastContextDependency: ContextDependency<mixed> | null = null;
let lastContextWithAllBitsObserved: ReactContext<any> | null = null;

// We stash the variables above before entering user code in Hooks.
let stashedCurrentlyRenderingFiber: Fiber | null = null;
let stashedLastContextDependency: ContextDependency<mixed> | null = null;
let stashedLastContextWithAllBitsObserved: ReactContext<any> | null = null;

export function resetContextDependences(): void {
// This is called right before React yields execution, to ensure `readContext`
// cannot be called outside the render phase.
currentlyRenderingFiber = null;
lastContextDependency = null;
lastContextWithAllBitsObserved = null;

stashedCurrentlyRenderingFiber = null;
stashedLastContextDependency = null;
stashedLastContextWithAllBitsObserved = null;
}

export function stashContextDependencies(): void {
stashedCurrentlyRenderingFiber = currentlyRenderingFiber;
stashedLastContextDependency = lastContextDependency;
stashedLastContextWithAllBitsObserved = lastContextWithAllBitsObserved;

currentlyRenderingFiber = null;
lastContextDependency = null;
lastContextWithAllBitsObserved = null;
}

export function unstashContextDependencies(): void {
currentlyRenderingFiber = stashedCurrentlyRenderingFiber;
lastContextDependency = stashedLastContextDependency;
lastContextWithAllBitsObserved = stashedLastContextWithAllBitsObserved;
}

export function pushProvider<T>(providerFiber: Fiber, nextValue: T): void {
Expand Down
141 changes: 141 additions & 0 deletions packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js
Original file line number Diff line number Diff line change
Expand Up @@ -672,6 +672,147 @@ describe('ReactHooks', () => {
expect(root.toJSON()).toEqual('123');
});

it('throws when reading context inside useMemo', () => {
const {useMemo, createContext} = React;
const ReactCurrentDispatcher =
React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED
.ReactCurrentDispatcher;

const ThemeContext = createContext('light');
function App() {
return useMemo(() => {
return ReactCurrentDispatcher.current.readContext(ThemeContext);
}, []);
}

expect(() => ReactTestRenderer.create(<App />)).toThrow(
'Context can only be read while React is rendering',
);
});

it('throws when reading context inside useMemo after reading outside it', () => {
const {useMemo, createContext} = React;
const ReactCurrentDispatcher =
React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED
.ReactCurrentDispatcher;

const ThemeContext = createContext('light');
let firstRead, secondRead;
function App() {
firstRead = ReactCurrentDispatcher.current.readContext(ThemeContext);
useMemo(() => {});
secondRead = ReactCurrentDispatcher.current.readContext(ThemeContext);
return useMemo(() => {
return ReactCurrentDispatcher.current.readContext(ThemeContext);
}, []);
}

expect(() => ReactTestRenderer.create(<App />)).toThrow(
'Context can only be read while React is rendering',
);
expect(firstRead).toBe('light');
expect(secondRead).toBe('light');
});

it('throws when reading context inside useEffect', () => {
const {useEffect, createContext} = React;
const ReactCurrentDispatcher =
React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED
.ReactCurrentDispatcher;

const ThemeContext = createContext('light');
function App() {
useEffect(() => {
ReactCurrentDispatcher.current.readContext(ThemeContext);
});
return null;
}

const root = ReactTestRenderer.create(<App />);
expect(() => root.update(<App />)).toThrow(
// The exact message doesn't matter, just make sure we don't allow this
"Cannot read property 'readContext' of null",
);
});

it('throws when reading context inside useLayoutEffect', () => {
const {useLayoutEffect, createContext} = React;
const ReactCurrentDispatcher =
React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED
.ReactCurrentDispatcher;

const ThemeContext = createContext('light');
function App() {
useLayoutEffect(() => {
ReactCurrentDispatcher.current.readContext(ThemeContext);
});
return null;
}

expect(() => ReactTestRenderer.create(<App />)).toThrow(
// The exact message doesn't matter, just make sure we don't allow this
"Cannot read property 'readContext' of null",
);
});

it('throws when reading context inside useReducer', () => {
const {useReducer, createContext} = React;
const ReactCurrentDispatcher =
React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED
.ReactCurrentDispatcher;

const ThemeContext = createContext('light');
function App() {
useReducer(
() => {
ReactCurrentDispatcher.current.readContext(ThemeContext);
},
null,
{},
);
return null;
}

expect(() => ReactTestRenderer.create(<App />)).toThrow(
'Context can only be read while React is rendering',
);
});

// Edge case.
it('throws when reading context inside eager useReducer', () => {
const {useState, createContext} = React;
const ThemeContext = createContext('light');

const ReactCurrentDispatcher =
React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED
.ReactCurrentDispatcher;

let _setState;
function Fn() {
const [, setState] = useState(0);
_setState = setState;
return null;
}

class Cls extends React.Component {
render() {
_setState(() => {
ReactCurrentDispatcher.current.readContext(ThemeContext);
});
return null;
}
}

expect(() =>
ReactTestRenderer.create(
<React.Fragment>
<Fn />
<Cls />
</React.Fragment>,
),
).toThrow('Context can only be read while React is rendering');
});

it('throws when calling hooks inside useReducer', () => {
const {useReducer, useRef} = React;
function App() {
Expand Down