Skip to content

Conversation

@dpnova
Copy link
Contributor

@dpnova dpnova commented Nov 25, 2025

PR to add a traverse function to the dag. This allows visitors to be used to coalesce or collect things from the dag.

OLD PR DESC:
just a way to visualize a dag you are working on.

image

closes #50

@sha1n
Copy link
Owner

sha1n commented Nov 25, 2025

Hey @dpnova,

Thanks a lot for the PR!
After reviewing the changes and following what I wrote on #50, I want to suggest a design that will give you a lot of flexibility while keeping the DAG data structure clean of irrelevant logic.

Here is a traverse method that implements a DFS algorithm and calls a visitor for every visited node with the node and some contextual information, including user defined context you can use to accumulate data. I think this can give you what you need plus more:

// Visitor type def
type DAGVisitor<T, C extends object> = (node: T, parent: T | null, depth: number, context: C) => void;

  // This is a DAG method
  //  - 'context' can be any object the user wants to pass through the traversal
  traverse<C extends object>(visitor: DAGVisitor<T, C>, context: C): void {
    // Build outgoing edges map: parent_id -> [child_id, child_id, ...]
    // Note: We could use this.reverse() to get inverted dependencies, but building
    // the map directly is more efficient (O(E) vs O(V+E)) and avoids creating a full graph copy
    const outgoing = new Map<string, string[]>();
    for (const node of this.nodesById.values()) {
      for (const depId of node.dependencies) {
        let children = outgoing.get(depId);
        if (!children) {
          children = [];
          outgoing.set(depId, children);
        }
        children.push(node.id);
      }
    }

    // Recursive DFS traversal function
    const visitNode = (nodeId: string, parent: T | null, depth: number) => {
      const node = this.nodesById.get(nodeId);
      if (!node) return; // Safety check for missing nodes

      // Visit current node with parent context and depth information
      visitor(node.data, parent, depth, context);

      // Recursively visit all children in depth-first order
      const children = outgoing.get(nodeId) || [];
      for (const childId of children) {
        visitNode(childId, node.data, depth + 1);
      }
    };

    // Start DFS traversal from all root nodes (nodes with no dependencies)
    const roots = [...this.roots()];
    for (const root of roots) {
      visitNode(root.id, null, 0); // Root nodes have no parent and start at depth 0
    }
  }

Here is a very simple print implementation and its output:

const createDAG = require('./dist/index.js').default;

// Demo script
const dag = createDAG();
dag.addEdge({ id: 'A' }, { id: 'B' });
dag.addEdge({ id: 'B' }, { id: 'C' });
dag.addEdge({ id: 'B' }, { id: 'D' });
dag.addEdge({ id: 'D' }, { id: 'F' });
dag.addEdge({ id: 'A' }, { id: 'E' });
dag.addNode({ id: 'G' });

const visited = [];
dag.traverse((node, parent, depth, _ctx) => {
  visited.push(`${'  '.repeat(depth)}${node.id}`);
}, {});

console.log(visited.join('\n'));

Output:

node printVisitorDemo.js

A
  B
    C
    D
      F
  E
G

Do you think that can work for you? Would you pick this up and give it a nice polish to make it production grade? I'm pretty sure it works well, but only comprehensive tests can prove this 🤓

@dpnova
Copy link
Contributor Author

dpnova commented Nov 25, 2025

Thanks for the reply!

Can do.

I did consider doing something like this too, and moving the print specific part to a separate utils file, would you be OK with that addition?

@sha1n
Copy link
Owner

sha1n commented Nov 25, 2025

Sure it can be a useful utility! The main thing for me is the separation and opening the DAG to any traversal use-case (at least DFS). We can then provide a lib/visitors.ts with exports of common useful visitor implementations and anyone can compose or wrote custom ones...

I suggest that we start with the traverse function. Ger it merged, and then add the print visitor implementation in a separate PR.

10x 🙏

@dpnova
Copy link
Contributor Author

dpnova commented Nov 26, 2025

I've updated the PR. I added a couple of extra params to the visitor system to help with traversal progress context.

I'll open a separate PR with the two simple visitors we've seen so far.

EDIT: PR is here - I'll repoint it to your fork when we have merged this one 👍

@dpnova dpnova changed the title feat: first cut at a little visualisation util for the dag. feat: add traverse function to allow visitors access to the dag items Nov 26, 2025
@sha1n
Copy link
Owner

sha1n commented Nov 27, 2025

Thank you. I will try to get to it sometime over the weekend.

@sha1n
Copy link
Owner

sha1n commented Dec 2, 2025

Hey @dpnova

I have made a few significant changes and pushed them to a branch that mirrors the branch on your fork. Please review the last commit and let me know what you think.

Summary of changes:

  • Refactored the method signature for better clarity and args scoping
  • Added several test-cases to ground the contract

💡 Pay extra attention to asserted semantics, like sibling traversal order and possibly other conditions that are less obvious. If you can think about any behavior that is not properly covered, I would appreciate a comment.

Thanks a lot for the effort 🙏

@dpnova
Copy link
Contributor Author

dpnova commented Dec 2, 2025

Cheers! Just travelling in New Zealand at the moment, will be back into things in the last half of the week 👍

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

Successfully merging this pull request may close these issues.

Make nodesById public

2 participants