facebook / prop-types

Runtime type checking for React props and similar objects
MIT License
4.48k stars 356 forks source link

Why is this package recommended to be a dependency instead of devDependency? #312

Closed TheWirv closed 4 years ago

TheWirv commented 4 years ago

As the title says, is there any particular reason not to make prop-types a devDependency?

ljharb commented 4 years ago

Because it runs in your production code, even if React compiles out its propTypes checking in production. If you don't include prop-types in your app, PropTypes.oneOf() will throw an error, for example.

Dev deps are for things needed to develop a package, like babel/eslint/etc; regular deps are for things needed in/by your code, which includes propTypes.

TheWirv commented 4 years ago

Because it runs in your production code, even if React compiles out its propTypes checking in production. If you don't include prop-types in your app, PropTypes.oneOf() will throw an error, for example.

Dev deps are for things needed to develop a package, like babel/eslint/etc; regular deps are for things needed in/by your code, which includes propTypes.

Okay, thanks for the answer. The second part of that was already clear to me, but after having read the following, "... will check props passed to your components against those definitions, and warn in development if they don’t match," I didn't really expect it to run in production code.

ljharb commented 4 years ago

Fair enough :-) it's confusing because React doesn't warn on propTypes in production, but the propTypes themselves are still used.

TheWirv commented 4 years ago

Fair enough :-) it's confusing because React doesn't warn on propTypes in production, but the propTypes themselves are still used.

That makes sense :)

joetidee commented 3 years ago

Is three a way to keep prop-types enabled in production? I want to observe and report on prop-typ warnings against production data.

ljharb commented 3 years ago

I think you’d have to use the development build of react, with NODE_ENV not set to production.

joetidee commented 3 years ago

Would be great to make it a feature of the package, defaulting to production.

ljharb commented 3 years ago

That’s not within the power of this package - it’s defined within react itself.

joetidee commented 3 years ago

So, if the checking of the production value here was made optional, this still wouldn't expose prop-type warnings in production? https://github.com/facebook/prop-types/blob/b67bbd40b5bf8b27475801ecdb7617071ceaccac/index.js

ljharb commented 3 years ago

Yes, because React itself calls propTypes on components.

joetidee commented 3 years ago

Are you saying that react will not call propTypes in production?

Some.Component.propTypes = {
  foo: Proptypes.string.isRequired,
};
ljharb commented 3 years ago

Yes, that is precisely what I am saying.