tree-change / fast-deep-equal causes Maximum Call Stack Size Exceeded in jest

[amoshydra]

🐛 Bug Report

tree-change-hook used by react-floater is comparing between the properties in React fibre. This can sometimes cause many comparisons to be made. Eventually causes jest to throw "Maximum Call Stack Size Exceeded"

I am not able to verify if this is due to circular reference or due to too many comparisons being done.

To Reproduce

This issue is not reproducible in small project.

However, I have verified that comparison has indeed been made on the properties of React Fibre.

This issue is verified by placing a console.log before this line:

      console.log(key, keys);
      if (!equal(a[key], b[key])) return false;

https://github.com/epoberezkin/fast-deep-equal/blob/a8e7172b6c411ec320d6045fd4afbd2abc1b4bde/src/index.jst#L71

The resultant console.log look like this:

  console.log
    statusWrapper [ 'currentPlacement', 'positionWrapper', 'status', 'statusWrapper' ]

      at equal (node_modules/fast-deep-equal/index.js:39:15)

  console.log
    status [ 'currentPlacement', 'positionWrapper', 'status', 'statusWrapper' ]

      at equal (node_modules/fast-deep-equal/index.js:39:15)

  console.log
    children [
      'open',       'id',
      'eventDelay', 'event',
      'target',     'portalElement',
      'offset',     'styles',
      'hideArrow',  'placement',
      'component',  'children'
    ]

      at equal (node_modules/fast-deep-equal/index.js:39:15)

  console.log
    _store [
      '$$typeof', 'type',
      'key',      'ref',
      'props',    '_owner',
      '_store'
    ]

      at equal (node_modules/fast-deep-equal/index.js:39:15)

  console.log
    _owner [
      '$$typeof', 'type',
      'key',      'ref',
      'props',    '_owner',
      '_store'
    ]

      at equal (node_modules/fast-deep-equal/index.js:39:15)

  console.log
    _debugHookTypes [
      'tag',                'key',
      'elementType',        'type',
      'stateNode',          'return',
      'child',              'sibling',
      'index',              'ref',
      'pendingProps',       'memoizedProps',
      'updateQueue',        'memoizedState',
      'dependencies',       'mode',
      'flags',              'nextEffect',
      'firstEffect',        'lastEffect',
      'lanes',              'childLanes',
      'alternate',          'actualDuration',
      'actualStartTime',    'selfBaseDuration',
      'treeBaseDuration',   '_debugID',
      '_debugSource',       '_debugOwner',
      '_debugNeedsRemount', '_debugHookTypes'
    ]

      at equal (node_modules/fast-deep-equal/index.js:39:15)

  console.log
    _debugNeedsRemount [
      'tag',                'key',
      'elementType',        'type',
      'stateNode',          'return',
      'child',              'sibling',
      'index',              'ref',
      'pendingProps',       'memoizedProps',
      'updateQueue',        'memoizedState',
      'dependencies',       'mode',
      'flags',              'nextEffect',
      'firstEffect',        'lastEffect',
      'lanes',              'childLanes',
      'alternate',          'actualDuration',
      'actualStartTime',    'selfBaseDuration',
      'treeBaseDuration',   '_debugID',
      '_debugSource',       '_debugOwner',
      '_debugNeedsRemount', '_debugHookTypes'
    ]

//
//
// 2195 comparisons are done on the properties of React Fibre after this
//

Expected behavior

React-floater's useTreeChange (tree-change-hook) shouldn't compare React's fibre's node.

React-floater's tree-change-hook should use fast-deep-equal/react instead.

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');

Proposed fix: https://github.com/gilbarbara/tree-changes/pull/19

Link to repl or repo (highly encouraged)

N/A

Run npx envinfo --system --binaries --npmPackages react-floater

Paste the results here:

N/A

Using react-floater 0.8.0

[arkadiusz-czekajski-viacomcbs]

If you look at the react-floater code, you can see that this heavy-computation tree comparison lib is launched there on the entire props object (together with children prop) only to check if one of the three of them have changed. This is highly sub-optimal. This should be entirely rewritten using simple comparisons or ideally just hooks with right dependencies.

For me, the maximum call stack size error appeared on tests (using React Testing Library), on every test that involved Floater with children being passed to it.

As a temporary workaround, I had to move the children (activator element) out of the Floater, use target to point onto the activator element and write the opening-closing behavior with my own events using open prop on floater:

<Floater>
  <MyButton />
</Floater>

↓

<MyButton ref={targetRef} {...handleMouseEventsToManipulateOpenState} />
{targetRef.current && <Floater target={targetRef.current} open={open} />}

[gilbarbara]

Fixed in 0.8.1

I've replaced the fast-deep-equal package with @gilbarbara/deep-equal that supports ESM

Thanks