Skip to content

Flow 0.53 support #1377

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.

Sign up for GitHub

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

Merged
merged 13 commits into from
Aug 25, 2017
Merged

Flow 0.53 support #1377

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
3e85c55
Attempt at Flow 0.53 support
jseminck Aug 16, 2017
585b94a
Add invalid test case
jseminck Aug 16, 2017
f767f68
Add support and test for custom props name
jseminck Aug 17, 2017
deb3b61
Add invalid test case for custom prop names
jseminck Aug 17, 2017
1a8af56
Revert old implementation and replace by configurable flowVersion
jseminck Aug 18, 2017
d232811
Remove flow version and decide the Props argument location based on t…
jseminck Aug 21, 2017
1ebfcee
Add flowVersion back but without a default. When there is no flow ver…
jseminck Aug 21, 2017
a7957a1
update readme regarding the flowVersion setting
jseminck Aug 21, 2017
18ad186
Fix bug and add tests through helper function
jseminck Aug 21, 2017
bb0a457
Add explicit > check for params length
jseminck Aug 22, 2017
cd96ec9
remove test helper function as we cannot use spread operator
jseminck Aug 22, 2017
6d1b7f1
Add one more test case for the new syntax
jseminck Aug 22, 2017
a6d26d9
Move flowVersion setting in invalid tests closer to the code
jseminck Aug 22, 2017
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
6 changes: 6 additions & 0 deletions lib/rules/prop-types.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -917,6 +917,12 @@ module.exports = {
}
},

TypeParameterInstantiation: function(node) {
if (node.params && node.params[0].type === 'GenericTypeAnnotation' && node.params[0].id.name === 'Props') {
Copy link
Collaborator

@EvHaus EvHaus Aug 16, 2017

Choose a reason for hiding this comment

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

It would be best not to check for the type name here in case people want to pass in a type with a different name, eg:

type FancyPropsType = {
   ...
}

class Bar extends React.Component<FancyPropsType> { }

Copy link
Collaborator

@EvHaus EvHaus Aug 16, 2017

Choose a reason for hiding this comment

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

In fact, it would be great to add that as a unit test to ensure it is supported.

Copy link
Member

@ljharb ljharb Aug 17, 2017

Choose a reason for hiding this comment

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

good call; this definitely should be supported.

Copy link
Contributor Author

@jseminck jseminck Aug 17, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

That makes sense.

But what about this test case that we have. Is that still valid? I suppose not, as now we are passing void for Props and { person: Person } as State ? What was the third parameter?

type Person = {
  firstname: string
};
class Hello extends React.Component<void, { person: Person }, void> {
  render () {
    return <div>Hello {this.props.person.lastname}</div>;
  }
}

Copy link
Member

@ljharb ljharb Aug 17, 2017

Choose a reason for hiding this comment

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

the third might be context

markPropTypesAsDeclared(node, resolveTypeAnnotation(node.params[0]));
}
},

VariableDeclarator: function(node) {
const destructuring = node.init && node.id && node.id.type === 'ObjectPattern';
// let {props: {firstname}} = this
Expand Down
33 changes: 33 additions & 0 deletions tests/lib/rules/prop-types.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1558,6 +1558,20 @@ ruleTester.run('prop-types', rule, {
].join('\n'),
parser: 'babel-eslint'
},
{
code: `
type Props = {
foo: string,
};

class Bar extends React.Component<Props> {
render() {
return <div>{this.props.foo}</div>
}
}
`,
parser: 'babel-eslint'
},
// issue #1288
`function Foo() {
const props = {}
Expand Down Expand Up @@ -2928,6 +2942,25 @@ ruleTester.run('prop-types', rule, {
type: 'Identifier'
}],
parser: 'babel-eslint'
}, {
code: `
type Props = {
foo: string,
};

class Bar extends React.Component<Props> {
render() {
return <div>{this.props.bar}</div>
}
}
`,
errors: [{
message: '\'bar\' is missing in props validation',
line: 8,
column: 37,
type: 'Identifier'
}],
parser: 'babel-eslint'
}
]
});