Use hardcoded value for PropType secret

[aweary]

Addresses @spicyj's concern #7132 (comment)

cc @gaearon

[gaearon]

If I were third party lib author I wouldn’t feel guilty about passing a string like this.
I’m serious: let’s call it SECRET_DO_NOT_PASS_THIS_OR_YOU_WILL_BE_FIRED.

[sophiebits]

'I_PROMISE_I_WILL_ONLY_CALL_THESE_PROPTYPES_CHECKERS_IN_DEV_MODE'.

[sophiebits]

Whichever.

[maxdeviant]

There are already firing warnings elsewhere in the code, and I think it would make for a good running joke in the codebase 😄

[aweary]

If I were third party lib author I wouldn’t feel guilty about passing a string like this.
I’m serious: let’s call it SECRET_DO_NOT_PASS_THIS_OR_YOU_WILL_BE_FIRED.

I went with this one, for consistency 😉

[zpao]

→ semver-minor since it depends on another semver-minor (good edge case testing for my tool, but going to be hard to make work well when trying to do patch-only releases)

[ljharb]

Our tests fail on console warnings, so sadly this causes a semver-minor React update to break things :-/ Also, https://facebook.github.io/react/warnings/dont-call-proptypes.html 404s so I'm not even sure why it's a problem.

[gaearon]

Our tests fail on console warnings

This puts us in a very difficult position because this effectively means we can’t introduce new deprecations without doing a major release. Unfortunately this would slow down our development way too much, and for the benefit of the community we can’t do it this way. When we introduced our new versioning strategy with 15.0.0, we clearly said that we ship deprecation warnings in minor releases. This does not technically contradict semver.

The decision to fail tests on console warnings is yours. I can see why this is reasonable, but in this case you’ll want to use ~ instead of ^ since you intentionally want to be more conservative about the updates.

Also, https://facebook.github.io/react/warnings/dont-call-proptypes.html 404s so I'm not even sure why.

That’s bad, probably CI is stuck. I’ll look into why this happened, thanks for flagging.

[ljharb]

@gaearon Understood. However, our tests aren't run in production mode nor run with a production build of react, so I'm not sure why they'd be issuing warnings, since the point of the original issue was to deprecate calling them directly in production. We're still looking into it, and I'll update when I have more info.

[gaearon]

However, our tests aren't run in production mode nor run with a production build of react, so I'm not sure why they'd be issuing warnings, since the point of the original issue was to deprecate calling them directly in production.

Many people won’t notice warnings that happen only in production builds. Our intention is to deprecate the pattern of calling them directly per se because we want to treat them more as metadata. So we want the libraries that use PropTypes e.g. for API validation to use some fork (or a better system like tcomb) instead. Making them noops in production is a part of that, but the bigger issue is we want people to stop calling them manually at all.

[ljharb]

We use them to build custom propType validators on top of them - that are also stripped in production. What should we use instead if we want identical behavior as React's native PropTypes?

[gaearon]

We use them to build custom propType validators on top of them - that are also stripped in production.

This is still supported.

Please see: https://facebook.github.io/react/warnings/dont-call-proptypes.html#fixing-the-false-positive-in-third-party-proptypes

(The link is up now.)

[ljharb]

Thanks, that will likely resolve our problem :-)

[eugenebauling]

I know this is a long standing hard coded value, but is there a way we can change the value to exclude the word "Secret"? Our security scans are picking this up as a hard coded password/secret due to the value and naming. Unfortunately we cannot change the config of the security scans or flag this as a false positive as the config of the security scanning software is defined by the client.

[ljharb]

The client should change their config to not pick up false positives, or the software shouldn't be scanning third-party code for secrets ¯\_(ツ)_/¯