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

add an intersection version of PropType.oneOfType called PropType.allOfType #3974

Closed
wants to merge 1 commit into from

Conversation

pwmckenna
Copy link

A couple of use cases:

  • Password proptypes: can just combine proptypes for string, length, complexity, etc.
  • Object proptypes: combinations of existing proptypes (ie, if the view was accepting a prop that was a combination of the public user data and their authentication info, you could just say that object had to match both of your existing proptypes for public user data and auth info)

@samwgoldman
Copy link
Member

For what it's worth, we can definitely support this prop type in flow. One thing to watch out for: it's really easy to describe an intersection type that has no inhabitants.

@pwmckenna
Copy link
Author

The impossible to match scenario is worrisome. It wouldn't help in complex cases (where I think people would have the most trouble), but I could at least ensure that there aren't more than one of the default primitive type checks specified. Think it would be worth adding?

@samwgoldman
Copy link
Member

I am far from the right person to answer whether or not it is appropriate to try, but based on my understanding as a casual observer of this project, providing good error/warning messages is a priority.

That said, figuring out if an intersection type has no inhabitants isn't exactly simple. Sure, number & string is going to make things pretty clear, but there's also { foo: number } & { foo: string } and variants thereof. Lastly, what about () => void & { foo: string }? Is that a function with props, or an uninhabitable type? Similar issue for number[] & { foo: string }.

Apologies for using flow type syntax. It's just a lot more concise compared to PropTypes.

@zpao
Copy link
Member

zpao commented Jun 3, 2015

cc @sebmarkbage

@sebmarkbage
Copy link
Collaborator

cc @avikchaudhuri Does this make sense to add as a propType, and therefore to Flow's React bindings? Does this implementation map to the Flow semantics for intersection types? Especially for shapes.

@sebmarkbage
Copy link
Collaborator

cc @gabelevi same question

@sophiebits
Copy link
Collaborator

@gaearon
Copy link
Collaborator

gaearon commented Mar 26, 2016

I’m closing because we have been treating PropTypes as being in maintenance mode for the past year. We don’t plan to add any new features to them, and we encourage you to try using Flow or distribute third-party prop types as custom npm packages.

Thank you for sending a PR! We hope to see you contributing again. I suggest you to raise an issue to discuss any significant changes or changes to the public API before spending time to implement them because API and code review are two separate activities and it’s best to agree on API (or even intent) first.

Cheers!

@gaearon gaearon closed this Mar 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants