react-test-renderer does not include key on fragments

[SimenB]

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
From the docs (https://reactjs.org/docs/fragments.html#keyed-fragments):

function Glossary(props) {
  return (
    <dl>
      {props.items.map(item => (
        // Without the `key`, React will fire a key warning
        <React.Fragment key={item.id}>
          <dt>{item.term}</dt>
          <dd>{item.description}</dd>
        </React.Fragment>
      ))}
    </dl>
  );
}

If I do this:

renderer
  .create(
    <Glossary
      items={[{ id: 'id', term: 'term', description: 'description' }]}
    />
  )
  .toJSON();

The result is:

{
  "type": "dl",
  "props": {},
  "children": [
    { "type": "dt", "props": {}, "children": ["term"] },
    { "type": "dd", "props": {}, "children": ["description"] }
  ]
}

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://jsfiddle.net or similar (template for React 16: https://jsfiddle.net/Luktwrdm/, template for React 15: https://jsfiddle.net/hmbg7e9w/).
See above

What is the expected behavior?
That, somehow, the key is not lost. I'm not sure where it would make sense, but it should be in there somewhere.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
16.2.0

[gaearon]

Want to send a PR with a fix?

[SimenB]

Sure, if you've got a pointer as to how it should look. Where would the key be?

[gaearon]

Maybe ReactTestRenderer.js. My gut feeling is there’s a switch there, and the Fragment case doesn’t specify keys.

[SimenB]

I meant in the output of toJSON. Currently the fragment is not there at all, children is just an array of the fragment's children.

EDIT: the children prop here is a fragment, but it seems very much on purpose that it's not rendered.

I'm not sure if it makes sense to include it, except that it looks odd in Jest snapshots that it's gone. But maybe not? I don't write React at all (sadly), I just found it weird that key was gone when I tested out fragments with Jest snapshots

[Jessidhia]

React.Fragment is completely transparent to the render tree; while the reconciler will use it, the resulting tree essentially has no fragments at all (they're all flattened, like arrays).

Maybe it could be helpful to have them stay in tree for the test renderer result, but that would probably require some "fun" changes inside fiber 🤔

[gaearon]

Do we include keys for the regular nodes? My intuition is it seems unnecessary. Keys are a hint to the reconciler about how the content changes over time. However snapshots are static.