Skip to content

Commit

Permalink
[New] prop-types: Support Flow Type spread
Browse files Browse the repository at this point in the history 
Fixes #2138.
  • Loading branch information
@moroine @ljharb
moroine authored and ljharb committed Oct 5, 2019
1 parent 7fa31df commit 11dc56b
Show file tree
Hide file tree
Showing 6 changed files with 242 additions and 10 deletions.
2 changes: 1 addition & 1 deletion lib/rules/prop-types.js
View file
Expand Up @@ -103,7 +103,7 @@ module.exports = {
return true;
}
// Consider every children as declared
if (propType.children === true || propType.containsSpread || propType.containsIndexers) {
if (propType.children === true || propType.containsUnresolvedSpread || propType.containsIndexers) {
return true;
}
if (propType.acceptedProperties) {
Expand Down
3 changes: 3 additions & 0 deletions lib/util/ast.js
View file
Expand Up @@ -165,6 +165,9 @@ function getKeyValue(context, node) {
stripQuotes(tokens[0].value)
);
}
if (node.type === 'GenericTypeAnnotation') {
return node.id.name;
}
const key = node.key || node.argument;
return key.type === 'Identifier' ? key.name : key.value;
}
Expand Down
42 changes: 34 additions & 8 deletions lib/util/propTypes.js
View file
Expand Up @@ -32,13 +32,19 @@ function isSuperTypeParameterPropsDeclaration(node) {
* @param {Object[]} properties Array of properties to iterate.
* @param {Function} fn Function to call on each property, receives property key
and property value. (key, value) => void
* @param {Function} [handleSpreadFn] Function to call on each ObjectTypeSpreadProperty, receives the
argument
*/
function iterateProperties(context, properties, fn) {
function iterateProperties(context, properties, fn, handleSpreadFn) {
if (properties && properties.length && typeof fn === 'function') {
for (let i = 0, j = properties.length; i < j; i++) {
const node = properties[i];
const key = getKeyValue(context, node);

if (node.type === 'ObjectTypeSpreadProperty' && typeof handleSpreadFn === 'function') {
handleSpreadFn(node.argument);
}

const value = node.value;
fn(key, value, node);
}
Expand Down Expand Up @@ -121,30 +127,41 @@ module.exports = function propTypesInstructions(context, components, utils) {
},

ObjectTypeAnnotation(annotation, parentName, seen) {
let containsObjectTypeSpread = false;
let containsUnresolvedObjectTypeSpread = false;
let containsSpread = false;
const containsIndexers = Boolean(annotation.indexers && annotation.indexers.length);
const shapeTypeDefinition = {
type: 'shape',
children: {}
};
iterateProperties(context, annotation.properties, (childKey, childValue, propNode) => {
const fullName = [parentName, childKey].join('.');
if (!childKey && !childValue) {
containsObjectTypeSpread = true;
} else {
if (childKey || childValue) {
const types = buildTypeAnnotationDeclarationTypes(childValue, fullName, seen);
types.fullName = fullName;
types.name = childKey;
types.node = propNode;
types.isRequired = !childValue.optional;
shapeTypeDefinition.children[childKey] = types;
}
},
(spreadNode) => {
const key = getKeyValue(context, spreadNode);
const types = buildTypeAnnotationDeclarationTypes(spreadNode, key, seen);
if (!types.children) {
containsUnresolvedObjectTypeSpread = true;
} else {
Object.assign(shapeTypeDefinition, types.children);
}
containsSpread = true;
});

// Mark if this shape has spread or an indexer. We will know to consider all props from this shape as having propTypes,
// but still have the ability to detect unused children of this shape.
shapeTypeDefinition.containsSpread = containsObjectTypeSpread;
shapeTypeDefinition.containsUnresolvedSpread = containsUnresolvedObjectTypeSpread;
shapeTypeDefinition.containsIndexers = containsIndexers;
// Deprecated: containsSpread is not used anymore in the codebase, ensure to keep API backward compatibility
shapeTypeDefinition.containsSpread = containsSpread;

return shapeTypeDefinition;
},
Expand Down Expand Up @@ -241,7 +258,7 @@ module.exports = function propTypesInstructions(context, components, utils) {
}

/**
* Marks all props found inside ObjectTypeAnnotaiton as declared.
* Marks all props found inside ObjectTypeAnnotation as declared.
*
* Modifies the declaredProperties object
* @param {ASTNode} propTypes
Expand All @@ -253,7 +270,7 @@ module.exports = function propTypesInstructions(context, components, utils) {

iterateProperties(context, propTypes.properties, (key, value, propNode) => {
if (!value) {
ignorePropsValidation = true;
ignorePropsValidation = ignorePropsValidation || propNode.type !== 'ObjectTypeSpreadProperty';
return;
}

Expand All @@ -263,6 +280,15 @@ module.exports = function propTypesInstructions(context, components, utils) {
types.node = propNode;
types.isRequired = !propNode.optional;
declaredPropTypes[key] = types;
}, (spreadNode) => {
const key = getKeyValue(context, spreadNode);
const spreadAnnotation = getInTypeScope(key);
if (!spreadAnnotation) {
ignorePropsValidation = true;
} else {
const spreadIgnoreValidation = declarePropTypesForObjectTypeAnnotation(spreadAnnotation, declaredPropTypes);
ignorePropsValidation = ignorePropsValidation || spreadIgnoreValidation;
}
});

return ignorePropsValidation;
Expand Down
117 changes: 116 additions & 1 deletion tests/lib/rules/default-props-match-prop-types.js
View file
Expand Up @@ -733,6 +733,58 @@ ruleTester.run('default-props-match-prop-types', rule, {
].join('\n'),
parser: parsers.BABEL_ESLINT
},
{
code: [
'type DefaultProps1 = {|',
' bar1?: string',
'|};',
'type DefaultProps2 = {|',
' ...DefaultProps1,',
' bar2?: string',
'|};',
'type Props = {',
' foo: string,',
' ...DefaultProps2',
'};',

'function Hello(props: Props) {',
' return <div>Hello {props.foo}</div>;',
'}',

'Hello.defaultProps = {',
' bar1: "bar1",',
' bar2: "bar2",',
'};'
].join('\n'),
parser: parsers.BABEL_ESLINT
},
{
code: [
'type DefaultProps1 = {|',
' bar1?: string',
'|};',
'type DefaultProps2 = {|',
' ...DefaultProps1,',
' bar2?: string',
'|};',
'type Props = {',
' foo: string,',
' ...DefaultProps2',
'};',

'class Hello extends React.Component<Props> {',
' render() {',
' return <div>Hello {props.foo}</div>;',
' }',
'}',

'Hello.defaultProps = {',
' bar1: "bar1",',
' bar2: "bar2",',
'};'
].join('\n'),
parser: parsers.BABEL_ESLINT
},
// don't error when variable is not in scope
{
code: [
Expand Down Expand Up @@ -1460,7 +1512,6 @@ ruleTester.run('default-props-match-prop-types', rule, {
column: 3
}]
},
// Investigate why this test fails. Flow type not finding foo?
{
code: [
'function Hello(props: { foo: string }) {',
Expand Down Expand Up @@ -1590,6 +1641,70 @@ ruleTester.run('default-props-match-prop-types', rule, {
message: 'defaultProp "firstProperty" defined for isRequired propType.'
}
]
},
{
code: [
'type DefaultProps = {',
' baz?: string,',
' bar?: string',
'};',

'type Props = {',
' foo: string,',
' ...DefaultProps',
'}',

'function Hello(props: Props) {',
' return <div>Hello {props.foo}</div>;',
'}',
'Hello.defaultProps = { foo: "foo", frob: "frob", baz: "bar" };'
].join('\n'),
parser: parsers.BABEL_ESLINT,
errors: [
{
message: 'defaultProp "foo" defined for isRequired propType.',
line: 12,
column: 24
},
{
message: 'defaultProp "frob" has no corresponding propTypes declaration.',
line: 12,
column: 36
}
]
},
{
code: [
'type DefaultProps = {',
' baz?: string,',
' bar?: string',
'};',

'type Props = {',
' foo: string,',
' ...DefaultProps',
'}',

'class Hello extends React.Component<Props> {',
' render() {',
' return <div>Hello {props.foo}</div>;',
' }',
'}',
'Hello.defaultProps = { foo: "foo", frob: "frob", baz: "bar" };'
].join('\n'),
parser: parsers.BABEL_ESLINT,
errors: [
{
message: 'defaultProp "foo" defined for isRequired propType.',
line: 14,
column: 24
},
{
message: 'defaultProp "frob" has no corresponding propTypes declaration.',
line: 14,
column: 36
}
]
}
]
});
23 changes: 23 additions & 0 deletions tests/lib/rules/no-unused-prop-types.js
View file
Expand Up @@ -4619,6 +4619,29 @@ ruleTester.run('no-unused-prop-types', rule, {
errors: [{
message: '\'aProp\' PropType is defined but prop is never used'
}]
}, {
// issue #2138
code: `
type UsedProps = {|
usedProp: number,
|};
type UnusedProps = {|
unusedProp: number,
|};
type Props = {| ...UsedProps, ...UnusedProps |};
function MyComponent({ usedProp, notOne }: Props) {
return <div>{usedProp}</div>;
}
`,
parser: parsers.BABEL_ESLINT,
errors: [{
message: "'unusedProp' PropType is defined but prop is never used",
line: 7,
column: 23
}]
}, {
code: `
type Props = {
Expand Down
65 changes: 65 additions & 0 deletions tests/lib/rules/prop-types.js
View file
Expand Up @@ -1632,6 +1632,23 @@ ruleTester.run('prop-types', rule, {
'}'
].join('\n'),
parser: parsers.BABEL_ESLINT
}, {
code: [
'type OtherProps = {',
' firstname: string,',
'};',
'type Props = {',
' ...OtherProps,',
' lastname: string',
'};',
'class Hello extends React.Component {',
' props: Props;',
' render () {',
' return <div>Hello {this.props.firstname}</div>;',
' }',
'}'
].join('\n'),
parser: parsers.BABEL_ESLINT
}, {
code: [
'type Person = {',
Expand Down Expand Up @@ -2335,6 +2352,30 @@ ruleTester.run('prop-types', rule, {
`,
parser: parsers.BABEL_ESLINT
},
{
// issue #2138
code: `
type UsedProps = {|
usedProp: number,
|};
type UnusedProps = {|
unusedProp: number,
|};
type Props = {| ...UsedProps, ...UnusedProps |};
function MyComponent({ usedProp }: Props) {
return <div>{usedProp}</div>;
}
`,
parser: parsers.BABEL_ESLINT,
errors: [{
message: "'notOne' is missing in props validation",
line: 8,
column: 34
}]
},
{
// issue #1259
code: `
Expand Down Expand Up @@ -4728,6 +4769,30 @@ ruleTester.run('prop-types', rule, {
message: '\'initialValues\' is missing in props validation'
}]
},
{
// issue #2138
code: `
type UsedProps = {|
usedProp: number,
|};
type UnusedProps = {|
unusedProp: number,
|};
type Props = {| ...UsedProps, ...UnusedProps |};
function MyComponent({ usedProp, notOne }: Props) {
return <div>{usedProp}</div>;
}
`,
parser: parsers.BABEL_ESLINT,
errors: [{
message: "'notOne' is missing in props validation",
line: 12,
column: 42
}]
},
{
// issue #2298
code: `
Expand Down

0 comments on commit 11dc56b

Please sign in to comment.