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

A way to work around deprecation of directly calling propTypes validators? #7623

Closed
jwietelmann opened this issue Aug 31, 2016 · 15 comments
Closed

Comments

@jwietelmann
Copy link

jwietelmann commented Aug 31, 2016

I came up with a rather crude (but I think promising) way to autogen component rendering tests. I'm throwing a big list of values at my components' propTypes validators (kind of like using a fuzzer). I use that to construct a set of supposedly-valid values for each prop, and subsequently combine those into supposedly-valid sets of props.

With direct calling of prop validators being deprecated, is there some other way to accomplish this that I'm missing? I think this test generator would be really useful, but given that I'm dependent on a deprecated feature, I'm not clear if it has a future.

fiddle demo: https://jsfiddle.net/6g3r6amb/1/

@gaearon
Copy link
Collaborator

gaearon commented Aug 31, 2016

The warning is a measure meant to educate people to not rely on them existing in production code (and thus not using them for product features, e.g. for creating schemas that are executed in production).

I think that we can remove the warning in React 16 completely. We’ll make PropTypes throw hard in production, but work in development. Then this kind of functionality should still work, without risk of somebody relying on this in prod (because it would throw right away in 16 in prod).

@gaearon
Copy link
Collaborator

gaearon commented Aug 31, 2016

However, looking into the future, we are likely to start deemphasizing PropTypes eventually, and recommend that people use Flow or TypeScript instead. So I would rather recommend looking at that side of the tool spectrum.

@brigand
Copy link
Contributor

brigand commented Sep 1, 2016

@jwietelmann It'd probably be less crude if you went with flowtype and used babel to parse it. Then you could clearly identify things like enums in the prop types, or generate metadata in the format you want. I did something similar in this repo. You could modify makePropTypeAst.js to generate metadata for use in your test generator. Also, it just looks nicer in the source code 😄

@busticated
Copy link

i have the same basic problem as @jwietelmann - i have a bunch of custom proptype validators with tests where i end up calling the validator directly.

my code isn't public but is very similar to this --> https://github.com/HurricaneJames/react-immutable-proptypes/blob/master/src/__tests__/ImmutablePropTypes-test.js

any suggestions short of moving to flow / typescript?

@busticated
Copy link

heh. just realized the warnings are output via console.error

@gaearon
Copy link
Collaborator

gaearon commented Sep 7, 2016

@busticated Yep, for now you can just call React.createElement and test the behavior by spying on console.error. You'll need to make sure displayName of tested component is random in every test so that duplicate warnings don't get suppressed. We will remove this warning in next major release so it's not forever either.

@busticated
Copy link

@gaearon heh. yeah that's probably a better approach than just supplying the secret and getting fired. which some people say works just fine ;-)

@gaearon
Copy link
Collaborator

gaearon commented Sep 7, 2016

Haha. For testing this works, just make sure to remove this with React 16 that won't need it.

@jedwards1211
Copy link
Contributor

jedwards1211 commented Sep 18, 2016

@gaearon

We’ll make PropTypes throw hard in production, but work in development.

That would be a great solution for #7765

@gaearon
Copy link
Collaborator

gaearon commented Sep 18, 2016

@jedwards1211 It’s not really a solution, throwing hard in production is the only reason why this warning was added in the first place. This warning exists to educate users about a breaking change in React 16. When we land that breaking change, we‘ll remove the warning because it won’t be necessary.

@jedwards1211
Copy link
Contributor

I understand, what I meant is it would work with react-dom-stream because it should only check propTypes in development anyway.

@jedwards1211
Copy link
Contributor

@gaearon as an aside, a lot of my initial worries stemmed from the fact that the error message said "this may not work in the next version". If future changes will make something stop working in production without affecting development, it would be good to make error messages say "this may not work in production with the next version"

@gaearon
Copy link
Collaborator

gaearon commented Sep 20, 2016

Fair enough. I wanted the message to seem strict so that we learn faster about use cases. I think we can relax it now and I'll take a PR that adds "in production" to the warning. Would you like to make it?

@jedwards1211
Copy link
Contributor

Sure, here it is: #7777

@gaearon
Copy link
Collaborator

gaearon commented Oct 27, 2016

#7777 has since been merged, so closing. Thanks!

@gaearon gaearon closed this as completed Oct 27, 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

5 participants