Skip to content

Commit a129259

Browse files
gaearongaearon
authored
Disallow reading context during useMemo etc (#14653)
* Revert "Revert "Double-render function components with Hooks in DEV in StrictMode" (#14652)" This reverts commit 3fbebb2. * Revert "Revert "Disallow reading context during useMemo etc" (#14651)" This reverts commit 5fce648. * Add extra passing test for an edge case Mentioned by @acdlite to watch out for * More convoluted test * Don't rely on expirationTime Addresses @acdlite's concerns * Edge case: forbid readContext() during eager reducer
1 parent c068d31 commit a129259

File tree

4 files changed

+186
-9
lines changed

4 files changed

+186
-9
lines changed

‎packages/react-reconciler/src/ReactFiberClassComponent.js‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import {
2020
import ReactStrictModeWarnings from './ReactStrictModeWarnings';
2121
import {isMounted} from 'react-reconciler/reflection';
2222
import {get as getInstance, set as setInstance} from 'shared/ReactInstanceMap';
23-
import ReactSharedInternals from 'shared/ReactSharedInternals';
2423
import shallowEqual from 'shared/shallowEqual';
2524
import getComponentName from 'shared/getComponentName';
2625
import invariant from 'shared/invariant';
@@ -48,20 +47,14 @@ import {
4847
hasContextChanged,
4948
emptyContextObject,
5049
} from './ReactFiberContext';
50+
import {readContext} from './ReactFiberNewContext';
5151
import {
5252
requestCurrentTime,
5353
computeExpirationForFiber,
5454
scheduleWork,
5555
flushPassiveEffects,
5656
} from './ReactFiberScheduler';
5757

58-
const ReactCurrentDispatcher = ReactSharedInternals.ReactCurrentDispatcher;
59-
60-
function readContext(contextType: any): any {
61-
const dispatcher = ReactCurrentDispatcher.current;
62-
return dispatcher.readContext(contextType);
63-
}
64-
6558
const fakeInternalInstance = {};
6659
const isArray = Array.isArray;
6760

‎packages/react-reconciler/src/ReactFiberHooks.js‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,11 @@ import type {HookEffectTag} from './ReactHookEffectTags';
1414

1515
import {NoWork} from './ReactFiberExpirationTime';
1616
import {enableHooks} from 'shared/ReactFeatureFlags';
17-
import {readContext} from './ReactFiberNewContext';
17+
import {
18+
readContext,
19+
stashContextDependencies,
20+
unstashContextDependencies,
21+
} from './ReactFiberNewContext';
1822
import {
1923
Update as UpdateEffect,
2024
Passive as PassiveEffect,
@@ -600,8 +604,10 @@ export function useReducer<S, A>(
600604
const action = update.action;
601605
// Temporarily clear to forbid calling Hooks in a reducer.
602606
currentlyRenderingFiber = null;
607+
stashContextDependencies();
603608
newState = reducer(newState, action);
604609
currentlyRenderingFiber = fiber;
610+
unstashContextDependencies();
605611
update = update.next;
606612
} while (update !== null);
607613

@@ -672,8 +678,10 @@ export function useReducer<S, A>(
672678
const action = update.action;
673679
// Temporarily clear to forbid calling Hooks in a reducer.
674680
currentlyRenderingFiber = null;
681+
stashContextDependencies();
675682
newState = reducer(newState, action);
676683
currentlyRenderingFiber = fiber;
684+
unstashContextDependencies();
677685
}
678686
}
679687
prevUpdate = update;
@@ -704,6 +712,7 @@ export function useReducer<S, A>(
704712
}
705713
// Temporarily clear to forbid calling Hooks in a reducer.
706714
currentlyRenderingFiber = null;
715+
stashContextDependencies();
707716
// There's no existing queue, so this is the initial render.
708717
if (reducer === basicStateReducer) {
709718
// Special case for `useState`.
@@ -714,6 +723,7 @@ export function useReducer<S, A>(
714723
initialState = reducer(initialState, initialAction);
715724
}
716725
currentlyRenderingFiber = fiber;
726+
unstashContextDependencies();
717727
workInProgressHook.memoizedState = workInProgressHook.baseState = initialState;
718728
queue = workInProgressHook.queue = {
719729
last: null,
@@ -947,8 +957,10 @@ export function useMemo<T>(
947957

948958
// Temporarily clear to forbid calling Hooks.
949959
currentlyRenderingFiber = null;
960+
stashContextDependencies();
950961
const nextValue = nextCreate();
951962
currentlyRenderingFiber = fiber;
963+
unstashContextDependencies();
952964
workInProgressHook.memoizedState = [nextValue, nextDeps];
953965
currentHookNameInDev = null;
954966
return nextValue;
@@ -1044,7 +1056,13 @@ function dispatchAction<S, A>(
10441056
if (eagerReducer !== null) {
10451057
try {
10461058
const currentState: S = (queue.eagerState: any);
1059+
// Temporarily clear to forbid calling Hooks in a reducer.
1060+
let maybeFiber = currentlyRenderingFiber; // Note: likely null now unlike `fiber`
1061+
currentlyRenderingFiber = null;
1062+
stashContextDependencies();
10471063
const eagerState = eagerReducer(currentState, action);
1064+
currentlyRenderingFiber = maybeFiber;
1065+
unstashContextDependencies();
10481066
// Stash the eagerly computed state, and the reducer used to compute
10491067
// it, on the update object. If the reducer hasn't changed by the
10501068
// time we enter the render phase, then the eager state can be used

‎packages/react-reconciler/src/ReactFiberNewContext.js‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,37 @@ let currentlyRenderingFiber: Fiber | null = null;
5252
let lastContextDependency: ContextDependency<mixed> | null = null;
5353
let lastContextWithAllBitsObserved: ReactContext<any> | null = null;
5454

55+
// We stash the variables above before entering user code in Hooks.
56+
let stashedCurrentlyRenderingFiber: Fiber | null = null;
57+
let stashedLastContextDependency: ContextDependency<mixed> | null = null;
58+
let stashedLastContextWithAllBitsObserved: ReactContext<any> | null = null;
59+
5560
export function resetContextDependences(): void {
5661
// This is called right before React yields execution, to ensure `readContext`
5762
// cannot be called outside the render phase.
5863
currentlyRenderingFiber = null;
5964
lastContextDependency = null;
6065
lastContextWithAllBitsObserved = null;
66+
67+
stashedCurrentlyRenderingFiber = null;
68+
stashedLastContextDependency = null;
69+
stashedLastContextWithAllBitsObserved = null;
70+
}
71+
72+
export function stashContextDependencies(): void {
73+
stashedCurrentlyRenderingFiber = currentlyRenderingFiber;
74+
stashedLastContextDependency = lastContextDependency;
75+
stashedLastContextWithAllBitsObserved = lastContextWithAllBitsObserved;
76+
77+
currentlyRenderingFiber = null;
78+
lastContextDependency = null;
79+
lastContextWithAllBitsObserved = null;
80+
}
81+
82+
export function unstashContextDependencies(): void {
83+
currentlyRenderingFiber = stashedCurrentlyRenderingFiber;
84+
lastContextDependency = stashedLastContextDependency;
85+
lastContextWithAllBitsObserved = stashedLastContextWithAllBitsObserved;
6186
}
6287

6388
export function pushProvider<T>(providerFiber: Fiber, nextValue: T): void {

‎packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js‎

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -672,6 +672,147 @@ describe('ReactHooks', () => {
672672
expect(root.toJSON()).toEqual('123');
673673
});
674674

675+
it('throws when reading context inside useMemo', () => {
676+
const {useMemo, createContext} = React;
677+
const ReactCurrentDispatcher =
678+
React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED
679+
.ReactCurrentDispatcher;
680+
681+
const ThemeContext = createContext('light');
682+
function App() {
683+
return useMemo(() => {
684+
return ReactCurrentDispatcher.current.readContext(ThemeContext);
685+
}, []);
686+
}
687+
688+
expect(() => ReactTestRenderer.create(<App />)).toThrow(
689+
'Context can only be read while React is rendering',
690+
);
691+
});
692+
693+
it('throws when reading context inside useMemo after reading outside it', () => {
694+
const {useMemo, createContext} = React;
695+
const ReactCurrentDispatcher =
696+
React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED
697+
.ReactCurrentDispatcher;
698+
699+
const ThemeContext = createContext('light');
700+
let firstRead, secondRead;
701+
function App() {
702+
firstRead = ReactCurrentDispatcher.current.readContext(ThemeContext);
703+
useMemo(() => {});
704+
secondRead = ReactCurrentDispatcher.current.readContext(ThemeContext);
705+
return useMemo(() => {
706+
return ReactCurrentDispatcher.current.readContext(ThemeContext);
707+
}, []);
708+
}
709+
710+
expect(() => ReactTestRenderer.create(<App />)).toThrow(
711+
'Context can only be read while React is rendering',
712+
);
713+
expect(firstRead).toBe('light');
714+
expect(secondRead).toBe('light');
715+
});
716+
717+
it('throws when reading context inside useEffect', () => {
718+
const {useEffect, createContext} = React;
719+
const ReactCurrentDispatcher =
720+
React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED
721+
.ReactCurrentDispatcher;
722+
723+
const ThemeContext = createContext('light');
724+
function App() {
725+
useEffect(() => {
726+
ReactCurrentDispatcher.current.readContext(ThemeContext);
727+
});
728+
return null;
729+
}
730+
731+
const root = ReactTestRenderer.create(<App />);
732+
expect(() => root.update(<App />)).toThrow(
733+
// The exact message doesn't matter, just make sure we don't allow this
734+
"Cannot read property 'readContext' of null",
735+
);
736+
});
737+
738+
it('throws when reading context inside useLayoutEffect', () => {
739+
const {useLayoutEffect, createContext} = React;
740+
const ReactCurrentDispatcher =
741+
React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED
742+
.ReactCurrentDispatcher;
743+
744+
const ThemeContext = createContext('light');
745+
function App() {
746+
useLayoutEffect(() => {
747+
ReactCurrentDispatcher.current.readContext(ThemeContext);
748+
});
749+
return null;
750+
}
751+
752+
expect(() => ReactTestRenderer.create(<App />)).toThrow(
753+
// The exact message doesn't matter, just make sure we don't allow this
754+
"Cannot read property 'readContext' of null",
755+
);
756+
});
757+
758+
it('throws when reading context inside useReducer', () => {
759+
const {useReducer, createContext} = React;
760+
const ReactCurrentDispatcher =
761+
React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED
762+
.ReactCurrentDispatcher;
763+
764+
const ThemeContext = createContext('light');
765+
function App() {
766+
useReducer(
767+
() => {
768+
ReactCurrentDispatcher.current.readContext(ThemeContext);
769+
},
770+
null,
771+
{},
772+
);
773+
return null;
774+
}
775+
776+
expect(() => ReactTestRenderer.create(<App />)).toThrow(
777+
'Context can only be read while React is rendering',
778+
);
779+
});
780+
781+
// Edge case.
782+
it('throws when reading context inside eager useReducer', () => {
783+
const {useState, createContext} = React;
784+
const ThemeContext = createContext('light');
785+
786+
const ReactCurrentDispatcher =
787+
React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED
788+
.ReactCurrentDispatcher;
789+
790+
let _setState;
791+
function Fn() {
792+
const [, setState] = useState(0);
793+
_setState = setState;
794+
return null;
795+
}
796+
797+
class Cls extends React.Component {
798+
render() {
799+
_setState(() => {
800+
ReactCurrentDispatcher.current.readContext(ThemeContext);
801+
});
802+
return null;
803+
}
804+
}
805+
806+
expect(() =>
807+
ReactTestRenderer.create(
808+
<React.Fragment>
809+
<Fn />
810+
<Cls />
811+
</React.Fragment>,
812+
),
813+
).toThrow('Context can only be read while React is rendering');
814+
});
815+
675816
it('throws when calling hooks inside useReducer', () => {
676817
const {useReducer, useRef} = React;
677818
function App() {

0 commit comments

Comments
 (0)