facebook / prop-types

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

False positive isRequired validation for never rendered components #409

Closed extempl closed 6 months ago

extempl commented 6 months ago

With conditional wrapper like this:

import "./styles.css";
import PropTypes from "prop-types";

const RenderWrapper = ({ condition, children }) => {
  return <>{condition ? children : null}</>;
};

const NeverRendered = ({ requiredProp }) => {
  console.log("render", requiredProp.foo);
};

NeverRendered.propTypes = {
  requiredProp: PropTypes.object.isRequired,
};

export default function App() {
  return (
    <div className="App">
      <h1>Hello CodeSandbox</h1>
      <h2>Start editing to see some magic happen!</h2>
      <RenderWrapper condition={false}>
        <NeverRendered requiredProp={null} />
      </RenderWrapper>
    </div>
  );
}

It spams with prop data is marked as required, but its value is null warnings, while in fact, even that the component returned from render method, its render never called.

If you wandering why would anyone use it like so - we use for loading state wrapper, where required prop is provided later and when it is - nested component is rendered.

Live Demo: https://codesandbox.io/p/sandbox/prop-types-49c3tn

ljharb commented 6 months ago

React doesn't know, at the time that App is rendered, whether RenderWrapper is going to render its children or not, so it has to always, unconditionally, evaluate its propTypes. The current behavior is correct - if the prop is required, it needs to always be provided. A better approach would be an inline jsx ternary instead of a wrapper that does the conditional rendering far away from where the element is created.

Either way, this is a question for React itself, and not the prop-types repo.