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

forbid-foreign-prop-types should allow another component's propTypes in a propTypes definition #1647

Closed
iansu opened this issue Jan 17, 2018 · 5 comments

Comments

@iansu
Copy link
Contributor

iansu commented Jan 17, 2018

Currently this results in a warning but shouldn't:

Button.propTypes = {
  ...Touchable.propTypes,
  color: PropTypes.string
}

The error message could also be improved to explain why using another component's propTypes is bad. For example: "Using propTypes from another component is not safe because they may be removed in production builds".

Comments via @gaearon on this issue: facebook/create-react-app#3818

@ljharb
Copy link
Member

ljharb commented Jan 17, 2018

I think that by default, it definitely should warn on it; however, I'd be open to adding an option that explicitly allowed this case.

@iansu
Copy link
Contributor Author

iansu commented Jan 17, 2018

Something like allowPropTypesInPropTypes? Or is that too confusing?

@iansu
Copy link
Contributor Author

iansu commented Jan 17, 2018

allowPropTypesComposition?

Also, how do you feel about softening the error message a bit?

@ljharb
Copy link
Member

ljharb commented Jan 17, 2018

I wouldn't call it "softening"; just making it more accurate :-) but yes, I'm also open to that.

For an option name, allowInPropTypes? (since it's already a subset of "foreign propTypes")

@iansu
Copy link
Contributor Author

iansu commented Jan 17, 2018

Sounds good to me. I'll work on implementing this change.

iansu added a commit to iansu/eslint-plugin-react that referenced this issue Jan 23, 2018
ljharb pushed a commit to iansu/eslint-plugin-react that referenced this issue Jan 29, 2018
ljharb pushed a commit to iansu/eslint-plugin-react that referenced this issue Jan 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants