Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add rootParentId option to fit the case when all rows contain a valid parentId string #23

Closed
rrbe opened this issue Aug 11, 2020 · 4 comments

Comments

@rrbe
Copy link
Contributor

rrbe commented Aug 11, 2020

Why

In most cases, our tree list contains a row whose parentId field is one of null / undefined / '', but if the tree list is a subset of original tree list, chances are that the transform result is not right, no matter the throwIfOrphans option is set to true or false

example:

import { arrayToTree } from 'performant-array-to-tree'

const list = [
  { id: "aaa", parentId: "notexist" },
  { id: "bbb", parentId: "aaa" },
  { id: "ccc", parentId: "bbb" },
  { id: "ddd", parentId: "bbb" },
];
const result = arrayToTree(list, {
  dataField: null,
});
console.log(JSON.stringify(result, null, 2)); // the output is []

Suggestion

If we add an option named rootParentId, items whose parentId equals rootParentId will be treat as a root item.

The above code's output should be like this

const result = arrayToTree(list, {
  dataField: null,
  rootParentId: "notexist",
});
[
  {
    "id": "aaa",
    "parentId": "notexist",
    "children": [
      {
        "id": "bbb",
        "parentId": "aaa",
        "children": [
          {
            "id": "ccc",
            "parentId": "bbb",
            "children": []
          },
          {
            "id": "ddd",
            "parentId": "bbb",
            "children": []
          }
        ]
      }
    ]
  }
]

How

Add rootParentId into option, then replace

if (parentId === null || parentId === undefined || parentId === '') {
      rootItems.push(TreeItem)
    }

with

if (parentId === null || parentId === undefined || parentId === '' || parentId === conf.rootParentId) {
      // is a root item
      rootItems.push(TreeItem)
    }

Of course we can also make rootParentId an array, the initial value is [null, undefined, ''] and will later be merged with user defined rootParentIds

@philipstanislaus
Copy link
Owner

That makes sense to me. Would you create a PR with test cases for this suggestion?

Many thanks!

@rrbe
Copy link
Contributor Author

rrbe commented Aug 13, 2020

I have created the pr with test cases.

All tests passed but it seems there is something wrong with circle ci config. Runing yarn run test-and-send-cov-to-coveralls, ci gives Bad response: 422 {"message":"Couldn't find a repository matching this job.","error":true} response.
So the build is blocked

@philipstanislaus
Copy link
Owner

Thanks! Could you try to push again to your branch?

@philipstanislaus
Copy link
Owner

Landed in #24

Thanks for your contribution 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants