refactor(GraphEdge): Disable events on GraphEdge

src/graph-edge.ts

@@ -1,4 +1,3 @@
-import { EventDispatcher, GraphEdgeEvent } from './event-dispatcher.js';
 import { GraphNode } from './graph-node.js';
 
 /**
@@ -9,22 +8,21 @@ import { GraphNode } from './graph-node.js';
  * that link. The resource does not hold a reference to the link or to the owner,
  * although that reverse lookup can be done on the graph.
  */
-export class GraphEdge<Parent extends GraphNode, Child extends GraphNode> extends EventDispatcher<GraphEdgeEvent> {
+export class GraphEdge<Parent extends GraphNode, Child extends GraphNode> {
 	private _disposed = false;
 
 	constructor(
 		private readonly _name: string,
 		private readonly _parent: Parent,
 		private _child: Child,
-		private _attributes: Record<string, unknown> = {}
+		private _attributes: Record<string, unknown> = {},
 	) {
-		super();
 		if (!_parent.isOnGraph(_child)) {
 			throw new Error('Cannot connect disconnected graphs.');
 		}
 	}
 
-	/** Name. */
+	/** Name (attribute name from parent {@link GraphNode}). */
 	getName(): string {
 		return this._name;
 	}
@@ -58,9 +56,9 @@ export class GraphEdge<Parent extends GraphNode, Child extends GraphNode> extend
 	/** Destroys a (currently intact) edge, updating both the graph and the owner. */
 	dispose(): void {
 		if (this._disposed) return;
+		// @ts-expect-error GraphEdge doesn't know types of parent GraphNode.
+		this._parent._destroyRef(this);
 		this._disposed = true;
-		this.dispatchEvent({ type: 'dispose', target: this });
-		super.dispose();
 	}
 
 	/** Whether this link has been destroyed. */

src/graph-node.ts

@@ -22,10 +22,10 @@ type GraphNodeAttributesInternal<Parent extends GraphNode, Attributes extends ob
 	[Key in keyof Attributes]: Attributes[Key] extends GraphNode
 		? GraphEdge<Parent, Attributes[Key]>
 		: Attributes[Key] extends GraphNode[]
-		? GraphEdge<Parent, Attributes[Key][number]>[]
-		: Attributes[Key] extends { [key: string]: GraphNode }
-		? Record<string, GraphEdge<Parent, Attributes[Key][string]>>
-		: Attributes[Key];
+			? GraphEdge<Parent, Attributes[Key][number]>[]
+			: Attributes[Key] extends { [key: string]: GraphNode }
+				? Record<string, GraphEdge<Parent, Attributes[Key][string]>>
+				: Attributes[Key];
 };
 
 export const $attributes = Symbol('attributes');
@@ -100,8 +100,7 @@ export abstract class GraphNode<Attributes extends object = object> extends Even
 			// TODO(design): With Ref, RefList, and RefMap types, should users
 			// be able to pass them all here? Listeners must be added.
 			if (value instanceof GraphNode) {
-				const ref = this.graph.createEdge(key, this, value);
-				ref.addEventListener('dispose', () => value.dispose());
+				const ref = this.graph._createEdge(key, this, value);
 				this[$immutableKeys].add(key);
 				attributes[key] = ref as any;
 			} else {
@@ -223,11 +222,7 @@ export abstract class GraphNode<Attributes extends object = object> extends Even
 
 		if (!value) return this;
 
-		const ref = this.graph.createEdge(attribute as string, this, value, attributes);
-		ref.addEventListener('dispose', () => {
-			delete this[$attributes][attribute];
-			this.dispatchEvent({ type: 'change', attribute });
-		});
+		const ref = this.graph._createEdge(attribute as string, this, value, attributes);
 		(this[$attributes][attribute] as Ref) = ref;
 
 		return this.dispatchEvent({ type: 'change', attribute });
@@ -251,16 +246,10 @@ export abstract class GraphNode<Attributes extends object = object> extends Even
 		value: RefCollectionValue<Attributes[K]>,
 		attributes?: Record<string, unknown>,
 	): this {
-		const ref = this.graph.createEdge(attribute as string, this, value, attributes);
-
+		const ref = this.graph._createEdge(attribute as string, this, value, attributes);
 		const refs = this.assertRefList(attribute);
 		refs.add(ref);
 
-		ref.addEventListener('dispose', () => {
-			refs.remove(ref);
-			this.dispatchEvent({ type: 'change', attribute });
-		});
-
 		return this.dispatchEvent({ type: 'change', attribute });
 	}
 
@@ -272,11 +261,11 @@ export abstract class GraphNode<Attributes extends object = object> extends Even
 		const refs = this.assertRefList(attribute);
 
 		if (refs instanceof RefList) {
-			for (const ref of refs.removeChild(value)) {
+			for (const ref of refs.listRefsByChild(value)) {
 				ref.dispose();
 			}
 		} else {
-			const ref = refs.removeChild(value);
+			const ref = refs.getRefByChild(value);
 			if (ref) ref.dispose();
 		}
 
@@ -285,10 +274,10 @@ export abstract class GraphNode<Attributes extends object = object> extends Even
 
 	/** @hidden */
 	private assertRefList<K extends RefListKeys<Attributes> | RefSetKeys<Attributes>>(attribute: K): RefList | RefSet {
-		const list = this[$attributes][attribute];
+		const refs = this[$attributes][attribute];
 
-		if (list instanceof RefList || list instanceof RefSet) {
-			return list;
+		if (refs instanceof RefList || refs instanceof RefSet) {
+			return refs;
 		}
 
 		// TODO(v3) Remove warning.
@@ -338,11 +327,7 @@ export abstract class GraphNode<Attributes extends object = object> extends Even
 		if (!value) return this;
 
 		metadata = Object.assign(metadata || {}, { key: key });
-		const ref = this.graph.createEdge(attribute as string, this, value, { ...metadata, key });
-		ref.addEventListener('dispose', () => {
-			refMap.delete(key as string);
-			this.dispatchEvent({ type: 'change', attribute, key });
-		});
+		const ref = this.graph._createEdge(attribute as string, this, value, { ...metadata, key });
 		refMap.set(key as string, ref);
 
 		return this.dispatchEvent({ type: 'change', attribute, key });
@@ -373,4 +358,35 @@ export abstract class GraphNode<Attributes extends object = object> extends Even
 		this.graph.dispatchEvent({ ...event, target: this, type: `node:${event.type}` });
 		return this;
 	}
+
+	/**********************************************************************************************
+	 * Internal.
+	 */
+
+	/** @hidden */
+	_destroyRef<
+		K extends RefKeys<Attributes> | RefListKeys<Attributes> | RefSetKeys<Attributes> | RefMapKeys<Attributes>,
+	>(ref: GraphEdge<this, GraphNode & Attributes[K]>): void {
+		const attribute = ref.getName() as K;
+		if (this[$attributes][attribute] === ref) {
+			(this[$attributes][attribute as RefKeys<Attributes>] as Ref | null) = null;
+			// TODO(design): See _createAttributes().
+			if (this[$immutableKeys].has(attribute as string)) ref.getChild().dispose();
+		} else if (this[$attributes][attribute] instanceof RefList) {
+			(this[$attributes][attribute as RefListKeys<Attributes>] as RefList).remove(ref);
+		} else if (this[$attributes][attribute] instanceof RefSet) {
+			(this[$attributes][attribute as RefSetKeys<Attributes>] as RefSet).remove(ref);
+		} else if (this[$attributes][attribute] instanceof RefMap) {
+			const refMap = this[$attributes][attribute as RefMapKeys<Attributes>] as RefMap;
+			for (const key of refMap.keys()) {
+				if (refMap.get(key) === ref) {
+					refMap.delete(key);
+				}
+			}
+		} else {
+			return;
+		}
+		this.graph._destroyEdge(ref);
+		this.dispatchEvent({ type: 'change', attribute });
+	}
 }

src/graph.ts

@@ -47,35 +47,33 @@ export class Graph<T extends GraphNode> extends EventDispatcher<GraphEvent | Gra
 	}
 
 	public disconnectParents(node: T, filter?: (n: T) => boolean): this {
-		let edges = this.listParentEdges(node);
-		if (filter) {
-			edges = edges.filter((edge) => filter(edge.getParent()));
+		for (const edge of this.listParentEdges(node)) {
+			if (!filter || filter(edge.getParent())) {
+				edge.dispose();
+			}
 		}
-		edges.forEach((edge) => edge.dispose());
 		return this;
 	}
 
+	/**********************************************************************************************
+	 * Internal.
+	 */
+
 	/**
 	 * Creates a {@link GraphEdge} connecting two {@link GraphNode} instances. Edge is returned
 	 * for the caller to store.
 	 * @param a Owner
 	 * @param b Resource
+	 * @hidden
+	 * @internal
 	 */
-	public createEdge<A extends T, B extends T>(
+	public _createEdge<A extends T, B extends T>(
 		name: string,
 		a: A,
 		b: B,
 		attributes?: Record<string, unknown>,
 	): GraphEdge<A, B> {
-		return this._registerEdge(new GraphEdge(name, a, b, attributes)) as GraphEdge<A, B>;
-	}
-
-	/**********************************************************************************************
-	 * Internal.
-	 */
-
-	/** @hidden */
-	private _registerEdge(edge: GraphEdge<T, T>): GraphEdge<T, T> {
+		const edge = new GraphEdge(name, a, b, attributes);
 		this._edges.add(edge);
 
 		const parent = edge.getParent();
@@ -86,16 +84,17 @@ export class Graph<T extends GraphNode> extends EventDispatcher<GraphEvent | Gra
 		if (!this._childEdges.has(child)) this._childEdges.set(child, new Set());
 		this._childEdges.get(child)!.add(edge);
 
-		edge.addEventListener('dispose', () => this._removeEdge(edge));
 		return edge;
 	}
 
 	/**
-	 * Removes the {@link GraphEdge} from the {@link Graph}. This method should only
-	 * be invoked by the onDispose() listener created in {@link _registerEdge()}. The
-	 * public method of removing an edge is {@link GraphEdge.dispose}.
+	 * Detaches a {@link GraphEdge} from the {@link Graph}. Before calling this
+	 * method, ensure that the GraphEdge has first been detached from any
+	 * associated {@link GraphNode} attributes.
+	 * @hidden
+	 * @internal
 	 */
-	private _removeEdge(edge: GraphEdge<T, T>): this {
+	public _destroyEdge(edge: GraphEdge<T, T>): this {
 		this._edges.delete(edge);
 		this._parentEdges.get(edge.getParent())!.delete(edge);
 		this._childEdges.get(edge.getChild())!.delete(edge);