React: React.cloneElement does not avoid: "Warning: Failed propType: Required prop"

Created on 29 Apr 2016  路  10Comments  路  Source: facebook/react

Ok so here is what happens, I have some wrapper component that wraps a children:

<WrapperComponent oneThing={oneThing}>
  <MyComponent />
</WrapperComponent>

Then, the WrapperComponent clone any children I pass with its own props and some new ones:

React.cloneElement(this.props.children, Object.assign({}, {onSubmit: this.onSubmit}, this.props));

The thing is that <MyComponent /> has `somepropTypes that are required. The warning is shown even when the props are there:

Warning: Failed propType: Required prop `oneThing` was not specified in `MyComponent`.

Is this expected? I am using React 0.14.7.

Most helpful comment

@gaearon I think throwing this warning is not appropriate, since it really amounts to a false alert, and setting the default prop would mask a real problem.

It seems like cloneElement here is a downstream solution to an upstream problem: The original element/component that gets created is just garbage that will immediately get thrown out, so why waste the resources creating it in the first place? It will _never_ be used.

All 10 comments

Yes, this is expected because elements are checked at the creation time. So when you write <MyComponent />, it turns into a React.createElement() call, and the props are validated.

When you rely on cloneElement() to specify required props, our recommendation is to use defaultProps in your component for sensible fallbacks, e.g.

MyComponent.defaultProps = {
  onSubmit: () => {}
}

why not just write a fake propTypes validation function and run it first up inside the component?

const myComponent = (props) => {
  const hasValidProps = wrappingPropTypesValidation(props); 
  if (!hasValidProps) return null;

  return <div></div>
};

@gaearon I'm running into this problem too.

I don't have access to the child components to make their props optional or give them defaults. Its whole point is that it accepts any children that work through value and onChange.

(It's a <Form> component that dynamically puts value and onChange props on its children so it can provide access to a composition of its children's values through its own props.)

I'm not sure how I can help you.

I'm just adding my voice to the issue. At some point, it would be nice if React could support this use-case.

@gaearon I think throwing this warning is not appropriate, since it really amounts to a false alert, and setting the default prop would mask a real problem.

It seems like cloneElement here is a downstream solution to an upstream problem: The original element/component that gets created is just garbage that will immediately get thrown out, so why waste the resources creating it in the first place? It will _never_ be used.

I am also confused about it. I understand that a component fully knows about its properties when its parent renders it. And at that time props should be validate.

Here is the example: https://codesandbox.io/s/1qv24zwzm3. I inject selected property to every child component. That is why child component has selected property as required.

What do you think? @gaearon does the validation has to be at creation time? Can you explain why it is there?

I have same problem with HoC, I use cloneElement over createElement for performance reasons. I would prefer avoid setting meaningless defaultProps because in that case I would not notice the missing required prop.

Details:
https://stackoverflow.com/questions/56832586/react-proptypes-isrequired-warning-with-cloneelement

A way to get around the Failed prop type warnings and not setting defaultProps is to pass the children prop explicitly to your HOC. For example, instead of:

<HOC>
    <Child />
</HOC>

Do:

<HOC children={Child} />

I'm not sure if there are any other ramifications to this, but it avoids unnecessarily instantiating a dummy Child component.

Another Solution - Currying

You can wrap your component with a delegator component.

The obvious caveat is that you add another anonymous component to the component tree.

Change this:

Line.propTypes = {
  id: PropTypes.string.isRequired,
};

export default Line;

To this:

Line.propTypes = {
  id: PropTypes.string.isRequired,
};

export default props => <Line {...props} />;

Or the more generic:

const withCurry = Component => props => <Component {...props} />

export default withCurry(Line);

On Other Solutions

  • defaultProps

    • Soon to be deprecated.

    • Sometimes a property is truly required. What default would you give to a connection source and destination?

  • Internal validation

    • An exception to the standard approach (and blending two approaches is never good).

    • propTypes is a form of API documentation if you wish - some doc generators rely on them.

    • propTypes is a declarative solution to a cross-cutting concern, whereas internal validation is bound to be imperative and local.

Actual Use-Case

Instead of:

    renderConnection={props => (
      <Line {...props} strokeWidth={2} />
    )}

Do this:

    connection={<Line strokeWidth={2} />}

Which seem only slightly less verbose and less prone to errors.

But the src and dst properties of Line are objects, which one may want to define partly in the incoming props, partially in the element properties. So these need to be merged, which the second form would do automatically, whereas with the first form users will have to do it manually.

Was this page helpful?
0 / 5 - 0 ratings