Skip to content
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

Fix handling of identifiers #1356

Merged
merged 5 commits into from Aug 15, 2017
Merged

Conversation

dustinsoftware
Copy link
Contributor

@dustinsoftware dustinsoftware commented Aug 11, 2017

Fixes #1352

This fixes incorrect behavior when propTypes were stored in a variable and then referenced later. jsx-eslint#1352
@dustinsoftware dustinsoftware changed the title Fix handling of identifiers in forbid-prop-types Fix handling of identifiers Aug 11, 2017
Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

Tests look great - there's a lot of changes here tho, so it should get a few more reviews before going in.

@@ -565,6 +565,42 @@ ruleTester.run('sort-prop-types', rule, {
}]
}, {
code: [
'const First = (props) => <div />;',
'const propTypes = {',
Copy link
Member

Choose a reason for hiding this comment

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

can you also (just in case) add a test with export const propTypes = … to this rule too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, just so I can understand, what behavior will that change be verifying?

Copy link
Member

Choose a reason for hiding this comment

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

Hopefully nothing; I'm just making sure that an export declaration works just as well as a variable declaration :-)

'export const propTypes = {a: PropTypes.any};',
'export default function Component() {}',
'Component.propTypes = forbidExtraProps(propTypes);'
].join('\n'),
// errors: [{message: ANY_ERROR_MESSAGE}], // TODO: make this pass
Copy link
Member

Choose a reason for hiding this comment

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

<3

@ljharb ljharb merged commit 8365d29 into jsx-eslint:master Aug 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants