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

The join is not actually used. #44

Open
AkatQuas opened this issue Mar 2, 2022 · 2 comments
Open

The join is not actually used. #44

AkatQuas opened this issue Mar 2, 2022 · 2 comments

Comments

@AkatQuas
Copy link

AkatQuas commented Mar 2, 2022

webtreemap/src/tree.ts

Lines 68 to 82 in 5dbfbff

export function flatten(
n: Node,
join = (parent: string, child: string) => `${parent}/${child}`
) {
if (n.children) {
for (const c of n.children) {
flatten(c, join);
}
if (n.children.length === 1) {
const child = n.children[0];
n.id += '/' + child.id;
n.children = child.children;
}
}
}

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.

@paulirish
Copy link
Contributor

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.

@AkatQuas
Copy link
Author

AkatQuas commented Mar 3, 2022

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

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