Skip to content

Commit

Permalink
Remove propTypes checks for legacy context (#28324)
Browse files Browse the repository at this point in the history 
Part of #28207, this is easy to
land in isolation.

The approach I'm taking is slightly different — instead of leaving
validation on for legacy context, I disable the validation (it's
DEV-only) and leave just the parts that drive the runtime logic. I.e.
`contexTypes` and `childContextTypes` *values* are now ignored, the keys
are used just like before.
  • Loading branch information
@gaearon
gaearon authored Feb 14, 2024
1 parent f0e808e commit 64e755d
Show file tree
Hide file tree
Showing 3 changed files with 2 additions and 171 deletions.
10 changes: 0 additions & 10 deletions packages/react-reconciler/src/ReactFiberContext.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import {isFiberMounted} from './ReactFiberTreeReflection';
import {disableLegacyContext} from 'shared/ReactFeatureFlags';
import {ClassComponent, HostRoot} from './ReactWorkTags';
import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber';
import checkPropTypes from 'shared/checkPropTypes';

import {createCursor, push, pop} from './ReactFiberStack';

Expand Down Expand Up @@ -101,11 +100,6 @@ function getMaskedContext(
context[key] = unmaskedContext[key];
}

if (__DEV__) {
const name = getComponentNameFromFiber(workInProgress) || 'Unknown';
checkPropTypes(contextTypes, context, 'context', name);
}

// Cache unmasked context so we can avoid recreating masked context unless necessary.
// Context is created before the class component is instantiated so check for instance.
if (instance) {
Expand Down Expand Up @@ -212,10 +206,6 @@ function processChildContext(
);
}
}
if (__DEV__) {
const name = getComponentNameFromFiber(fiber) || 'Unknown';
checkPropTypes(childContextTypes, childContext, 'child context', name);
}

return {...parentContext, ...childContext};
}
Expand Down
154 changes: 0 additions & 154 deletions packages/react/src/__tests__/ReactContextValidator-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
let PropTypes;
let React;
let ReactDOMClient;
let ReactDOMServer;
let ReactTestUtils;
let act;

Expand All @@ -29,7 +28,6 @@ describe('ReactContextValidator', () => {
PropTypes = require('prop-types');
React = require('react');
ReactDOMClient = require('react-dom/client');
ReactDOMServer = require('react-dom/server');
ReactTestUtils = require('react-dom/test-utils');
act = require('internal-test-utils').act;
});
Expand Down Expand Up @@ -159,136 +157,6 @@ describe('ReactContextValidator', () => {
expect(componentDidUpdateContext).toEqual({foo: 'def'});
});

// @gate !disableLegacyContext || !__DEV__
it('should check context types', () => {
class Component extends React.Component {
render() {
return <div />;
}
}
Component.contextTypes = {
foo: PropTypes.string.isRequired,
};

expect(() => ReactTestUtils.renderIntoDocument(<Component />)).toErrorDev(
'Warning: Failed context type: ' +
'The context `foo` is marked as required in `Component`, but its value ' +
'is `undefined`.\n' +
' in Component (at **)',
);

class ComponentInFooStringContext extends React.Component {
getChildContext() {
return {
foo: this.props.fooValue,
};
}

render() {
return <Component />;
}
}
ComponentInFooStringContext.childContextTypes = {
foo: PropTypes.string,
};

// No additional errors expected
ReactTestUtils.renderIntoDocument(
<ComponentInFooStringContext fooValue={'bar'} />,
);

class ComponentInFooNumberContext extends React.Component {
getChildContext() {
return {
foo: this.props.fooValue,
};
}

render() {
return <Component />;
}
}
ComponentInFooNumberContext.childContextTypes = {
foo: PropTypes.number,
};

expect(() =>
ReactTestUtils.renderIntoDocument(
<ComponentInFooNumberContext fooValue={123} />,
),
).toErrorDev(
'Warning: Failed context type: ' +
'Invalid context `foo` of type `number` supplied ' +
'to `Component`, expected `string`.\n' +
' in Component (at **)\n' +
' in ComponentInFooNumberContext (at **)',
);
});

