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

Proposals for new React.PropTypes #6166

Closed
chicoxyzzy opened this issue Mar 2, 2016 · 14 comments
Closed

Proposals for new React.PropTypes #6166

chicoxyzzy opened this issue Mar 2, 2016 · 14 comments

Comments

@chicoxyzzy
Copy link
Contributor

PropTypes.exactly

I took use case below from some gitter chat where @JustBlackBird wanted some property to be String or exactly false. So maybe he can add some comments here.

Use case

PropTypes.oneOfType([
  PropTypes.string,
  PropTypes.exactly(false)
]);

instead of

PropTypes.oneOfType([
  PropTypes.string,
  PropTypes.oneOf([false])
]);

PropTypes.range

It is very common case when we need prop to be a Number in some range. Let's say it should be percent value from 0 to 100 or other inputs with type range for example.

Use cases:

PropTypes.range(0, 100);

or even

PropTypes.number.integer.range(0, 100); // only integer numbers. subproposal!

Another example

PropTypes.range(-Infinity, -0); // negative numbers only
@chicoxyzzy
Copy link
Contributor Author

actually I don't sure PropTypes.range is a good idea because prop may become invalid only in production and this will lead to potential errors when developers rely on PropTypes and don't validate props inside their components but I'll leave this proposal here for further discussion

@quantizor
Copy link
Contributor

@chicoxyzzy PropType checking is disabled in production

@chicoxyzzy
Copy link
Contributor Author

@yaycmyk sure. That's why devs may not have warnings in production and if they do not validate data in their components this will lead to potential production-only errors which is bad thing

@quantizor
Copy link
Contributor

Yeah, I also just took a second look over all the current PropTypes and none of them are so granular beyond the typing and shape of the given prop. All other more specific matching criteria are meant to be implemented as a custom prop type.

@rmilesson
Copy link

Why do you have a prop that is either a string or false? How are you using that? Sounds a bit like bad design.

@JustBlackBird
Copy link

false can stands for e.g. "no value"

@gaearon
Copy link
Collaborator

gaearon commented Mar 2, 2016

Currently, as far as I know there are no plans to add new features to PropTypes.
The longer term plan is to use Flow instead (and potentially add optional runtime validation to Flow).

See #3510 (comment).

@rmilesson
Copy link

@JustBlackBird That's what null is used for.

@JustBlackBird
Copy link

@rmilesson, then you can treat it as "string or null" condition =)

It actually does not matter. The point is to explicitly pass "no value" to component.

@rmilesson
Copy link

@JustBlackBird My point is that you should not use strings and booleans interchangeably. I mean, you can, absolutely, but it's pretty bad form.
Use null if you want to signify when something might get a value, but hasn't yet. It's clearer.

@JustBlackBird
Copy link

Ok, you've got you. Even if I want to use null or string I should use validator like:

PropTypes.oneOfType([
  PropTypes.string,
  PropTypes.oneOf([null])
]);

(So it does not matter null or false is used.)

And this proposal is about adding PropTypes.exactly validator.

@rmilesson
Copy link

No. Not at all.

PropTypes.string would suffice in that case. Unless you add isRequired (I suggest reading #2166).

Edit: A better approach would be to allow null values if a prop is required. At least in this case.

@JustBlackBird
Copy link

That is the point. isRequired + (string or null) allows to check if "no value" is passed explicitly and is not forgotten by accident.

@jimfb
Copy link
Contributor

jimfb commented Mar 2, 2016

@JustBlackBird If it was forgotten by accident, it would be === undefined. If it was passed explicitly but simply has no value, it would be === null. In the interest of API intuitiveness, It's probably not a good idea to use a non-standard sentinel value.

Proptypes are mostly legacy and in maintenance mode, having been replaced by tools like Flow. My guess is we aren't going to add this, so let's close it out. We can re-open if our thinking on this matter changes substantially.

It is worth noting that proptypes are just validator functions. You can define your own, and share/distribute your own proptypes. This is the optimal outcome, because people can download the additional proptypes if they find them useful.

@jimfb jimfb closed this as completed Mar 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants