Skip to content

[DevTools] Support FunctionComponent.contextTypes #19028

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

Closed
wants to merge 18 commits into from
Closed
Show file tree
Hide file tree
Changes from 17 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
43 changes: 37 additions & 6 deletions packages/react-debug-tools/src/ReactDebugHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -568,9 +568,10 @@ function processDebugValues(
}

export function inspectHooks<Props>(
renderFunction: Props => React$Node,
renderFunction: (Props, any) => React$Node,
props: Props,
currentDispatcher: ?CurrentDispatcherRef,
legacyContext: any,
): HooksTree {
// DevTools will pass the current renderer's injected dispatcher.
// Other apps might compile debug hooks as part of their app though.
Expand All @@ -584,7 +585,11 @@ export function inspectHooks<Props>(
let ancestorStackError;
try {
ancestorStackError = new Error();
renderFunction(props);
if (legacyContext) {
renderFunction(props, legacyContext);
} else {
renderFunction(props);
}
} finally {
readHookLog = hookLog;
hookLog = [];
Expand All @@ -594,7 +599,22 @@ export function inspectHooks<Props>(
return buildTree(rootStack, readHookLog);
}

function setupContexts(contextMap: Map<ReactContext<any>, any>, fiber: Fiber) {
function getLegacyContext(fiber: Fiber): Object | null {
const hasLegacyContext = !!fiber.elementType.contextTypes;
if (hasLegacyContext) {
let current = fiber;
while (current !== null) {
const instance = current.stateNode;
if (instance && instance.__reactInternalMemoizedMergedChildContext) {
return instance.__reactInternalMemoizedMergedChildContext;
}
current = current.return;
}
}
return null;
}

function setupContext(contextMap: Map<ReactContext<any>, any>, fiber: Fiber) {
let current = fiber;
while (current) {
if (current.tag === ContextProvider) {
Expand Down Expand Up @@ -686,16 +706,27 @@ export function inspectHooksOfFiber(
currentHook = (fiber.memoizedState: Hook);
const contextMap = new Map();
try {
setupContexts(contextMap, fiber);
if (fiber.tag === ForwardRef) {
if (fiber.tag === FunctionComponent) {
const hasLegacyContext = !!fiber.elementType.contextTypes;
if (hasLegacyContext) {
const legacyContext = getLegacyContext(fiber);
return inspectHooks(type, props, currentDispatcher, legacyContext);
} else {
setupContext(contextMap, fiber);
return inspectHooks(type, props, currentDispatcher);
}
} else if (fiber.tag === ForwardRef) {
setupContext(contextMap, fiber);
return inspectHooksOfForwardRef(
type.render,
props,
fiber.ref,
currentDispatcher,
);
} else {
setupContext(contextMap, fiber);
return inspectHooks(type, props, currentDispatcher);
}
return inspectHooks(type, props, currentDispatcher);
} finally {
currentHook = null;
restoreContexts(contextMap);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
'use strict';

let React;
let PropTypes;
let ReactTestRenderer;
let Scheduler;
let ReactDebugTools;
Expand All @@ -20,6 +21,7 @@ describe('ReactHooksInspectionIntegration', () => {
beforeEach(() => {
jest.resetModules();
React = require('react');
PropTypes = require('prop-types');
ReactTestRenderer = require('react-test-renderer');
Scheduler = require('scheduler');
act = ReactTestRenderer.act;
Expand Down Expand Up @@ -335,6 +337,46 @@ describe('ReactHooksInspectionIntegration', () => {
]);
});

it('should inject legacy context which is not a hook', () => {
class LegacyContextProvider extends React.Component<any> {
static childContextTypes = {
string: PropTypes.string,
};

getChildContext() {
return {
string: 'abc',
};
}

render() {
return this.props.children;
}
}

function FunctionalLegacyContextConsumer(props, context) {
return <div>{context.string}</div>;
}
FunctionalLegacyContextConsumer.contextTypes = {
string: PropTypes.string,
};
const renderer = ReactTestRenderer.create(
<LegacyContextProvider>
<FunctionalLegacyContextConsumer />
</LegacyContextProvider>,
);

const childFiber = renderer.root
.findByType(FunctionalLegacyContextConsumer)
._currentFiber();
const hasLegacyContext = !!childFiber.elementType.contextTypes;
if (hasLegacyContext) {
Copy link
Contributor

@bvaughn bvaughn Sep 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the test doing this check? Seems kind of like lower-level implementation details, and could also lead to a false positive (e.g. replace with !!childFiber.elementType.contextTypess above and the test would still pass because the boolean check would just fail silently).

Since we know the legacy context is being used here, why do we need a conditional at all?

The test also doesn't verify that the expected context value gets injected.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was my probably inappropriate attempt to prevent test from running on versions where context should be deprecated.
Removed a conditional & added value verification.

expect(() =>
ReactDebugTools.inspectHooksOfFiber(childFiber),
).not.toThrow();
}
});

it('should inspect custom hooks', () => {
function useCustom() {
const [value] = React.useState('hello');
Expand Down
18 changes: 18 additions & 0 deletions packages/react-devtools-shared/src/backend/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ type ReactTypeOfSideEffectType = {|
|};

// Some environments (e.g. React Native / Hermes) don't support the performance API yet.

const getCurrentTime =
typeof performance === 'object' && typeof performance.now === 'function'
? () => performance.now()
Expand Down Expand Up @@ -2250,6 +2251,23 @@ export function attach(
if (!shouldHideContext) {
context = stateNode.context;
}
} else if (tag === FunctionComponent) {
const hasLegacyContext = !!fiber.elementType.contextTypes;
if (hasLegacyContext) {
let current = fiber.return;
let childContextFound = false;
while (current !== null && childContextFound === false) {
const instance = current.stateNode;
if (
instance &&
instance.__reactInternalMemoizedMergedChildContext
) {
childContextFound = true;
context = instance.__reactInternalMemoizedMergedChildContext;
}
current = current.return;
}
}
}
} else if (
typeSymbol === CONTEXT_NUMBER ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,14 @@ class LegacyContextConsumer extends Component<any> {
}
}

function FunctionalLegacyContextConsumer(props, context) {
return null;
}

FunctionalLegacyContextConsumer.contextTypes = {
null: PropTypes.any,
};

const ModernContext = createContext();
ModernContext.displayName = 'ModernContext';
const ArrayContext = createContext(contextData.array);
Expand Down Expand Up @@ -92,22 +100,33 @@ class ModernContextType extends Component<any> {
}
}

function FunctionalContextConsumer() {
function FunctionalModernContextConsumer() {
useContext(StringContext);
return null;
}

function FunctionalLegacyAndModernContextConsumer(props, context) {
useContext(StringContext);
return null;
}

FunctionalLegacyAndModernContextConsumer.contextTypes = {
null: PropTypes.any,
};

export default function Contexts() {
return (
<Fragment>
<LegacyContextProvider>
<LegacyContextConsumer />
<FunctionalLegacyContextConsumer />
<FunctionalLegacyAndModernContextConsumer />
</LegacyContextProvider>
<ModernContext.Provider value={contextData}>
<ModernContext.Consumer>{value => null}</ModernContext.Consumer>
<ModernContextType />
</ModernContext.Provider>
<FunctionalContextConsumer />
<FunctionalModernContextConsumer />
<ArrayContext.Consumer>{value => null}</ArrayContext.Consumer>
<BoolContext.Consumer>{value => null}</BoolContext.Consumer>
<FuncContext.Consumer>{value => null}</FuncContext.Consumer>
Expand Down