using a component as title in a column definition can cause massive performance issues

[paintier]

Hi all,

the componentDidUpdate() method in material-table.js calls equal to figure out if the columns definition has changed.

This can lead a big performance issues if the title attribute is a component, e.g.

const columns = [
  {
    title: <FancyHeader />,
    field: 'some_field'
  }
...
]

see also this bugreport which describes why comparing two components using "equal" is a bad idea.

In edge cases this can lead to "maximum stack size exceeded" error messages.

Unfortunately there seems to exist no easy way to figure out if two react-components are rendered identically, so IMHO it would be best to consider the column definitions to have changed if the title attribute is a component and skip the equals test.

[matthewoestreich]

I think I'm following what you're saying.. but aren't we comparing two JavaScript objects? Not two React components?

Couldn't it cause issues elsewhere skipping the check (if the title is is an Element)?

So it is easy to cause "Maximum call stack exceeded" error when developer compares props.children in deep way

^^ quote from bug report

Lastly, as the bugreport states, you shouldn't compare children, as in props.children, using equal, which we are not. We are using equal to compare two JS objects.. eg. { foo: "bar" } === { foo: "baz" } ? true : false

[paintier]

.. but aren't we comparing two JavaScript objects? Not two React components?

If the "title" Attribute in the columns object is a React component, then it ends up comparing components (and their children).

Please have a look at this Codepen Example. I added a console.log() statement to show the contents of the columns object. Please have a look a components[0].title, and try to follow the _owner.alternate.alternate.alternate.... property.

Two things can happen:

• The _owner.alternate. structure is recursive: maximum stack size exceeded error.
• The _owner.alternate. structure is not recursive: equal(old, new) follows the _owner.alternate structure until it finds a difference

In the provided example it finds a difference after follwing _owner.alternate about 4 times, but this heavily depends on the component.

[Domino987]

@paintier I checked out your codepen and the default time of the equal check is around 0.02587890625ms. Are you sure that this causes the problems? Or can you create an example where the "maximum stack size exceeded" errors is thrown?

[paintier]

It actually happens in my production code which I cannot share here, maybe I'll find a way to extract the problematic part which causes the infinite recursion, but that might take a while

[thefat32]

fast-deep-equal has a custom implementation for react elements

To use with React (avoiding the traversal of React elements' _owner property that contains circular references and is not needed when comparing the elements - borrowed from react-fast-compare):

var equal = require('fast-deep-equal/react');
var equal = require('fast-deep-equal/es6/react');

It could help in such case, you can also use memos for title objects till this is resolved

[matthewoestreich]

Should we use that in this library? @peacechen @Domino987

[peacechen]

That looks like a great solution 👍

FYI: This project is already using fast-deep-equal.

core/src/material-table.js

Line 12 in bc64a60

import equal from 'fast-deep-equal';

Should be able to simply change that line to
import equal from 'fast-deep-equal/react';

[Domino987]

Yes this looks good. Can we create a PR?

[matthewoestreich]

I went ahead and made this change. If the following is correct, just let me know and I will publish a new version.

Thanks everyone!!

core/src/material-table.js

Line 12 in e880710

import equal from 'fast-deep-equal/react';

[Domino987]

I will close this, since it should be fixed with the new equal. Thanks everybody.