React: React.memo and React.lazy ignore propTypes

Created on 9 Nov 2018  路  10Comments  路  Source: facebook/react

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
PropTypes.*.isRequired with React.memo doesn't get triggered in the console when the required prop is undefined.

import React, { memo } from "react";
import ReactDOM from "react-dom";
import PropTypes from "prop-types";

// ********************************************
// It should appear two propTypes console errors
// ********************************************

// without memo(..): it will throw the propTypes error like it should
const MemoButton = memo(() => {
  return <button>Memo Button</button>;
});
MemoButton.propTypes = {
  memocolor: PropTypes.string.isRequired
};

// it will throw the error in the console
const Button = () => {
  return <button>Button</button>;
};
Button.propTypes = {
  color: PropTypes.string.isRequired
};

const rootElement = document.getElementById("root");
ReactDOM.render(
  <>
    <MemoButton />
    <Button />
  </>,
  rootElement
);

Edit 501jppv64n

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

// affected versions
"prop-types": "15.6.2",
"react": "16.6.1",
"react-dom": "16.6.1",
Bug Needs Investigation

Most helpful comment

This turned out to be harder than expected 馃槄
Fix in https://github.com/facebook/react/pull/14298.

All 10 comments

It should work if the propTypes are attached to the component that you give as an argument to React.memo.


Nevermind, I was thinking of defaultProps. propTypes should be checked by createElement without caring whether the component is memoized 馃

I came across this issue in the 16.7 alpha as well. I've updated the above codeSandbox to show that adding propTypes before or after memoizing the component has no effect.

"prop-types": "^15.6.2",
"react": "^16.7.0-alpha.0",
"react-dom": "^16.7.0-alpha.0",

Edit zky2km9703

We need to decide on what we actually want to support. Arguably with memo it is valid to decorate either side (depending on how you define it). With lazy decorating outer side seems shady and we should probably warn if you attempt to do so.

You'd want adding or removing memo to be transparent - ie, not a breaking change - so I'd expect propTypes, defaultProps, etc to all work the same on it.

We have just encountered something similar but from a different angle. The react-router package has prop types check for Route.component prop and it's using react-is package to do this. Surprisingly enough, the React.memo returns an object wrapping a function. So I guess that package needs to be updated to reflect on this.

An originating issue for a reference: https://github.com/mobxjs/mobx-react-lite/issues/12

Edit: I can work on PR, should be easy enough, I'll just wait for the green light :)

The react-router package has prop types check for Route.component prop and it's using react-is package to do this. Surprisingly enough, the React.memo returns an object wrapping a function. So I guess that package needs to be updated to reflect on this.

Can you please be more specific as to how it checks that? There's a bunch of different methods exported from react-is. Which one is it using, and how?

The update to use react-is is not yet @latest; the @latest release still uses PropTypes.func.

This is the current check on master: https://github.com/ReactTraining/react-router/blob/d5979813abcd53f7ab0e518248374c61c22f8e15/packages/react-router/modules/Route.js#L131-L137

Any update with this? I'm using React.lazy in a new project and noticed PropType validation errors does not show on console when the component loaded this way.

I tried loading with regular dynamic import(), and static import and then wrap in Suspense. None of these methods break prop types.

My versions are:

"prop-types": "^15.6.2", "react": "^16.6.3", "react-dom": "^16.6.3"

If there was an update on this, it would be literally on this issue. Such as a comment, or it getting closed.

You can see I have work in progress PR in https://github.com/facebook/react/pull/14219. It needs some changes. After I address them and merge it, we'll cut a patch with the fixes.

This turned out to be harder than expected 馃槄
Fix in https://github.com/facebook/react/pull/14298.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

jvorcak picture jvorcak  路  3Comments

zpao picture zpao  路  3Comments

zpao picture zpao  路  3Comments

kocokolo picture kocokolo  路  3Comments

trusktr picture trusktr  路  3Comments