webtreemap icon indicating copy to clipboard operation
webtreemap copied to clipboard

The `join` is not actually used.

Open AkatQuas opened this issue 2 years ago • 2 comments

https://github.com/evmar/webtreemap/blob/5dbfbff2619004ffeb38cd3230ee8bcf3f97a96c/src/tree.ts#L68-L82

The join function is not used though.

I think it's possible to modify the line 78. However the id property mismatch the join function type.

Maybe you can give me a hint.

Thanks.

AkatQuas avatar Mar 02 '22 07:03 AkatQuas

I agree that the join function isn't used. It looks like that can be deleted just fine.

diff --git a/src/tree.ts b/src/tree.ts
index 8d78894..e313c89 100644
--- a/src/tree.ts
+++ b/src/tree.ts
@@ -65,13 +65,10 @@ export function treeify(data: Array<[string, number]>): Node {
  * @param join If given, a function that joins the names of the parent and
  * child.
  */
-export function flatten(
-  n: Node,
-  join = (parent: string, child: string) => `${parent}/${child}`
-) {
+export function flatten(n: Node) {
   if (n.children) {
     for (const c of n.children) {
-      flatten(c, join);
+      flatten(c);
     }
     if (n.children.length === 1) {
       const child = n.children[0];

I'm not sure what you're saying about the id property though.

paulirish avatar Mar 02 '22 17:03 paulirish

As far as I concered, the purpose of flatten is to flatten nodes that have only one child, which means a child node hoist.

So the new id of the parent should have something to do with the child.id. And here comes the join function to do the job.

However, the id is optional, as commented, in the type Node. The TS compiler yells when make change like the following.

@@ -75,7 +109,7 @@ export function flatten(
     }
     if (n.children.length === 1) {
       const child = n.children[0];
-      n.id += '/' + child.id;
+      n.id = join(n.id, child.id);
       n.children = child.children;
     }
   }
image

AkatQuas avatar Mar 03 '22 02:03 AkatQuas