facebook / prop-types

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

Required by default? #241

Closed jamierumbelow closed 5 years ago

jamierumbelow commented 5 years ago

Thanks prop-types team for all your work so far.

Question: when's the next major prop-types release scheduled for, and has there been a discussion about enabling isRequired by default?

My basic assumption is that the majority of components require the majority of their props implicitly, even if developers don't label them as such.

I try to mark props as required by default:

Profile.propTypes = {
  id: PropTypes.number.isRequired,
  email: PropTypes.string.isRequired,
  openModal: PropTypes.func.isRequired,
  renderAdminActions: PropTypes.func
}

It's safer to be stricter rather than let avoidable failures happen further down the stack, or further away from their source – which is the basic rational for adding .isRequired, and for using prop types more generally, for that matter.

Repeating .isRequired, like above, is cumbersome and causes visual clutter. Instead, we could expose an .optional modifier to disable it:

Profile.propTypes = {
  id: PropTypes.number,
  email: PropTypes.string,
  openModal: PropTypes.func,
  renderAdminActions: PropTypes.func.optional
}

'Required by default' is a common pattern in software: many languages such as TypeScript and Swift require declared function parameters by default; NOT NULL must be set explicitly in most relational DB schemas; . Such conventions shape developer expectations (particularly amongst developers who are newer to the React ecosystem).

It may also have positive knock-on effects:

ljharb commented 5 years ago

Although I think that would have been a better initial default, I don't think the value in that breaking change is high enough to warrant the developer churn it would cause.