Regression in null key handling in 0.12 (possibly intentional)

[sophiebits]

In 0.11, the code

<div>
  <div key={null}>foo</div>
  <div key={null}>bar</div>
</div>

assigned implicit keys to both children. In 0.12, it gives the error

Warning: flattenChildren(...): Encountered two children with the same key, .$null. Child keys must be unique; when two children share a key, only the first child will be used.

and renders only "foo", because the key is cast to a string in ReactElement

react/src/core/ReactElement.js

Line 140 in be468c2

key = config.key === undefined ? null : '' + config.key;

before it gets checked against null in traverseAllChildren:

react/src/utils/traverseAllChildren.js

Line 50 in be468c2

if (component && component.key != null) {

I guess it's okay if this is intentional but it broke a part of KA in the upgrade.

@sebmarkbage? (cc @jdan)

[sebmarkbage]

hm. Yea, this was intentional to ensure more stable types through out the system.

I don't think null should be treated as a valid "non-value" to align it with defaultProps, default arguments and default values in destructuring. The intention of "null" is to symbolize a missing object. However, this can be of any type.

It's weird that undefined is coerced into null. It should be left as undefined.

For refs, we expect them to become objects so it is valid to have null there to represent a missing value.

We have a two options:

1. Allow any value to be coerced to strings.
2. Throw on non-string values.
3. Don't coerce to string and start using maps as keys. E.g. Dates would become by reference instead of by value. null would be treated as a key and would still be duplicate.

[sophiebits]

Maybe we can support null meaning no key for a 0.12 with a warning, then drop support in 0.13? Just trying to avoid breaking things (that are impossible to grep for) without a warning. (I have no idea if we have other places in our code that do this by accident…)

[sebmarkbage]

They don't in prod though, right? I thought we had a fallback for duplicate values that would be essentially the same?

[sophiebits]

No, that code previously outputted foo bar and outputs just foo with 0.12. We ignore all children after the first with a given key.

[sebmarkbage]

oh. fine, warning for one version and then swap behavior. Want to send a PR?

[sophiebits]

Will do.