Merge pull request #57 from sirgallifrey/fix/33k-children-issue-52

Fix expanding nodes with more than 32768 children

Author: Lodin

__tests__/FixedSizeTree.spec.tsx

@@ -1,6 +1,6 @@
 import {mount, ReactWrapper} from 'enzyme';
 import React, {createRef, FC} from 'react';
-import {FixedSizeList} from 'react-window';
+import {FixedSizeList, FixedSizeListProps} from 'react-window';
 import {
   FixedSizeNodeData,
   FixedSizeNodePublicState,
@@ -11,12 +11,13 @@ import {
   TreeWalker,
   TreeWalkerValue,
 } from '../src';
-import {NodeComponentProps} from '../src/Tree';
+import {NodeComponentProps, NodePublicState} from '../src/Tree';
 import {
   defaultTree,
   extractReceivedRecords,
   mockRequestIdleCallback,
   sleep,
+  treeWithLargeNode,
 } from './utils/misc';
 
 type TreeNode = Readonly<{
@@ -51,19 +52,10 @@ describe('FixedSizeTree', () => {
   let treeWalkerSpy: jest.Mock;
   let isOpenByDefault: boolean;
 
-  const getNodeData = (
+  let getNodeData: (
     node: TreeNode,
     nestingLevel: number,
-  ): TreeWalkerValue<ExtendedData, NodeMeta> => ({
-    data: {
-      id: node.id.toString(),
-      isOpenByDefault,
-      name: node.name,
-      nestingLevel,
-    },
-    nestingLevel,
-    node,
-  });
+  ) => TreeWalkerValue<ExtendedData, NodeMeta>;
 
   function* treeWalker(): ReturnType<TreeWalker<ExtendedData, NodeMeta>> {
     yield getNodeData(tree, 0);
@@ -105,6 +97,20 @@ describe('FixedSizeTree', () => {
 
     isOpenByDefault = true;
 
+    getNodeData = (
+      node: TreeNode,
+      nestingLevel: number,
+    ): TreeWalkerValue<ExtendedData, NodeMeta> => ({
+      data: {
+        id: node.id.toString(),
+        isOpenByDefault,
+        name: node.name,
+        nestingLevel,
+      },
+      nestingLevel,
+      node,
+    });
+
     treeWalkerSpy = jest.fn(treeWalker);
 
     component = mountComponent();
@@ -596,5 +602,82 @@ describe('FixedSizeTree', () => {
       list = component.find(FixedSizeList);
       expect(list.prop('itemCount')).toBe(7);
     });
+
+    it('correctly collapses node with 100.000 children', async () => {
+      tree = treeWithLargeNode;
+      component = mountComponent();
+
+      const records = extractReceivedRecords<
+        FixedSizeListProps,
+        ExtendedData,
+        NodePublicState<ExtendedData>
+      >(component.find(FixedSizeList));
+
+      const {setOpen} = records.find(
+        (record) => record.data.id === 'largeNode-1',
+      )!;
+
+      await setOpen(false);
+      component.update(); // Update the wrapper to get the latest changes
+
+      const updatedRecords = extractReceivedRecords(
+        component.find(FixedSizeList),
+      );
+
+      expect(updatedRecords.map(({data: {id}}) => id)).toEqual([
+        'root-1',
+        'smallNode-1',
+        'smallNodeChild-1',
+        'smallNodeChild-2',
+        'largeNode-1',
+        'smallNode-2',
+        'smallNodeChild-3',
+        'smallNodeChild-4',
+      ]);
+    });
+
+    it('correctly expands node with 100.000 children', async () => {
+      getNodeData = (
+        node: TreeNode,
+        nestingLevel: number,
+      ): TreeWalkerValue<ExtendedData, NodeMeta> => ({
+        data: {
+          id: node.id.toString(),
+          isOpenByDefault: node.id !== 'largeNode-1',
+          name: node.name,
+          nestingLevel,
+        },
+        nestingLevel,
+        node,
+      });
+      tree = treeWithLargeNode;
+
+      component = mountComponent();
+
+      const records = extractReceivedRecords<
+        FixedSizeListProps,
+        ExtendedData,
+        NodePublicState<ExtendedData>
+      >(component.find(FixedSizeList));
+
+      const {setOpen} = records.find(
+        (record) => record.data.id === 'largeNode-1',
+      )!;
+
+      await setOpen(true);
+      component.update(); // Update the wrapper to get the latest changes
+
+      const updatedRecords = extractReceivedRecords(
+        component.find(FixedSizeList),
+      );
+
+      expect(updatedRecords.slice(-5).map(({data: {id}}) => id)).toEqual([
+        'largeNodeChild-99999',
+        'largeNodeChild-100000',
+        'smallNode-2',
+        'smallNodeChild-3',
+        'smallNodeChild-4',
+      ]);
+    });
   });
 });

__tests__/utils/misc.ts

@@ -78,3 +78,44 @@ export const defaultTree = {
   id: 'foo-1',
   name: 'Foo #1',
 };
+
+const getLargeSetOfChildren = () => {
+  const children = [];
+
+  for (let i = 0; i < 100000; i++) {
+    children.push({
+      id: `largeNodeChild-${i + 1}`,
+      name: `Large Node Child #${i + 1}`,
+    });
+  }
+
+  return children;
+};
+
+export const treeWithLargeNode = {
+  children: [
+    {
+      children: [
+        {id: 'smallNodeChild-1', name: 'Small Node Child #1'},
+        {id: 'smallNodeChild-2', name: 'Small Node Child #2'},
+      ],
+      id: 'smallNode-1',
+      name: 'Small Node #1',
+    },
+    {
+      children: getLargeSetOfChildren(),
+      id: 'largeNode-1',
+      name: 'Large Node #1',
+    },
+    {
+      children: [
+        {id: 'smallNodeChild-3', name: 'Small Node Child #3'},
+        {id: 'smallNodeChild-4', name: 'Small Node Child #4'},
+      ],
+      id: 'smallNode-2',
+      name: 'Small Node #2',
+    },
+  ],
+  id: 'root-1',
+  name: 'Root #1',
+};

src/Tree.tsx

@@ -386,6 +386,7 @@ const generateNewTree = <
 };
 
 const MAX_FUNCTION_ARGUMENTS = 32768;
+const SPLICE_DEFAULT_ARGUMENTS_NUMBER = 2;
 
 // If we need to perform only the update, treeWalker won't be used. Update will
 // work internally, traversing only the subtree of elements that require
@@ -456,8 +457,6 @@ const updateExistingTree = <
           [index + 1, countToRemove],
         ];
 
-        let orderPartsCursor = 0;
-
         // Unfortunately, splice cannot work with big arrays. If array exceeds
         // some length it may fire an exception. The length is specific for
         // each engine; e.g., MDN says about 65536 for Webkit. So, to avoid this
@@ -478,14 +477,15 @@ const updateExistingTree = <
             : true;
 
           if (record.isShown) {
-            orderParts[orderPartsCursor].push(record.public.data.id);
+            const currentOrderPart = orderParts[orderParts.length - 1];
+            currentOrderPart.push(record.public.data.id);
 
             if (
-              orderParts[orderPartsCursor].length === MAX_FUNCTION_ARGUMENTS
+              currentOrderPart.length ===
+              MAX_FUNCTION_ARGUMENTS + SPLICE_DEFAULT_ARGUMENTS_NUMBER
             ) {
-              orderPartsCursor += 1;
               orderParts.push([
-                index + 1 + orderPartsCursor * MAX_FUNCTION_ARGUMENTS,
+                index + 1 + MAX_FUNCTION_ARGUMENTS * orderParts.length,
                 0,
               ]);
             }