State is carried between components inside and outside of fragments

[dacioromero]

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

Bug

What is the current behavior?

If a component rendered in a Fragment and then outside of it it is considered the same instance by React.

This means that:

<Fragment>
  <Component />
</Fragment>

shares state with:

<Component />

I see how this could be seen as beneficial behavior and can be easily fixed with keys.

When I was conditionally rendering either one Material UI Button or two inside a fragment would cause the ripple effect from the first Button in the fragment to be "transferred" to the single Button which is odd since the JSX structure is different.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. Your bug will get fixed much faster if we can run your code and it doesn't have dependencies other than React. Paste the link to your JSFiddle (https://jsfiddle.net/Luktwrdm/) or CodeSandbox (https://codesandbox.io/s/new) example below:

https://github.com/dacioromero/react-unexpected-state-fragments

What is the expected behavior?

Fragments should be a factor that React considers when determining if instances of a component is the same. Components should be different instances between renders if one is placed inside a Fragment and another is not.

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

React 16.11.0, Firefox Developer Edition 71.0b7, Ubuntu 19.10.

Not sure if this worked as I would expect in previous versions.

[threepointone]

Yeah, this looks like a bug. codesandbox version - https://codesandbox.io/s/amazing-shadow-kmume

[gaearon]

This was an intentional decision: #10783 (comment), #10783 (comment), https://gist.github.com/clemmy/b3ef00f9507909429d8aa0d3ee4f986b. I think that was decided to align with the array behavior. I recall this was a hot point of debate but don't quite recall why this conclusion was reached. @sebmarkbage or @acdlite might have more context.

[gaearon]

I think the motivation for this is that it's pretty common to start with code like return <Button /> and then want to add a branch like

if (foo) {
 // your old code
  return <Button />
} else {
  return (
    <>
      <Button />
      <OkMessage />
    </>
  )
}

At this point the person is usually thinking "I'm just adding another item" and they often don't realize reconciliation semantics mean the state would be lost. Explaining that now they have to wrap both in a fragment can get pretty confusing. Hence this special case for top-level fragments/arrays.

[dacioromero]

At this point the person is usually thinking "I'm just adding another item"

I was thinking of a situation like that when I realized this could be intentional. It definitely makes sense and I see why it was debated.

Personally, I think it's confusing that the behavior changes like that. I could just add keys or make a fake Fragment (({ children }) => children) if I wanted it to behave as I'd expect, but it's not my preference.

[gaearon]

Yeah. It’s hard to say which case is more common but anecdotally this feedback is very rare (I think the last time I heard about this issue was when I filed a similar one four years ago). So it means the current heuristic works well enough and it’s probably not worth breaking it.