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

Apply allowInPropTypes option to class property #2040

Merged
merged 1 commit into from Nov 15, 2018

Conversation

Sheile
Copy link
Contributor

@Sheile Sheile commented Nov 12, 2018

Fix @csantos1113 problem in #2023

Problem

The allowInPropTypes option can't apply following definition for propTypes.

const space = () => {};
space.propTypes = {
    name: PropTypes.string
};

export default class List extends PureComponent {
  static propTypes = {
    ...space.propTypes, // but I'm getting the warning here!
    message: PropTypes.string
  }

  render() {
     return <div>{this.props.message}</div>
  }
}

Reason

Got following node hierarchy when define propTypes by each method, AssignmentExpression is only contained by the latter.

# define by static propTypes = {}
ExperimentalSpreadProperty
	ObjectExpression
		ClassProperty
# define by Class.propTypes = {}
ExperimentalSpreadProperty
	ObjectExpression
		AssignmentExpression

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.

Thanks! Could you add a regression test that would have caught this?

@Sheile
Copy link
Contributor Author

Sheile commented Nov 12, 2018

Thank you for your quick reply.
I've added some tests and amend previous commit.

@ljharb ljharb added the bug label Nov 15, 2018
@ljharb ljharb merged commit e133f1c into jsx-eslint:master Nov 15, 2018
@Sheile Sheile deleted the fix/allow-in-proptypes branch November 16, 2018 02:29
This was referenced Dec 28, 2018
This was referenced Jan 4, 2019
@ghost ghost mentioned this pull request Jan 12, 2019
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

2 participants