// @gate !disableLegacyContext || !__DEV__
it('should check child context types', () => {
class Component extends React.Component {
getChildContext() {
return this.props.testContext;
}

render() {
return <div />;
}
}
Component.childContextTypes = {
foo: PropTypes.string.isRequired,
bar: PropTypes.number,
};

expect(() =>
ReactTestUtils.renderIntoDocument(<Component testContext={{bar: 123}} />),
).toErrorDev(
'Warning: Failed child context type: ' +
'The child context `foo` is marked as required in `Component`, but its ' +
'value is `undefined`.\n' +
' in Component (at **)',
);

expect(() =>
ReactTestUtils.renderIntoDocument(<Component testContext={{foo: 123}} />),
).toErrorDev(
'Warning: Failed child context type: ' +
'Invalid child context `foo` of type `number` ' +
'supplied to `Component`, expected `string`.\n' +
' in Component (at **)',
);

// No additional errors expected
ReactTestUtils.renderIntoDocument(
<Component testContext={{foo: 'foo', bar: 123}} />,
);

ReactTestUtils.renderIntoDocument(<Component testContext={{foo: 'foo'}} />);
});

it('warns of incorrect prop types on context provider', () => {
const TestContext = React.createContext();

TestContext.Provider.propTypes = {
value: PropTypes.string.isRequired,
};

ReactTestUtils.renderIntoDocument(<TestContext.Provider value="val" />);

class Component extends React.Component {
render() {
return <TestContext.Provider value={undefined} />;
}
}

expect(() => ReactTestUtils.renderIntoDocument(<Component />)).toErrorDev(
'Warning: Failed prop type: The prop `value` is marked as required in ' +
'`Context.Provider`, but its value is `undefined`.\n' +
' in Component (at **)',
);
});

// TODO (bvaughn) Remove this test and the associated behavior in the future.
// It has only been added in Fiber to match the (unintentional) behavior in Stack.
// @gate !disableLegacyContext || !__DEV__
Expand Down Expand Up @@ -371,8 +239,6 @@ describe('ReactContextValidator', () => {
'Warning: MiddleMissingContext.childContextTypes is specified but there is no ' +
'getChildContext() method on the instance. You can either define getChildContext() ' +
'on MiddleMissingContext or remove childContextTypes from it.',
'Warning: Failed context type: The context `bar` is marked as required ' +
'in `ChildContextConsumer`, but its value is `undefined`.',
]);
expect(childContext.bar).toBeUndefined();
expect(childContext.foo).toBe('FOO');
Expand Down Expand Up @@ -699,24 +565,4 @@ describe('ReactContextValidator', () => {
'Warning: ComponentB: Function components do not support contextType.',
);
});

it('should honor a displayName if set on the context type', () => {
const Context = React.createContext(null);
Context.displayName = 'MyContextType';
function Validator() {
return null;
}
Validator.propTypes = {dontPassToSeeErrorStack: PropTypes.bool.isRequired};

expect(() => {
ReactDOMServer.renderToStaticMarkup(
<Context.Provider>
<Context.Consumer>{() => <Validator />}</Context.Consumer>
</Context.Provider>,
);
}).toErrorDev(
'Warning: Failed prop type: The prop `dontPassToSeeErrorStack` is marked as required in `Validator`, but its value is `undefined`.\n' +
' in Validator (at **)',
);
});
});
9 changes: 2 additions & 7 deletions packages/react/src/__tests__/ReactJSXElementValidator-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ describe('ReactJSXElementValidator', () => {
});

// @gate !disableLegacyContext || !__DEV__
it('should warn on invalid context types', () => {
it('should not warn on invalid context types', () => {
class NullContextTypeComponent extends React.Component {
render() {
return <span>{this.props.prop}</span>;
Expand All @@ -321,12 +321,7 @@ describe('ReactJSXElementValidator', () => {
NullContextTypeComponent.contextTypes = {
prop: null,
};
expect(() =>
ReactTestUtils.renderIntoDocument(<NullContextTypeComponent />),
).toErrorDev(
'NullContextTypeComponent: context type `prop` is invalid; it must ' +
'be a function, usually from the `prop-types` package,',
);
ReactTestUtils.renderIntoDocument(<NullContextTypeComponent />);
});

it('should warn if getDefaultProps is specified on the class', () => {
Expand Down

0 comments on commit 64e755d

Please sign in to comment.