c-hive / guides

Guides for code style, way of working and other development concerns
MIT License
26 stars 5 forks source link

Consider using PropTypes #33

Closed gomorizsolt closed 4 years ago

gomorizsolt commented 4 years ago

https://github.com/facebook/prop-types

gomorizsolt commented 4 years ago

Stumbled upon the fact that the package does not warn in case of missing props/misaligned props unless the project runs in the browser. Found this "counter-intuitive". I think it'd make sense to receive errors/warnings without running the app and constantly checking the logs. Perhaps it's only possible with static type-checking, see https://reactjs.org/docs/static-type-checking.html.

Apart from this, incorporated prop-types into https://github.com/gomorizsolt/react-boilerplate to be able to play around with this a bit.

thisismydesign commented 4 years ago

AFAIK it only runs in development mode, wasn't aware of the browser limitation, do you have a source for this?

gomorizsolt commented 4 years ago

AFAIK it only runs in development mode, wasn't aware of the browser limitation,

Perhaps I wasn't so precise. It's not limited to browsers. My expectation was that prop-types would throw error messages in case of missing/misaligned props while editing the code but it warns in the browser's console instead. That's why it's required to have a running instance of the app in the background.

const Foo = ({ bar }) => <p>{bar}</p>;

Foo.propTypes = {
   bar: PropTypes.string.isRequired
};

// The IDE does not warn if the bar prop is omitted. It throws the warning in the browser's console.
// Warning: Failed prop type: The prop `name` is marked as required in `Foo`, but its value is `undefined`.
<Foo />

AFAIK it only runs in development mode

I'm not sure. Interestingly though, the owners do not suggest installing prop-types as a dev-dependency => npm install --save prop-types. I guess therefore it's definitely possible to receive the errors/warnings in production mode.

gomorizsolt commented 4 years ago

We can make sure whether it's possible to build the app if there's an error regarding poorly-written prop-types.

Does it make sense?

thisismydesign commented 4 years ago

prop-types only provides warning in dev mode on the console (whichever console that may be). This is by design:

When an invalid value is provided for a prop, a warning will be shown in the JavaScript console. For performance reasons, propTypes is only checked in development mode.

https://reactjs.org/docs/typechecking-with-proptypes.html

My expectation was that prop-types would throw error messages in case of missing/misaligned props while editing the code

You'd need compilation and static types to do that.

thisismydesign commented 4 years ago

The best you can do in enable an eslint rule for missing prop type definitions, you can only check prop types during execution.

gomorizsolt commented 4 years ago

The best you can do in enable an eslint rule for missing prop type definitions, you can only check prop types during execution.

Yea, that rule was disabled. Thanks for the help and clarifications.

thisismydesign commented 4 years ago

I'd close this, I think propTypes make sense unless there's a better alternative. By default our ESLint config will warn to use it.

I found this comparison of alternatives interesting: https://hackernoon.com/why-i-no-longer-use-typescript-with-react-and-why-you-shouldnt-either-e744d27452b4

Next I'd take a look at Flow and how well it's supported (is there good tooling, and whether it interferes with existing tooling).