diff --git a/packages/fdr-sdk/src/navigation/utils/deleteChild.ts b/packages/fdr-sdk/src/navigation/utils/deleteChild.ts index 6aebaa9f42..78930c79ed 100644 --- a/packages/fdr-sdk/src/navigation/utils/deleteChild.ts +++ b/packages/fdr-sdk/src/navigation/utils/deleteChild.ts @@ -26,6 +26,11 @@ export function mutableDeleteChild( // if the node to be deleted is a section, remove the overviewPageId if (FernNavigation.isSectionOverview(node)) { (node as MarkOptional).overviewPageId = undefined; + + if (node.children.length === 0) { + return "deleted"; + } + return "noop"; } else { throw new UnreachableCaseError(node); @@ -44,60 +49,65 @@ export function mutableDeleteChild( switch (parent.type) { case "apiPackage": parent.children = parent.children.filter((child) => child.id !== node.id); - return "deleted"; + break; case "apiReference": parent.children = parent.children.filter((child) => child.id !== node.id); parent.changelog = parent.changelog?.id === node.id ? undefined : parent.changelog; - return "deleted"; + break; case "changelog": parent.children = parent.children.filter((child) => child.id !== node.id); - return "deleted"; + break; case "changelogYear": parent.children = parent.children.filter((child) => child.id !== node.id); - return "deleted"; + break; case "changelogMonth": parent.children = parent.children.filter((child) => child.id !== node.id); - return "deleted"; + break; case "endpointPair": return "should-delete-parent"; case "productgroup": parent.children = parent.children.filter((child) => child.id !== node.id); parent.landingPage = parent.landingPage?.id === node.id ? undefined : parent.landingPage; - return "deleted"; + break; case "product": - return "should-delete-parent"; case "root": return "should-delete-parent"; case "unversioned": if (node.id === parent.landingPage?.id) { parent.landingPage = undefined; - return "deleted"; } - return "should-delete-parent"; + break; case "section": parent.children = parent.children.filter((child) => child.id !== node.id); - return "deleted"; + break; case "sidebarGroup": parent.children = parent.children.filter((child) => child.id !== node.id); - return "deleted"; + break; case "tab": return "should-delete-parent"; case "sidebarRoot": parent.children = parent.children.filter((child) => child.id !== node.id); - return "deleted"; + break; case "tabbed": parent.children = parent.children.filter((child) => child.id !== node.id); - return "deleted"; + break; case "version": if (node.id === parent.landingPage?.id) { parent.landingPage = undefined; - return "deleted"; } - return "should-delete-parent"; + break; case "versioned": parent.children = parent.children.filter((child) => child.id !== node.id); - return "deleted"; + break; default: throw new UnreachableCaseError(parent); } + + if (FernNavigation.isPage(parent)) { + return "noop"; + } else if (FernNavigation.getChildren(parent).length > 0) { + return "deleted"; + } else { + return "should-delete-parent"; + } } diff --git a/packages/fdr-sdk/src/navigation/utils/pruneNavigationTree.ts b/packages/fdr-sdk/src/navigation/utils/pruneNavigationTree.ts index 6d4e946fcf..2dea9530bf 100644 --- a/packages/fdr-sdk/src/navigation/utils/pruneNavigationTree.ts +++ b/packages/fdr-sdk/src/navigation/utils/pruneNavigationTree.ts @@ -31,8 +31,8 @@ function mutablePruneNavigationTree( // after deletion, if the node no longer has any children, we can delete the parent node too // but only if the parent node is NOT a visitable page - shouldDeleteParent: (parent: FernNavigation.NavigationNodeParent) => - !hasChildren(parent) && !FernNavigation.isPage(parent), + // shouldDeleteParent: (parent: FernNavigation.NavigationNodeParent) => + // !hasChildren(parent) && !FernNavigation.isPage(parent), }); if (result == null) { diff --git a/packages/fdr-sdk/src/utils/traversers/__test__/prunetree.test.ts b/packages/fdr-sdk/src/utils/traversers/__test__/prunetree.test.ts index fbf5f020ca..e52eb27a87 100644 --- a/packages/fdr-sdk/src/utils/traversers/__test__/prunetree.test.ts +++ b/packages/fdr-sdk/src/utils/traversers/__test__/prunetree.test.ts @@ -7,6 +7,11 @@ const DELETER = (parent: Record | undefined, child: Record): DeleterAction => { return "deleted"; } parent.children = parent.children.filter((c) => c.id !== child.id); + + if (parent.children.length === 0) { + return "should-delete-parent"; + } + return "deleted"; }; diff --git a/packages/fdr-sdk/src/utils/traversers/prunetree.ts b/packages/fdr-sdk/src/utils/traversers/prunetree.ts index 122d20ec5d..d34aad46dc 100644 --- a/packages/fdr-sdk/src/utils/traversers/prunetree.ts +++ b/packages/fdr-sdk/src/utils/traversers/prunetree.ts @@ -1,4 +1,3 @@ -import { UnreachableCaseError } from "ts-essentials"; import { bfs } from "./bfs"; import { DeleterAction } from "./types"; @@ -17,16 +16,6 @@ interface PruneTreeOptions { */ deleter: (parent: PARENT | undefined, child: NODE) => DeleterAction; - /** - * After the child is deleted, we can check if the parent should be deleted too, - * e.g. if the parent has no children left. - * - * @param parent node - * @returns **true** if the node should be deleted - * @default parent => getChildren(parent).length === 0 - */ - shouldDeleteParent?: (parent: PARENT) => boolean; - /** * If there are circular references, we can use this function to get a unique identifier for the node. * @@ -37,48 +26,52 @@ interface PruneTreeOptions { getPointer?: (node: NODE) => POINTER; } -// TODO: this algorithm is not optimal, as it traverses the tree twice, and should be refactored to traverse only once -// it would be more efficient to BFS the tree once, collect all the nodes in an array, and reverse the array to delete the nodes from the bottom up export function prunetree( root: ROOT, opts: PruneTreeOptions, ): [result: ROOT | undefined, deleted: ReadonlySet] { - const { - predicate, - getChildren, - deleter, - shouldDeleteParent = (parent) => getChildren(parent).length === 0, - getPointer = (node) => node as unknown as POINTER, - } = opts; + const { predicate, getChildren, deleter, getPointer = (node) => node as unknown as POINTER } = opts; const deleted = new Set(); - const visitor = (node: NODE, parents: readonly PARENT[]) => { - // if the node or its parents was already deleted, we don't need to traverse it - if ([...parents, node].some((parent) => deleted.has(getPointer(parent)))) { - return "skip"; + const nodes: [NODE, readonly PARENT[]][] = []; + + bfs( + root, + (node, parents) => { + nodes.unshift([node, parents]); + }, + getChildren, + ); + + nodes.forEach(([node, parents]) => { + const order = [...parents, node]; + const deletedIdx = order.findIndex((n) => deleted.has(getPointer(n))); + if (deletedIdx !== -1) { + order.slice(deletedIdx).forEach((n) => deleted.add(getPointer(n))); + return; } // continue traversal if the node is not to be deleted if (predicate(node)) { return; } + const ancestors = [...parents]; + const parent = ancestors.pop(); - deleteChildAndMaybeParent(node, parents, { - deleter, - shouldDeleteParent, - getPointer, - }).forEach((id) => { - deleted.add(id); - }); + let action = deleter(parent, node); + let toDelete = node; - // since the node was deleted, its children are deleted too - // we don't need to traverse them, nor do we need to keep them in the tree. - // note: the deleted set will NOT contain the children of this node - return "skip"; - }; + while (action === "should-delete-parent" && parent != null) { + deleted.add(getPointer(toDelete)); + toDelete = parent; + action = deleter(ancestors.pop(), parent); + } - bfs(root, visitor, getChildren); + if (action === "deleted") { + deleted.add(getPointer(toDelete)); + } + }); if (deleted.has(getPointer(root))) { return [undefined, deleted]; @@ -86,49 +79,3 @@ export function prunetree { - deleter: (parent: PARENT | undefined, child: NODE) => DeleterAction; - shouldDeleteParent: (parent: PARENT) => boolean; - getPointer: (node: NODE) => POINTER; -} - -function deleteChildAndMaybeParent( - node: NODE, - parents: readonly PARENT[], - opts: DeleteChildOptions, -): POINTER[] { - const { deleter, shouldDeleteParent, getPointer } = opts; - - const ancestors = [...parents]; - const parent = ancestors.pop(); - - const result = deleter(parent, node); - - // if the node was only updated, don't mark it as deleted - if (result === "noop") { - return []; - } - - // if no parent exists, then the node is the root - else if (parent == null) { - return [getPointer(node)]; - } - - // if the node was not deletable, then we need to delete the parent too - else if (result === "should-delete-parent") { - return [getPointer(node), ...deleteChildAndMaybeParent(parent, ancestors, opts)]; - } - - // traverse up the tree and delete the parent if necessary - else if (result === "deleted") { - if (shouldDeleteParent(parent)) { - return [getPointer(node), ...deleteChildAndMaybeParent(parent, ancestors, opts)]; - } else { - return [getPointer(node)]; - } - } - - // type safety check - throw new UnreachableCaseError(result); -}