React.cloneElement handles undefined props differently to React.createElement

[richardscarrott]

I'm not sure if this is expected but it caught me out earlier:

// Without clone
function MyComponent() {}

MyComponent.defaultProps = {
    foo: 'foo'
};

let element = React.createElement(MyComponent, {
    foo: undefined
});

console.log(element.props.foo); // 'foo'

// With clone
function MyComponent() {}

MyComponent.defaultProps = {
    foo: 'foo'
};

let element = React.createElement(MyComponent);
element = React.cloneElement(element, {
    foo: undefined
});

console.log(element.props.foo); // undefined

[acdlite]

Ooh, interesting! AFAICT defaultProps is only respected at element creation: https://github.com/facebook/react/blob/master/src/isomorphic/classic/element/ReactElement.js#L153-L160

Whether or not this is intended or cloneElement should be aware of defaultProps, too, that's a good question. Curious to know the answer.

[richardscarrott]

I was thinking cloneElement probably shouldn't be aware of defaultProps as such, it could just make that same check as createElement for typeof 'undefined' to determine whether the new prop gets copied over. That way undefined and null would have the same semantics as createElement.

[jimfb]

I think this is a bug.

createElement(Foo, {}) should pretty clearly be equal to cloneElement(createElement(Foo, {bar: 'noise'}), {bar: undefined})

[truongduy134]

I made a PR for this :) . #5997

[mattzeunert]

Why would it restore the default value? It's not the behavior I would expect intuitively.

[jimfb]

@mattzeunert Because otherwise you could create an element using React.cloneElement that could not have been created using React.createElement.

[zpao]

cc @sebmarkbage - I don't recall if we talked about this before but what should our expected behavior here be? defaultProps resolution was explicitly only resolved during createElement but now we're adding a second pass for cloning.

[sebmarkbage]

Yea, I think this is a bug. For the reason @jimfb mentioned above. defaultProps gives you a way to guarantee that the type of a prop is never going to be undefined.

The idea is that this should guarantee that you never get undefined:

type Props = { foo: number };
function Foo(props : Props) {
}
Foo.defaultProps = {
  foo: 123
};

Another example to explain this semantic is that cloneElement should be roughly equivalent to:

<element.type {...element.props} key={element.key} ref={element.ref} />

[arjunsharma]

Not sure if @truongduy134 has a PR that's going to get merged. I reused the same defaultProp process that is used in createElement in cloneElement to fix this bug. #6037

[mnpenner]

Uhh.. so what happens now if I use React.cloneElement and explicitly set a property to undefined?

Will the property:

1. Be set to undefined
2. Be reset to the defaultProp
3. Be left untouched (keep value from cloned element)

[arjunsharma]

It does the same thing as React.createElement, which is to set the property to the value in defaultProps. So item # 2 in your list.

If you want to look at the PR that fixed this, it was fixed in #5997.