redux-form: Performance issue (crash) in large apps when using Field with children in React v16

Are you submitting a bug report or a feature request?

bug

What is the current behavior?

Reproduction:

Use redux-form with React v16 in a large application (a page with a lot of different nested React components)
Add a field with children:

<Field component={SomeCustomInput} ...>
   <SomeChild \>
   <SomeChild \>
   <SomeChild \>
</Field>

The page becomes so slow that the browser becomes unresponsive and crashes

What seems to happen is that <ConnectedField /> will execute shouldComponentUpdate, which will call lodash isEqual on its props. The children prop in react v16 contains FiberNode objects, which seem to have references to a lot (if not all) other FiberNode on the page. Because of this, lodash isEqual will take a very very long time recursively traversing through these objects.

Sandbox Link

Unfortunately I haven’t been able to reproduce this in stand-alone example. If I create a <Field> with children in one of the examples of this repo, I can see lodash baseIsEqualDeep being called with very long call stacks. However, it is not enough to visually slow down the page.

What’s your environment?

All browsers React v16 redux-form v7.0.4

About this issue

Original URL
State: closed
Created 7 years ago
Reactions: 1
Comments: 18 (11 by maintainers)

Most upvoted comments

… when the calculation to decide not to render takes up more time than actually doing the render … 🤦‍♂️

+20

erikras on Oct 5, 2017

doing consulting, often I see:

shouldComponentUpdate(nextProps) {
  if (JSON.stringify(nextProps) !== JSON.stringify(this.props) {
    ....
  }
}

can be as bad as this one…

this issue will be another good argument for this scenario 😅

+15

oieduardorabelo on Oct 5, 2017

Oh god.

+13

gaearon on Oct 5, 2017

Yes. I’m saying that, unless it is your component and you’re in control of its API, you shouldn’t assume that the only place where React elements can appear is this.props.children. They could be in any prop (or in some nested data structure inside a prop).

To me, it seems like it is never safe to do deep equality checks on props. And we never recommended doing that either.

gaearon on Oct 5, 2017

I think @gaearon meant that no deep equality check should happen, regardless the prop name. If you want to preserve identities - just cache ur complex data structures (like <span/>) in ur example, so they can be shallowly equaled between renders.

Andarist on Oct 5, 2017

@umidbekkarimov Interesting approach, but I’m pretty happy with my solution in #3480. If there are (or were) children, shouldComponentUpdate() must return true.

erikras on Oct 5, 2017