facebook / prop-types

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

Failed prop type: Converting circular structure to JSON when stringifying React jsx #383

Open gurkohli opened 2 years ago

gurkohli commented 2 years ago

What is the issue?

In cases when react creates jsx that references itself and that jsx is passed as a prop, an exception is thrown when that prop fails validation. The exception is because type checkers are trying to perform JSON.stringify on the prop value but it contains the circular structured jsx

React's circular structure is expected as mentioned by Dan here: https://github.com/facebook/react/issues/24360 and it is up to prop-types to handle it.

Complete error:

Warning: Failed prop type: Converting circular structure to JSON
    --> starting at object with constructor 'HTMLDivElement'
    |     property '__reactFiber$dhuo6horm3g' -> object with constructor 'FiberNode'
    --- property 'stateNode' closes the circle

What is expected?

The validators when JSON.stringifying should ignore the react fields as they are not helpful to be included in the error message anyways.

prop-types circular structure error
ljharb commented 2 years ago

React's structure sure, but why would jsx element object be pointing to DOM elements? Are you passing a native HTML element as a prop value?

gurkohli commented 2 years ago

In this case yes, but the error shows up even if a rendered react component was passed as prop. My exact use case is such, we have a wrapper over ant-design's Select component [https://ant.design/components/select/]. We pass along the options as a either a list of flat options or a list of grouped options. See the prop-types below

const selectOption = PropTypes.exact<WeakValidationMap<SelectOption<unknown>>>({
  description: PropTypes.string,
  disabled: PropTypes.bool,
  icon: PropTypes.shape<WeakValidationMap<IconProps>>({
    name: PropTypes.string as Requireable<IconName>,
    tooltipText: PropTypes.node,
    weight: IconPropTypes.weight,
  }),
  label: PropTypes.node.isRequired,
  labelStr: PropTypes.string,
  value: PropTypes.any.isRequired,
});

export const flatSelectOptions = PropTypes.arrayOf(selectOption.isRequired);

export const groupedSelectOptions = PropTypes.arrayOf(
  PropTypes.exact<WeakValidationMap<GroupedSelectOption<unknown>>>({
    groupLabel: PropTypes.node.isRequired,
    options: PropTypes.arrayOf(selectOption.isRequired).isRequired,
  }).isRequired,
);

...

options: PropTypes.oneOfType([
   SelectPropTypes.flatSelectOptions,
   SelectPropTypes.groupedSelectOptions,
]).isRequired,

These props are then processed into AntSelect.Option and AntSelect.OptGroup and passed along. It is expected that the label property can contain a jsx value, rendering a native HTML element or a React component. In the case above the jsx has a stateNode property that's referencing itself.

prop-types circular structure jsx screenshot
ljharb commented 2 years ago

I'm not sure what you mean by a "rendered react component" - like the JSX element a component returns?

gurkohli commented 2 years ago

That’s right.

ljharb commented 2 years ago

It seems like React could address that for JSX elements by providing a toJSON method on them, rather than individual parts of the ecosystem having to account for it. The web could do the same for DOM elements if needed, but it seems like it's really a problem for jsx elements.

gurkohli commented 2 years ago

That's a valid solution but I also think that, given how React and prop-types are commonly used in combination, and that the JSON.stringify here is for the purposes of error message composition, prop-types should either outright ignore the react fields, or allow the calling components to provide a replacer function for JSON.stringify, or a custom stringify function so the components can handle the stringification in these cases.

These fields are internal to React and are not useful to include in the error message anyways for the end user.

Also see https://github.com/facebook/react/issues/24360, it seems like React does not guarantee its internal metadata to have stringifiable JSON