facebook / prop-types

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

v15.8.0 breaks `PropTypes.shape({...})` for objects and class-instances. #368

Closed Kegulf closed 2 years ago

Kegulf commented 2 years ago

propChecking related to PropTypes.shape being used with objects and class-instances seems to be broken in v15.8.0. It works in v15.7.2.

I think that it's this PR that broke it.

The result is my tests failing with the following warning:

Warning: Failed prop type: ReceiptSuccess: prop type `receiptLinks[0].title` is invalid; it must be a function, usually from the `prop-types` package, but received `undefined`.

My specific case:

receiptLinks: PropTypes.arrayOf(
  PropTypes.shape({
    title: PropTypes.string.isrequired,
    href: PropTypes.string.isrequired,
    internal: PropTypes.bool,
  })
).isRequired,
const receiptLinks = [
  new Link('link1', 'http://link1.no', true),
  new Link('link2', 'http://link2.no', true),
];

It also fails when I try to take in a shape instead of arrayOf(shape):

receiptLinks: PropTypes.shape({
  title: PropTypes.string.isrequired,
  href: PropTypes.string.isrequired,
  internal: PropTypes.bool,
}).isRequired,
const receiptLinks = new Link('link1', 'http://link1.no', true),

It fails with the same warning if I change receiptLinks to JS-objects as well:

receiptLinks: PropTypes.arrayOf(
  PropTypes.shape({
    title: PropTypes.string.isrequired,
    href: PropTypes.string.isrequired,
    internal: PropTypes.bool,
  })
).isRequired,
const receiptLinks = [
  { title: 'link1', href: 'http://link1.no', internal: true },
  { title: 'link2', href: 'http://link2.no', internal: true },
];
Kegulf commented 2 years ago

maybe @ljharb has any clue what happens here? 😅

ljharb commented 2 years ago

I have no idea how that PR could have broken it, but I’ll definitely look into it today.

ljharb commented 2 years ago

Can you share a bit more about your setup? There's extensive tests that PR adds to that already prove this works - and if this didn't work, I'd expect it to take much less than 6 days for the first bug report.

ljharb commented 2 years ago

In particular, a codesandbox that reproduces it would be most helpful.

serut commented 2 years ago

This error also occurs on our project RegardsOSS.
We downgraded to 15.7.2 for now, no time to investigate this issue yet

ljharb commented 2 years ago

I will be happy to fix it once it’s reproducible.

madflow commented 2 years ago

This happened in our codebase aswell. The warning was caused by using

  options: PropTypes.shape({
    readOnly: PropTypes.boolean,
  }),

changing it to

  options: PropTypes.shape({
    readOnly: PropTypes.bool,
  }),

helped.

In the above examples there is

    title: PropTypes.string.isrequired,

Maybe try changing it to (note the camel case)

    title: PropTypes.string.isRequired,

will help, too.

serut commented 2 years ago

Thanks, my build is passing now. I had some invalid proptypes that are reported with this updates.
I just had to fix them

ljharb commented 2 years ago

That’s great! So it sounds like v15.8 is helping you find invalid PropTypes, and isn’t actually breaking anything that should be working?

serut commented 2 years ago

Yes, any key associated with an undefined value now warns

I had function instead of func, boolean instead of bool, PropTypes.date instead of PropTypes.string, isRquired instead of isRequired.... This is a good release, thanks !

ljharb commented 2 years ago

@Kegulf given this, I’m going to close this issue, but will be happy to reopen it if there’s a repro case for something failing that should work.