Skip to content

Commit

Permalink
Don't bypass useContext to go straight to readContext anymore
Browse files Browse the repository at this point in the history
This adds a small amount of overhead, so I'm not sure if it will fly. I think it might be necessary though in order to support the react-debug-tools package.
  • Loading branch information
Brian Vaughn committed Feb 24, 2019
1 parent e777462 commit 1e9f755
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 30 deletions.
5 changes: 1 addition & 4 deletions packages/react-debug-tools/src/ReactDebugHooks.js
Expand Up @@ -93,10 +93,7 @@ function useContext<T>(
context: ReactContext<T>,
observedBits: void | number | boolean,
): T {
if (__DEV__) {
// ReactFiberHooks only adds context to the hooks list in DEV.
nextHook();
}
nextHook();
hookLog.push({
primitive: 'Context',
stackError: new Error(),
Expand Down
Expand Up @@ -431,27 +431,30 @@ describe('ReactHooksInspectionIntegration', () => {
// This test case is based on an open source bug report:
// facebookincubator/redux-react-hook/issues/34#issuecomment-466693787
it('should properly advance the current hook for useContext', () => {
const MyContext = React.createContext(123);

let hasInitializedState = false;
const initializeStateOnce = () => {
if (hasInitializedState) {
throw Error(
'State initialization function should only be called once.',
);
}
hasInitializedState = true;
return {foo: 'abc'};
};
const MyContext = React.createContext(1);

let incrementCount;

function Foo(props) {
React.useContext(MyContext);
const [data] = React.useState(initializeStateOnce);
return <div>foo: {data.foo}</div>;
const context = React.useContext(MyContext);
const [data, setData] = React.useState({ count: context });

incrementCount = () => setData(data => ({count: data.count + 1 }));

return <div>count: {data.count}</div>;
}

const renderer = ReactTestRenderer.create(<Foo />);
expect(renderer.toJSON()).toEqual({ type: 'div', props: {}, children: [ 'count: ', '1' ] });

act(incrementCount);
expect(renderer.toJSON()).toEqual({ type: 'div', props: {}, children: [ 'count: ', '2' ] });

const childFiber = renderer.root._currentFiber();
ReactDebugTools.inspectHooksOfFiber(childFiber);
const tree = ReactDebugTools.inspectHooksOfFiber(childFiber);
expect(tree).toEqual([
{name: 'Context', value: 1, subHooks: []},
{name: 'State', value: {count: 2}, subHooks: []},
]);
});
});
14 changes: 4 additions & 10 deletions packages/react-reconciler/src/ReactFiberHooks.js
Expand Up @@ -524,21 +524,15 @@ function mountContext<T>(
context: ReactContext<T>,
observedBits: void | number | boolean,
): T {
if (__DEV__) {
// If this DEV conditional is ever removed, update ReactDebugHooks useContext too.
mountWorkInProgressHook();
}
mountWorkInProgressHook();
return readContext(context, observedBits);
}

function updateContext<T>(
context: ReactContext<T>,
observedBits: void | number | boolean,
): T {
if (__DEV__) {
// If this DEV conditional is ever removed, update ReactDebugHooks useContext too.
updateWorkInProgressHook();
}
updateWorkInProgressHook();
return readContext(context, observedBits);
}

Expand Down Expand Up @@ -1156,7 +1150,7 @@ const HooksDispatcherOnMount: Dispatcher = {
readContext,

useCallback: mountCallback,
useContext: readContext,
useContext: mountContext,
useEffect: mountEffect,
useImperativeHandle: mountImperativeHandle,
useLayoutEffect: mountLayoutEffect,
Expand All @@ -1171,7 +1165,7 @@ const HooksDispatcherOnUpdate: Dispatcher = {
readContext,

useCallback: updateCallback,
useContext: readContext,
useContext: updateContext,
useEffect: updateEffect,
useImperativeHandle: updateImperativeHandle,
useLayoutEffect: updateLayoutEffect,
Expand Down

0 comments on commit 1e9f755

Please sign in to comment.