Tree Data Grid x Parent/Child Relation Dataset x Failures on subsequent component mount

[ghiscoding]

Discussed in https://github.com/ghiscoding/slickgrid-universal/discussions/1655

^{Originally posted by **tnaum-ms** August 28, 2024} I'm currently investigating an issue that arises when I mount my SlickGrid Tree Data as a React component for the second time. The problem occurs when I toggle between different data view formats in my application. Specifically, switching away from the Tree Data view and then returning to it results in a failure of the Tree Data Component. The data set I’m working with follows a Parent/Child structure, where the `id` and `parentId` fields are of type `string`. _(It would be helpful to include a note in the documentation indicating the preferred or faster data input method.)_ The SlickGrid Tree Data Grid throws the following error: > Each data element must implement a unique 'id' property Upon investigation, I discovered that on the second mount (not just a second render), one of the internal data mutation functions introduces duplicates into my data set. This causes the `ensureIdUniqueness` function to fail. The issue seems to be related to the function located here: https://github.com/ghiscoding/slickgrid-universal/blob/ce8b4000cf72fa78293e50fba886ae883c08f39b/packages/common/src/services/utilities.ts#L158 This specific loop appears to be introducing duplicates: https://github.com/ghiscoding/slickgrid-universal/blob/ce8b4000cf72fa78293e50fba886ae883c08f39b/packages/common/src/services/utilities.ts#L173-L187 I'm continuing to investigate the root cause, but since this issue only occurs on the second mount, I haven’t yet identified any obvious side effects. If this is something you’ve encountered before, I would greatly appreciate any insights or suggestions you could share. Thank you for your assistance. Best, Tomasz

[jano-kucera]

Hi, I've hit upon this issue a long time ago, the children array didn't get reset when recreating the tree. Fixed it via this patch.

diff --git a/node_modules/@slickgrid-universal/common/dist/esm/services/utilities.js b/node_modules/@slickgrid-universal/common/dist/esm/services/utilities.js
index 2e9b2f9..d737958 100644
--- a/node_modules/@slickgrid-universal/common/dist/esm/services/utilities.js
+++ b/node_modules/@slickgrid-universal/common/dist/esm/services/utilities.js
@@ -140,7 +140,10 @@ export function unflattenParentChildArrayToTree(flatArray, options) {
     const roots = []; // items without parent which at the root
     // make them accessible by guid on this map
     const all = {};
-    inputArray.forEach((item) => all[item[identifierPropName]] = item);
+    inputArray.forEach((item) => {
+        all[item[identifierPropName]] = item;
+        delete item[childrenPropName];
+    });
     // connect childrens to its parent, and split roots apart
     Object.keys(all).forEach((id) => {
         const item = all[id];

[ghiscoding]

@jano-kucera I'm not sure why you're referencing this when it's already been fixed and pushed to production? If there's another problem or if this is a better fix than mine, then please contribute a fix as a new pull request so that it can benefit everyone. Thanks

Actually I wonder if the delete prop has better perf compare to the .findIndex() that I added in the latest fix I made, it's probably faster and if so then a PR would be nice. It might also be good to compare perf with console.time on a large dataset if possible.

https://github.com/ghiscoding/slickgrid-universal/blob/78653f9ff2807a53b0622e35b86ad35bd53e345f/packages/common/src/services/utilities.ts#L208-L214

cc @tnaum-ms

Also note that another user also found and provided another perf improvement related to Tree Data in this PR #1670 that I just merged today (to be released soon)

[jano-kucera]

From the code snippet, your fix does not handle removing children - that's why I commented :). If I find time I will measure it, but I believe deleting a prop will always be faster than an index lookup.

[ghiscoding]

@jano-kucera then a Pull Request would be welcome to fix it, thanks

[jano-kucera]

Also I just realized a big problem in the implementation, you access the id field, not the identifierPropName - thus if you use any other property name for IDs (even from gridOptions), you end up children replacing each other.