React: Clarifying no key warning on Fragments

[ialexryan]

(Apologies if this has been discussed before, I searched and didn't find anything.)

Currently if you create an array of elements without keys you get a key warning message, and if you create a fragment of elements without keys you do not. Until today I assumed that meant "ah, React 16.2 introduced some clever new tech that obviated the need for keys". But I learned today that that is not the case. Right now, I believe you can make a dynamic list of elements, throw it in a fragment, and get no key warning message – but have bad performance and non-stable updates.

As best as I can tell, the reason for the difference in behavior is that fragments are _expected_ to be understood as having a different semantic meaning than arrays, to be used for static content only. But if we're relying on that expectation to avoid a major gotcha, I would assume it would be explicitly mentioned in the docs for fragments, and prominently featured in a warning-style yellow box. Instead, I don't believe it's mentioned at all.

The dual behaviors make a lot of sense to me (if React can't figure out on its own whether siblings need keys, it's great for us to have a way to explicitly tell it when they do or don't) but it seems to me that such an important and subtle nuance would call for a lot of documentation and instead has none that I can find.

Please let me know if I've misunderstood any of this – I figure that's more likely than not 😅

[aweary]

But I learned today that that is not the case. Right now, I believe you can make a dynamic list of elements, throw it in a fragment, and get no key warning message – but have bad performance and non-stable updates.

Can you provide an example of this? Rendering an array of elements in a Fragment should still warn if they don't have keys. Here's an example of this: https://jsfiddle.net/vg1u9p3v/

Maybe I'm misunderstanding what you're saying.

[nmain]

This is what's being referred to, I believe: https://jsfiddle.net/n88hd7ay/

The two divs have identical content, but only one fires a react warning.

[ialexryan]

Thanks for the jsfiddle @aweary; I'm shocked to see a Fragment giving key warnings, because I've never seen that behavior before. Is that new since 16.2.0? (That's the version that we're on.) I just revisited my code and was able to reproduce the behavior where I get a key warning with an array and no key warning with a fragment.

@nmain thanks also for the jsfiddle – I think that is exactly my problem.

I'm also clearly not the only person confused here:

Post 1: "Using the Fragment component, we can now accomplish the same without the array syntax _and without using keys._" (emphasis mine)

Post 2:

Post 3: "return[ing] an array of elements has problems...you have to add a key property to each element to avoid React warnings. Fragments solve this problem."

And @aweary my understanding is partly based on your and Dan's excellent comments on this issue https://github.com/facebook/react/issues/8920#issuecomment-277113897 which references the idea of a "separate syntax" that "could let React distinguish between the case where keys are necessary (dynamic lists), and the static lists."

[ialexryan]

For maximum clarity, this is the code I'm comparing:
This does not give a key warning:

r.div(
    { className: "myClass" },
    r.fragment(
        this.props.assets.map((asset, index) => {
            return this._renderThumbnail(asset, index);
        })
    )
);

This does give a key warning:

r.div(
    { className: "myClass" },
    this.props.assets.map((asset, index) => {
        return this._renderThumbnail(asset, index);
    })
);

(given _renderThumbnail returns a div without a key attribute)

[aweary]

The reason that @nmain's example doesn't warn is because the React.createElement call isn't passing an array of children, it's using the argument spread syntax to pass each item as an additional argument.

return React.createElement(React.Fragment, null, ...args);

Since it's spreading args this would be equivalent to:

<React.Fragment>
  <p>1</p>
  <p>2</p>
   {/* so on... */}
</React.Fragment>

Referring back to my comments in #8920 you can see why this wouldn't warn. This is a static set of children from React's perspective.

If you were to just pass args in directly, like so:

return React.createElement(React.Fragment, null, args);

That would be the equivalent of

<React.Fragment>
  {[
  <p>1</p>,
  <p>2</p>,
   {/* so on... */}
   ]}
</React.Fragment>

Which, from React's perspective, is potentially a dynamic array. In this case, you would receive the key warning. If you're not using JSX this can be a hard distinction to understand and track throughout your codebase. If you're using helper functions you should check to see how the React.createElement call is being handled.

If you're mapping dynamic children you should be passing them in as an array, not using argument spread to pass them as separate arguments. I suspect this will make reconciliation quicker (as long as you key the children).

Does that make sense? @ialexryan

[ialexryan]

Oh good point, I missed the spread operator there (why jsfiddle decided dark grey text on slightly darker grey was a good idea, I'll never know...)

So that's not my problem. Did you take a look at the code sample I added above? Or the code samples in the blog posts I linked? Those are what I'm talking about.

Thanks for the response :)

[ialexryan]

Oh wait, I think I missed the bombshell in that comment. If I understand you correctly, you're confirming that React won't give key warnings for fragments because fragments are semantically assumed to be static children.

That would take my confusion back to my original post: "if we're relying on that expectation to avoid a major gotcha, I would assume it would be explicitly mentioned in the docs for fragments, and prominently featured in a warning-style yellow box. Instead, I don't believe it's mentioned at all." Is that accurate? I don't see any mention of the distinction in the announcement blog post, the docs, or really anywhere except that long-closed Github Issue thread.

[aweary]

If I understand you correctly, you're confirming that React won't give key warnings for fragments because fragments are semantically assumed to be static children.

Correct. Fragments work the same as other elements (div, ul, etc.) or components. Only arrays of elements need to be keyed.

// You wouldn't expect keys to be required on the `li` elements here
<ul>
  <li>First</li>
  <li>Second</li>
</ul>

// But you WOULD expect them to be required here
<ul>{items.map(item => <li key={item}>{item}</li>}</ul>

The same applies to React.Fragment. If a child is _an array of elements_ those elements must be keyed. If they're not, they don't need to be.

// No keys required on the `li` elements
<React.Fragment>
  <li>First</li>
  <li>Second</li>
</React.Fragment>

// Keys required on the array of `li` elements
<React.Fragment>
  {items.map(item => <li key={item}>{item}</li>}
</React.Fragment>

Is that accurate? I don't see any mention of the distinction in the announcement blog post, the docs, or really anywhere except that long-closed Github Issue thread.

The docs on React.Fragment link to this post, which discusses keys:

https://reactjs.org/blog/2017/11/28/react-v16.2.0-fragment-support.html

You can use the same way you’d use any other element, without changing the way you write JSX. No commas, no keys, no quotes.

Since this doesn't seem to be a bug I'm going to close this out, but feel free to ask any clarifying questions!

[ialexryan]

OK, I think I'm narrowing the issue down. Thanks so much for bearing with me.

It makes sense that any case where keys are useful/needed would involve an array at some point, since it probably comes from a map operation or something like it.

So the fact that

<React.Fragment>
  <p>1</p>
  <p>2</p>
   {/* so on... */}
</React.Fragment>

does not produce a key warning seems fine.

What confuses me is that

<React.Fragment>
  {[
  <p>1</p>,
  <p>2</p>,
   {/* so on... */}
   ]}
</React.Fragment>

also does not produce a key warning for me. That seems bad. That's what made me 😱.

Except...you said it should create a key warning, and jsfiddle seems to back that up. So I think it's likely that my experience is nonstandard and thus this isn't the big issue I thought it was.

Now just to figure out why 😅

[aweary]

No problem! If you're using helper functions like r.div and r.fragment, I would look there and see how they're passing children to React.createElement.

[ialexryan]

You read my mind. I just dug into our react factories and sure enough, there's the darn spread operator! And only in the special-case factory for fragment. It all makes sense now. Thanks so much for your patience and clarity 😄

[nmain]

Yes, the spread operator in my example was intentional (and I apologize for the JSFiddle coloring), and it looks like whoever made your factory had the same thought that I did: I've used spread fragments before in my own code in situations where I have an array but I know it's really static-ish and isn't going to have inserts/deletes done on it in a future render, so that I can clearly state my intent.

[rambabusaravanan]

My component prepares array of items. Within Fragment, now i am able to render without keys using React.createElement

  render() {
    return React.createElement(React.Fragment, {}, ...items)
  }

Thanks @ialexryan