fix(reactivity): lay out the graph in bands of connected nodes

.changeset/graph-bands.md

@@ -0,0 +1,10 @@
+---
+'@solidjs/start-devtools': patch
+---
+
+The reactivity graph lays out each group of connected nodes in a band of its own, so unrelated chains no longer share columns.
+
+- Nodes with no edges share a grid under the bands instead of one tall column.
+- An edge that skips columns runs in a lane of its own instead of through other nodes.
+- A signal sits in the column right before the node that reads it.
+- A band keeps its arrangement while its nodes and edges stay the same. A band whose structure changed is arranged again, so its edges cross as little as possible.

src/dev-toolbar/reactivity/index.tsx

@@ -13,6 +13,7 @@ import { reactiveReport } from '../report/reactive-report.js';
 import { formatValue, typeName } from './format.js';
 import {
   ARROW_SIZE,
+  edgeKey,
   edgePath,
   layoutGraph,
   NODE_HEIGHT,
@@ -640,7 +641,11 @@ export default function ReactivityViewer(props: ReactivityViewerProps): JSX.Elem
                                   {(to) => (
                                     <path
                                       data-solid-reactivity-edge={role()}
-                                      d={edgePath(from(), to())}
+                                      d={edgePath(
+                                        from(),
+                                        to(),
+                                        layout().routes.get(edgeKey(edge())),
+                                      )}
                                       marker-end={
                                         role() === 'plain' || role() === 'muted'
                                           ? 'url(#solid-reactivity-arrow)'

src/dev-toolbar/reactivity/layout.test.ts

@@ -55,19 +55,25 @@ describe('layoutGraph', () => {
   });
 
   it('stacks nodes of one column without overlap', () => {
-    const layout = layoutGraph([node('a'), node('b')], []);
-    const first = layout.nodes.get('a')!;
-    const second = layout.nodes.get('b')!;
+    const layout = layoutGraph(
+      [node('a'), node('b'), node('c')],
+      [
+        { from: 'a', to: 'b' },
+        { from: 'a', to: 'c' },
+      ],
+    );
+    const first = layout.nodes.get('b')!;
+    const second = layout.nodes.get('c')!;
 
     expect(Math.abs(second.y - first.y)).toBeGreaterThanOrEqual(NODE_HEIGHT);
   });
 
-  it('puts an effect that subscribes to nothing before the signals', () => {
+  it('puts a node with no edges in the grid under the bands', () => {
     const layout = layoutGraph([node('a'), effect('lonely'), node('b')], [{ from: 'a', to: 'b' }]);
 
-    expect(layout.nodes.get('lonely')!.layer).toBe(0);
-    expect(layout.nodes.get('a')!.layer).toBe(1);
-    expect(layout.nodes.get('b')!.layer).toBe(2);
+    expect(layout.nodes.get('a')!.layer).toBe(0);
+    expect(layout.nodes.get('b')!.layer).toBe(1);
+    expect(layout.nodes.get('lonely')!.y).toBeGreaterThan(layout.nodes.get('b')!.y + NODE_HEIGHT);
   });
 
   it('leaves an effect with sources where its sources put it', () => {
@@ -77,10 +83,93 @@ describe('layoutGraph', () => {
     expect(layout.nodes.get('watcher')!.layer).toBe(1);
   });
 
-  it('keeps one column when every node is a detached effect', () => {
-    const layout = layoutGraph([effect('one'), effect('two')], []);
+  it('spreads nodes with no edges over rows of at least four columns', () => {
+    const layout = layoutGraph(
+      ['one', 'two', 'three', 'four', 'five'].map((id) => effect(id)),
+      [],
+    );
 
-    expect(layout.layers).toBe(1);
+    expect(layout.layers).toBe(4);
+    expect(layout.nodes.get('four')!.layer).toBe(3);
+    expect(layout.nodes.get('five')!.layer).toBe(0);
+    expect(layout.nodes.get('five')!.index).toBe(1);
+  });
+
+  it('gives each group of connected nodes a band of its own', () => {
+    const layout = layoutGraph(
+      [node('a'), node('b'), node('c'), node('d')],
+      [
+        { from: 'a', to: 'b' },
+        { from: 'c', to: 'd' },
+      ],
+    );
+    const firstBandBottom = Math.max(layout.nodes.get('a')!.y, layout.nodes.get('b')!.y);
+
+    expect(layout.nodes.get('c')!.y).toBeGreaterThan(firstBandBottom + NODE_HEIGHT);
+    expect(layout.nodes.get('d')!.y).toBe(layout.nodes.get('c')!.y);
+  });
+
+  it('routes an edge that skips a column through a waypoint beside the nodes', () => {
+    const layout = layoutGraph(
+      [node('a'), node('b'), node('c')],
+      [
+        { from: 'a', to: 'b' },
+        { from: 'b', to: 'c' },
+        { from: 'a', to: 'c' },
+      ],
+    );
+    const route = layout.routes.get('a>c')!;
+
+    expect(route).toHaveLength(1);
+    expect(route[0]!.layer).toBe(1);
+    expect(route[0]!.y).not.toBe(layout.nodes.get('b')!.y);
+  });
+
+  it('moves a node with no sources to the column before its nearest reader', () => {
+    const layout = layoutGraph(
+      [node('a'), node('b'), node('c'), node('late')],
+      [
+        { from: 'a', to: 'b' },
+        { from: 'b', to: 'c' },
+        { from: 'late', to: 'c' },
+      ],
+    );
+
+    expect(layout.nodes.get('late')!.layer).toBe(1);
+    expect(layout.routes.has('late>c')).toBe(false);
+  });
+
+  it('orders rows so edges do not cross', () => {
+    const layout = layoutGraph(
+      [node('a'), node('b'), node('c'), node('d')],
+      [
+        { from: 'a', to: 'd' },
+        { from: 'b', to: 'c' },
+        { from: 'a', to: 'c' },
+      ],
+    );
+
+    expect(layout.nodes.get('a')!.index).toBe(layout.nodes.get('d')!.index);
+  });
+
+  it('keeps the arrangement of a band whose structure did not change', () => {
+    const nodes = [node('a'), node('b'), node('c'), node('d')];
+    const edges = [
+      { from: 'a', to: 'd' },
+      { from: 'b', to: 'c' },
+      { from: 'a', to: 'c' },
+    ];
+    const first = layoutGraph(nodes, edges);
+    const second = layoutGraph(
+      [...nodes, node('other'), node('next')],
+      [...edges, { from: 'other', to: 'next' }],
+      first,
+    );
+
+    for (const id of ['a', 'b', 'c', 'd']) {
+      expect(second.nodes.get(id)).toEqual(first.nodes.get(id));
+    }
+    expect([...second.bands.values()]).toContain([...first.bands.values()][0]);
   });
 
   it('keeps a node in its slot when another node appears', () => {
@@ -89,15 +178,15 @@ describe('layoutGraph', () => {
 
     expect(second.nodes.get('a')!.y).toBe(first.nodes.get('a')!.y);
     expect(second.nodes.get('b')!.y).toBe(first.nodes.get('b')!.y);
-    expect(second.nodes.get('c')!.index).toBe(2);
+    expect(second.nodes.get('c')!.layer).toBe(2);
   });
 
   it('keeps the previous order when the caller passes the nodes in another order', () => {
     const first = layoutGraph([node('a'), node('b')], []);
     const second = layoutGraph([node('b'), node('a')], [], first);
 
-    expect(second.nodes.get('a')!.index).toBe(0);
-    expect(second.nodes.get('b')!.index).toBe(1);
+    expect(second.nodes.get('a')!.layer).toBe(0);
+    expect(second.nodes.get('b')!.layer).toBe(1);
   });
 
   it('does not move one column when another column grows', () => {
@@ -146,6 +235,17 @@ describe('edgePath', () => {
   });
 });
 
+describe('edgePath through waypoints', () => {
+  it('runs straight through each waypoint', () => {
+    const from = { id: 'a', layer: 0, index: 0, x: 0, y: 0 };
+    const via = [{ id: 'w', layer: 1, index: 1, x: 226, y: 58 }];
+    const to = { id: 'c', layer: 2, index: 0, x: 452, y: 0 };
+    const middle = 58 + NODE_HEIGHT / 2;
+
+    expect(edgePath(from, to, via)).toContain(`226 ${middle} L ${226 + NODE_WIDTH} ${middle}`);
+  });
+});
+
 describe('placeHoverCard', () => {
   const canvas = { width: 800, height: 600 };
   const card = { width: 288, height: 150 };