diff --git a/package-lock.json b/package-lock.json index d429ebc..227898c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -16,7 +16,6 @@ "debounce": "^2.2.0", "es-toolkit": "^1.52.0", "eventemitter3": "^5.0.4", - "memoize-one": "^6.0.0", "mnemonist": "^0.40.3", "nullthrows": "^1.1.1", "react": "^19.1.1", @@ -2290,12 +2289,6 @@ "integrity": "sha512-dn6wd0uw5GsdswPFfsgMp5NSB0/aDe6fK94YJV/AJDYXL6HVLWBsxeq7js7Ad+mU2K9LAlwpk6kN2D5mwCPVow==", "license": "CC0-1.0" }, - "node_modules/memoize-one": { - "version": "6.0.0", - "resolved": "https://registry.npmjs.org/memoize-one/-/memoize-one-6.0.0.tgz", - "integrity": "sha512-rkpe71W0N0c0Xz6QD0eJETuWAJGnJ9afsl1srmwPrI+yBCkge5EycXXbYRyvL29zZVUWQCY7InPRCv3GDXuZNw==", - "license": "MIT" - }, "node_modules/mnemonist": { "version": "0.40.3", "resolved": "https://registry.npmjs.org/mnemonist/-/mnemonist-0.40.3.tgz", diff --git a/package.json b/package.json index 5d93aa6..d2c1def 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,6 @@ "debounce": "^2.2.0", "es-toolkit": "^1.52.0", "eventemitter3": "^5.0.4", - "memoize-one": "^6.0.0", "mnemonist": "^0.40.3", "nullthrows": "^1.1.1", "react": "^19.1.1", diff --git a/src/pages/edit/Editor/components/ContextMenu.tsx b/src/pages/edit/Editor/components/ContextMenu.tsx index cc0fbba..f22a59e 100644 --- a/src/pages/edit/Editor/components/ContextMenu.tsx +++ b/src/pages/edit/Editor/components/ContextMenu.tsx @@ -35,6 +35,14 @@ export default function CCComponentEditorContextMenu({ if (!componentEditorState.contextMenuState) return null; + // The selection can still hold objects removed since, e.g. along with a node pin + const selectedNodeIds = [...componentEditorState.selectedNodeIds].filter( + (nodeId) => store.nodes.get(nodeId), + ); + const selectedConnectionIds = [ + ...componentEditorState.selectedConnectionIds, + ].filter((connectionId) => store.connections.get(connectionId)); + return ( Create a node */} - {componentEditorState.selectedNodeIds.size > 0 && ( + {selectedNodeIds.length > 0 && ( { - const oldNodes = [...componentEditorState.selectedNodeIds].map( - (nodeId) => { - const node = store.nodes.get(nodeId); - invariant(node); - return node; - }, + const oldNodes = selectedNodeIds.map((nodeId) => + nullthrows(store.nodes.get(nodeId)), + ); + const oldConnections = selectedConnectionIds.map((connectionId) => + nullthrows(store.connections.get(connectionId)), ); - const oldConnections = [ - ...componentEditorState.selectedConnectionIds, - ].map((connectionId) => { - const connection = store.connections.get(connectionId); - invariant(connection); - return connection; - }); const newComponent = CCComponentStore.create({ name: "New Component", }); @@ -116,10 +116,8 @@ export default function CCComponentEditorContextMenu({ store.connections.register(connection, { allowBitWidthConflict: true, }); - store.connections.unregister([ - ...componentEditorState.selectedConnectionIds, - ]); - store.nodes.unregister([...componentEditorState.selectedNodeIds]); + store.connections.unregister(selectedConnectionIds); + store.nodes.unregister(selectedNodeIds); componentEditorState.closeContextMenu(); onEditComponent(newComponent.id); }} @@ -127,20 +125,14 @@ export default function CCComponentEditorContextMenu({ Create a new component... )} - {(componentEditorState.selectedNodeIds.size > 0 || - componentEditorState.selectedConnectionIds.size > 0) && ( + {(selectedNodeIds.length > 0 || selectedConnectionIds.length > 0) && ( { - if (componentEditorState.selectedNodeIds.size > 0) - store.nodes.unregister([ - ...componentEditorState.selectedNodeIds, - ]); - if (componentEditorState.selectedConnectionIds.size > 0) - store.connections.unregister([ - ...componentEditorState.selectedConnectionIds, - ]); - // componentEditorState.selectNode([], true); - componentEditorState.selectConnection([], false); + if (selectedNodeIds.length > 0) + store.nodes.unregister(selectedNodeIds); + if (selectedConnectionIds.length > 0) + store.connections.unregister(selectedConnectionIds); + componentEditorState.selectConnection([], true); componentEditorState.closeContextMenu(); }} > @@ -148,13 +140,9 @@ export default function CCComponentEditorContextMenu({ )} {(() => { - if (componentEditorState.selectedNodeIds.size !== 1) return undefined; - const iteratorResult = componentEditorState.selectedNodeIds - .values() - .next(); - invariant(!iteratorResult.done); - const targetNode = store.nodes.get(iteratorResult.value); - invariant(targetNode); + const [targetNodeId] = selectedNodeIds; + if (selectedNodeIds.length !== 1 || !targetNodeId) return undefined; + const targetNode = nullthrows(store.nodes.get(targetNodeId)); const targetComponent = store.components.get(targetNode.componentId); invariant(targetComponent); if (targetComponent.intrinsicType) return undefined; diff --git a/src/pages/edit/Editor/renderer/Connection/index.tsx b/src/pages/edit/Editor/renderer/Connection/index.tsx index 4ff2b66..4b1c6e0 100644 --- a/src/pages/edit/Editor/renderer/Connection/index.tsx +++ b/src/pages/edit/Editor/renderer/Connection/index.tsx @@ -4,7 +4,6 @@ import type { Vector2 } from "../../../../../common/vector2"; import type { CCComponentPinType } from "../../../../../store/componentPin"; import type { CCConnectionId } from "../../../../../store/connection"; import { useStore } from "../../../../../store/react"; -import ensureStoreItem from "../../../../../store/react/error"; import { useIsConnectionInBitWidthConflict, useNode, @@ -85,7 +84,9 @@ export function CCComponentEditorRendererConnectionCore({ } e.preventDefault(); e.stopPropagation(); - componentEditorState.selectConnection([connectionId], true); + if (!componentEditorState.selectedConnectionIds.has(connectionId)) { + componentEditorState.selectConnection([connectionId], true); + } componentEditorState.openContextMenu(e); }} id={connectionId} @@ -98,73 +99,71 @@ export function CCComponentEditorRendererConnectionCore({ export type CCComponentEditorRendererConnectionProps = { connectionId: CCConnectionId; }; -const CCComponentEditorRendererConnection = ensureStoreItem( - (props, store) => store.connections.get(props.connectionId), - ({ connectionId }: CCComponentEditorRendererConnectionProps) => { - const { store } = useStore(); - const componentEditorState = useComponentEditorStore()(); - const connection = nullthrows(store.connections.get(connectionId)); - const fromNodePin = nullthrows(store.nodePins.get(connection.from)); - const fromComponentPin = nullthrows( - store.componentPins.get(fromNodePin.componentPinId), - ); - const toNodePin = nullthrows(store.nodePins.get(connection.to)); - const toComponentPin = nullthrows( - store.componentPins.get(toNodePin.componentPinId), - ); - const fromNode = useNode(fromNodePin.nodeId); - const toNode = useNode(toNodePin.nodeId); - const fromNodeGeometry = getCCComponentEditorRendererNodeGeometry( +export default function CCComponentEditorRendererConnection({ + connectionId, +}: CCComponentEditorRendererConnectionProps) { + const { store } = useStore(); + const componentEditorState = useComponentEditorStore()(); + const connection = store.connections.get(connectionId); + const fromNodePin = connection && store.nodePins.get(connection.from); + const toNodePin = connection && store.nodePins.get(connection.to); + const fromNode = useNode(fromNodePin?.nodeId); + const toNode = useNode(toNodePin?.nodeId); + const isInBitWidthConflict = useIsConnectionInBitWidthConflict(connectionId); + const [isHovered, setIsHovered] = useState(false); + + if (!connection || !fromNodePin || !toNodePin || !fromNode || !toNode) { + return null; + } + const fromComponentPin = nullthrows( + store.componentPins.get(fromNodePin.componentPinId), + ); + const toComponentPin = nullthrows( + store.componentPins.get(toNodePin.componentPinId), + ); + const fromNodePinPosition = nullthrows( + getCCComponentEditorRendererNodeGeometry( store, fromNode.id, - ); - const toNodeGeometry = getCCComponentEditorRendererNodeGeometry( + ).nodePinPositionById.get(fromNodePin.id), + ); + const toNodePinPosition = nullthrows( + getCCComponentEditorRendererNodeGeometry( store, toNode.id, - ); - const fromNodePinPosition = nullthrows( - fromNodeGeometry.nodePinPositionById.get(fromNodePin.id), - ); - const toNodePinPosition = nullthrows( - toNodeGeometry.nodePinPositionById.get(toNodePin.id), - ); - - const [isHovered, setIsHovered] = useState(false); - const isInBitWidthConflict = - useIsConnectionInBitWidthConflict(connectionId); + ).nodePinPositionById.get(toNodePin.id), + ); - return ( - <> - { - setIsHovered(true); - }} - onMouseLeave={() => { - setIsHovered(false); - }} - /> - {isHovered && componentEditorState.editorMode === "play" && ( - - - {stringifySimulationValue( - nullthrows(componentEditorState.getNodePinValue(toNodePin.id)), - )} - - - )} - - ); - }, -); -export default CCComponentEditorRendererConnection; + return ( + <> + { + setIsHovered(true); + }} + onMouseLeave={() => { + setIsHovered(false); + }} + /> + {isHovered && componentEditorState.editorMode === "play" && ( + + + {stringifySimulationValue( + nullthrows(componentEditorState.getNodePinValue(toNodePin.id)), + )} + + + )} + + ); +} diff --git a/src/pages/edit/Editor/renderer/Node/components/Interface/index.tsx b/src/pages/edit/Editor/renderer/Node/components/Interface/index.tsx index 0c718b1..303bf22 100644 --- a/src/pages/edit/Editor/renderer/Node/components/Interface/index.tsx +++ b/src/pages/edit/Editor/renderer/Node/components/Interface/index.tsx @@ -17,13 +17,11 @@ export function CCComponentEditorRendererNodeInterfaceRenderer( const { store } = useStore(); const editorState = useComponentEditorStore()(); - const nodePin = nullthrows( - store.nodePins.getManyByNodeId(props.node.id)[0], - `Interface node ${props.node.id} is missing its pin`, - ); + const nodePin = store.nodePins.getManyByNodeId(props.node.id)[0]; const componentPin = useComponentPin( - nullthrows(store.componentPins.getByImplementation(nodePin.id)).id, + nodePin && store.componentPins.getByImplementation(nodePin.id)?.id, ); + if (!nodePin || !componentPin) return null; const isEditMode = editorState.editorMode === "edit"; const simulation = isEditMode diff --git a/src/pages/edit/Editor/renderer/Node/index.tsx b/src/pages/edit/Editor/renderer/Node/index.tsx index 4c95b42..3c79012 100644 --- a/src/pages/edit/Editor/renderer/Node/index.tsx +++ b/src/pages/edit/Editor/renderer/Node/index.tsx @@ -6,7 +6,6 @@ import { } from "../../../../../store/intrinsics/types"; import type { CCNodeId } from "../../../../../store/node"; import { useStore } from "../../../../../store/react"; -import ensureStoreItem from "../../../../../store/react/error"; import { useComponent, useNode } from "../../../../../store/react/selectors"; import { useComponentEditorStore } from "../../store"; import CCComponentEditorRendererNodePin from "../NodePin"; @@ -39,98 +38,95 @@ const specialRenderers: Partial< export type CCComponentEditorRendererNodeProps = { nodeId: CCNodeId; }; -const CCComponentEditorRendererNode = ensureStoreItem( - (props, store) => store.nodes.get(props.nodeId), - ({ nodeId }: CCComponentEditorRendererNodeProps) => { - const { store } = useStore(); - const node = useNode(nodeId); - const component = useComponent(node.componentId); - const componentEditorState = useComponentEditorStore()(); - const [dragging, setDragging] = useState(false); - const [dragStartPosition, setDragStartPosition] = useState(vector2.zero); - const [previousNodePosition, setPreviousNodePosition] = useState( - vector2.zero, - ); +export default function CCComponentEditorRendererNode({ + nodeId, +}: CCComponentEditorRendererNodeProps) { + const { store } = useStore(); + const node = useNode(nodeId); + const component = useComponent(node?.componentId); + const componentEditorState = useComponentEditorStore()(); + const [dragging, setDragging] = useState(false); + const [dragStartPosition, setDragStartPosition] = useState(vector2.zero); + const [previousNodePosition, setPreviousNodePosition] = useState( + vector2.zero, + ); - const layout = getCCComponentEditorRendererNodeLayout(store, nodeId); - const geometry = ccComponentEditorRendererLayoutToGeometry( - layout, - node.position, - ); - const Renderer = - (component.intrinsicType && specialRenderers[component.intrinsicType]) || - CCComponentEditorRendererNodeDefaultRenderer; - const isSelected = componentEditorState.selectedNodeIds.has(nodeId); + if (!node || !component) return null; - const handlePointerDown = (e: React.PointerEvent) => { - if (e.button === 0) { - componentEditorState.selectNode([nodeId], !e.shiftKey); - } - setDragStartPosition(vector2.fromDomEvent(e.nativeEvent)); - setPreviousNodePosition(node.position); - setDragging(true); - e.currentTarget.setPointerCapture(e.pointerId); - }; + const layout = getCCComponentEditorRendererNodeLayout(store, nodeId); + const geometry = ccComponentEditorRendererLayoutToGeometry( + layout, + node.position, + ); + const Renderer = + (component.intrinsicType && specialRenderers[component.intrinsicType]) || + CCComponentEditorRendererNodeDefaultRenderer; + const isSelected = componentEditorState.selectedNodeIds.has(nodeId); - const handlePointerMove = (e: React.PointerEvent) => { - if (dragging) { - store.nodes.update(nodeId, { - position: vector2.add( - previousNodePosition, - vector2.mul( - vector2.sub( - vector2.fromDomEvent(e.nativeEvent), - dragStartPosition, - ), - componentEditorState.perspective.scale, - ), + const handlePointerDown = (e: React.PointerEvent) => { + if (e.button === 0) { + componentEditorState.selectNode([nodeId], !e.shiftKey); + } + setDragStartPosition(vector2.fromDomEvent(e.nativeEvent)); + setPreviousNodePosition(node.position); + setDragging(true); + e.currentTarget.setPointerCapture(e.pointerId); + }; + + const handlePointerMove = (e: React.PointerEvent) => { + if (dragging) { + store.nodes.update(nodeId, { + position: vector2.add( + previousNodePosition, + vector2.mul( + vector2.sub(vector2.fromDomEvent(e.nativeEvent), dragStartPosition), + componentEditorState.perspective.scale, ), - }); - } - }; + ), + }); + } + }; - const handlePointerUp = (e: React.PointerEvent) => { - setDragging(false); - e.currentTarget.releasePointerCapture(e.pointerId); - }; + const handlePointerUp = (e: React.PointerEvent) => { + setDragging(false); + e.currentTarget.releasePointerCapture(e.pointerId); + }; - return ( - <> - {/** biome-ignore lint/a11y/noSvgWithoutTitle: SVG */} - { - e.preventDefault(); - const selectedNodeIds = componentEditorState.selectedNodeIds; - if (!selectedNodeIds.has(nodeId)) { - componentEditorState.selectNode([nodeId], true); - } - componentEditorState.openContextMenu(e); - }} - > - - - {store.nodePins.getManyByNodeId(nodeId).map((nodePin) => ( - - ))} - - ); - }, -); -export default CCComponentEditorRendererNode; + return ( + <> + {/** biome-ignore lint/a11y/noSvgWithoutTitle: SVG */} + { + e.preventDefault(); + const selectedNodeIds = componentEditorState.selectedNodeIds; + if (!selectedNodeIds.has(nodeId)) { + componentEditorState.selectNode([nodeId], true); + } + componentEditorState.openContextMenu(e); + }} + > + + + {store.nodePins.getManyByNodeId(nodeId).map((nodePin) => ( + + ))} + + ); +} diff --git a/src/store/base.ts b/src/store/base.ts index d7b41ba..91edba9 100644 --- a/src/store/base.ts +++ b/src/store/base.ts @@ -1,6 +1,7 @@ import EventEmitter from "eventemitter3"; import type CCStore from "."; import type { + CCObject, CCObjectByType, CCObjectId, CCObjectStore, @@ -25,7 +26,11 @@ export abstract class CCObjectStoreBase< { protected readonly store: CCStore; - protected readonly objects: Map, CCObjectByType[K]> = new Map(); + readonly #objects: Map, CCObjectByType[K]> = new Map(); + + protected get objects(): ReadonlyMap, CCObjectByType[K]> { + return this.#objects; + } /** * Fields of the objects holding the id of another object, mapped to the object type of that id. @@ -33,6 +38,19 @@ export abstract class CCObjectStoreBase< */ abstract readonly references: CCReferences; + protected setObject( + object: CCObjectByType[K] & CCObject, + { changesCircuit = true }: { changesCircuit?: boolean } = {}, + ): void { + this.#objects.set(object.id, object); + if (changesCircuit) this.store.countCircuitChange(); + } + + protected deleteObject(id: CCObjectId): void { + this.#objects.delete(id); + this.store.countCircuitChange(); + } + constructor(store: CCStore) { super(); this.store = store; diff --git a/src/store/bitWidth.test.ts b/src/store/bitWidth.test.ts index aa94d2b..b3c25d7 100644 --- a/src/store/bitWidth.test.ts +++ b/src/store/bitWidth.test.ts @@ -331,6 +331,42 @@ describe("bit width groups", () => { expect(conflictingConnectionCount(store)).toBe(0); }); + it("drops a removed connection from the bit widths before announcing its removal", () => { + const store = createTestStore(); + const root = registerTestComponent(store, "Root"); + const constant = registerConst(store, root.id, 8); + const notNode = registerTestNode(store, root.id, not.component.id); + const screen = registerDisplay(store, root.id, 4, 2); + connectTestNodePins( + store, + root.id, + constant.out, + testNodePin(store, notNode, not.inputPin.In), + ); + connectTestNodePins( + store, + root.id, + testNodePin(store, notNode, not.outputPin.Out), + screen.pixels, + ); + store.nodes.update(screen.node.id, { + config: { resolution: { x: 2, y: 2 }, pixelOrder: "row-major" }, + }); + expect(conflictingConnectionCount(store)).toBe(2); + + const [toScreen] = store.connections.getConnectionsByNodePinId( + screen.pixels, + ); + void store.connections.unregister([nullthrows(toScreen).id]); + + expect(conflictingConnectionCount(store)).toBe(0); + expect( + store.nodePins.getNodePinBitWidthStatus( + testNodePin(store, notNode, not.outputPin.Out), + ), + ).toEqual({ isFixed: true, bitWidth: 8 }); + }); + it("refuses a new connection that would tie conflicting widths together", () => { const store = createTestStore(); const inverter = registerInverterComponent(store); diff --git a/src/store/component.ts b/src/store/component.ts index 58ec852..32af888 100644 --- a/src/store/component.ts +++ b/src/store/component.ts @@ -43,7 +43,7 @@ export class CCComponentStore extends CCObjectStoreBase< */ register(component: CCComponent): void { invariant(!this.objects.has(component.id)); - this.objects.set(component.id, component); + this.setObject(component); this.emit("didRegister", component); } @@ -56,7 +56,7 @@ export class CCComponentStore extends CCObjectStoreBase< if (!component) throw new Error(`Component ${id} not found`); await this.store.transactionManager.runInTransaction(() => { this.emit("willUnregister", component); - this.objects.delete(id); + this.deleteObject(id); }); this.emit("didUnregister", component); } @@ -69,8 +69,9 @@ export class CCComponentStore extends CCObjectStoreBase< update(id: CCComponentId, value: Pick): void { const component = this.objects.get(id); invariant(component); - this.objects.set(id, { ...component, ...value }); - this.emit("didUpdate", component); + const newComponent = { ...component, ...value }; + this.setObject(newComponent); + this.emit("didUpdate", newComponent); } /** diff --git a/src/store/componentPin.ts b/src/store/componentPin.ts index 4845078..42203d7 100644 --- a/src/store/componentPin.ts +++ b/src/store/componentPin.ts @@ -64,7 +64,7 @@ export class CCComponentPinStore extends CCObjectStoreBase< import(componentPins: CCComponentPin[]): void { for (const pin of componentPins) { - this.objects.set(pin.id, pin); + this.setObject(pin); } } @@ -90,7 +90,7 @@ export class CCComponentPinStore extends CCObjectStoreBase< */ register(pin: CCComponentPin): void { invariant(this.store.components.get(pin.componentId)); - this.objects.set(pin.id, pin); + this.setObject(pin); this.emit("didRegister", pin); } @@ -124,7 +124,7 @@ export class CCComponentPinStore extends CCObjectStoreBase< const pin = nullthrows(this.objects.get(id)); await this.store.transactionManager.runInTransaction(() => { this.emit("willUnregister", pin); - this.objects.delete(id); + this.deleteObject(id); }); this.emit("didUnregister", pin); } @@ -171,8 +171,9 @@ export class CCComponentPinStore extends CCObjectStoreBase< ): void { const pin = this.objects.get(id); invariant(pin); - this.objects.set(id, { ...pin, ...value }); - this.emit("didUpdate", pin); + const newPin = { ...pin, ...value }; + this.setObject(newPin); + this.emit("didUpdate", newPin); } /** diff --git a/src/store/connection.test.ts b/src/store/connection.test.ts new file mode 100644 index 0000000..6e0d70a --- /dev/null +++ b/src/store/connection.test.ts @@ -0,0 +1,37 @@ +import nullthrows from "nullthrows"; +import { describe, expect, it } from "vitest"; +import { not, true_ } from "./intrinsics/definitions"; +import { + connectTestNodePins, + createTestStore, + registerTestComponent, + registerTestNode, + testNodePin, +} from "./testUtils"; + +describe("Connection store", () => { + it("announces the removal of a connection when its transaction ends, like the other stores", async () => { + const store = createTestStore(); + const root = registerTestComponent(store, "Root"); + const source = registerTestNode(store, root.id, true_.component.id); + const sink = registerTestNode(store, root.id, not.component.id); + connectTestNodePins( + store, + root.id, + testNodePin(store, source, true_.outputPin.Out), + testNodePin(store, sink, not.inputPin.In), + ); + const connection = nullthrows(store.connections.getMany()[0]); + const announced: string[] = []; + store.connections.on("didUnregister", (removed) => + announced.push(removed.id), + ); + + const removal = store.connections.unregister([connection.id]); + + expect(store.connections.get(connection.id)).toBeUndefined(); + expect(announced).toEqual([]); + await removal; + expect(announced).toEqual([connection.id]); + }); +}); diff --git a/src/store/connection.ts b/src/store/connection.ts index 2c65844..4b124ac 100644 --- a/src/store/connection.ts +++ b/src/store/connection.ts @@ -40,7 +40,7 @@ export class CCConnectionStore extends CCObjectStoreBase< import(connections: CCConnection[]): void { for (const connection of connections) { - this.objects.set(connection.id, connection); + this.setObject(connection); } } @@ -72,7 +72,7 @@ export class CCConnectionStore extends CCObjectStoreBase< ) { return; } - this.objects.set(connection.id, connection); + this.setObject(connection); this.emit("didRegister", connection); } @@ -85,17 +85,12 @@ export class CCConnectionStore extends CCObjectStoreBase< await this.store.transactionManager.runInTransaction(() => { for (const connection of connections) { this.emit("willUnregister", connection); - this.objects.delete(connection.id); - } - // Announced as soon as the connections are gone, not when the transaction ends. - // Otherwise a listener of a change announced in between, such as a renderer - // subscribed to the node at one end, would look up a connection that is gone - // while the list of connections still has it. Removing a connection cascades - // to nothing, so its listeners see no half-removed state. - for (const connection of connections) { - this.emit("didUnregister", connection); + this.deleteObject(connection.id); } }); + for (const connection of connections) { + this.emit("didUnregister", connection); + } } /** diff --git a/src/store/index.ts b/src/store/index.ts index 998a30b..18a73db 100644 --- a/src/store/index.ts +++ b/src/store/index.ts @@ -43,11 +43,18 @@ export default class CCStore implements CCObjectStores { * How many circuit changes the store has seen. Anything derived from the circuit, such * as a simulation netlist, can be cached against this number and rebuilt once it * changes. Moving a node does not change the circuit, so it does not count. + * + * A change is counted when it is made, not when it is announced, since a removal is + * announced only when its transaction ends. */ get revision(): number { return this.#revision; } + countCircuitChange(): void { + this.#revision += 1; + } + /** * Constructor of CCStore * @param rootComponent root component @@ -68,16 +75,12 @@ export default class CCStore implements CCObjectStores { this.componentPins.register(pin); } } - - this.subscribeToCircuitChanges(() => { - this.#revision += 1; - }); } /** - * Listen to every change of the circuit, i.e. of what the store {@link revision} - * counts. Anything derived from more than one object, such as a bit width that a - * change in another component can alter, has to be recomputed on any of them. + * Listen to the announcements of the changes of the circuit. Anything derived from + * more than one object, such as a bit width that a change in another component can + * alter, has to be recomputed on any of them. * @param listener called synchronously on each change * @returns a function to stop listening */ @@ -143,8 +146,7 @@ export default class CCStore implements CCObjectStores { /** * Restore the objects of a store saved with {@link toJSON}. Some stores add them - * without announcing them, so this must run before {@link mount}, while nothing - * derived from them (such as the bit widths cached by the node pin store) exists yet. + * without announcing them, so this must run before {@link mount}. */ importJson(json: string) { const { components, nodes, componentPins, nodePins, connections } = diff --git a/src/store/node.ts b/src/store/node.ts index 17bccbe..84873c9 100644 --- a/src/store/node.ts +++ b/src/store/node.ts @@ -71,7 +71,7 @@ export class CCNodeStore extends CCObjectStoreBase<"nodes", CCNodeStoreEvents> { register(node: CCNode): void { invariant(this.store.components.get(node.componentId)); invariant(this.store.components.get(node.parentComponentId)); - this.objects.set(node.id, node); + this.setObject(node); this.emit("didRegister", node); } @@ -84,7 +84,7 @@ export class CCNodeStore extends CCObjectStoreBase<"nodes", CCNodeStoreEvents> { await this.store.transactionManager.runInTransaction(() => { for (const node of nodes) { this.emit("willUnregister", node); - this.objects.delete(node.id); + this.deleteObject(node.id); } }); for (const node of nodes) { @@ -115,8 +115,10 @@ export class CCNodeStore extends CCObjectStoreBase<"nodes", CCNodeStoreEvents> { ): void { const existingNode = nullthrows(this.objects.get(id)); const newNode = { ...existingNode, ...value }; - this.objects.set(id, newNode); - if ("config" in value && value.config !== existingNode.config) { + const changesConfig = + "config" in value && value.config !== existingNode.config; + this.setObject(newNode, { changesCircuit: changesConfig }); + if (changesConfig) { this.emit("didUpdateConfig", newNode); } this.emit("didUpdate", newNode); diff --git a/src/store/nodePin.ts b/src/store/nodePin.ts index c03f205..d1384c7 100644 --- a/src/store/nodePin.ts +++ b/src/store/nodePin.ts @@ -36,17 +36,13 @@ export class CCNodePinStore extends CCObjectStoreBase< componentPinId: "componentPins", } as const; - #markedAsDeleted: Set = new Set(); - #bitWidthsByComponent: Map = new Map(); + #bitWidthsRevision = -1; + /** components whose bit widths are being analyzed, to catch one containing itself */ #componentsBeingAnalyzed: Set = new Set(); - #clearBitWidthCache(): void { - this.#bitWidthsByComponent.clear(); - } - import(nodePins: CCNodePin[]) { for (const nodePin of nodePins) { this.register(nodePin); @@ -54,12 +50,6 @@ export class CCNodePinStore extends CCObjectStoreBase< } mount() { - this.store.connections.on("didRegister", () => this.#clearBitWidthCache()); - this.store.connections.on("didUnregister", () => - this.#clearBitWidthCache(), - ); - // A calculated bit width may be derived from the config of its node - this.store.nodes.on("didUpdateConfig", () => this.#clearBitWidthCache()); this.store.nodes.on("didRegister", (node) => { const componentPins = this.store.componentPins.getManyByComponentId( node.componentId, @@ -110,8 +100,7 @@ export class CCNodePinStore extends CCObjectStoreBase< register(nodePin: CCNodePin): void { invariant(this.store.componentPins.get(nodePin.componentPinId)); invariant(this.store.nodes.get(nodePin.nodeId)); - this.objects.set(nodePin.id, nodePin); - this.#clearBitWidthCache(); + this.setObject(nodePin); this.emit("didRegister", nodePin); } @@ -121,23 +110,17 @@ export class CCNodePinStore extends CCObjectStoreBase< */ async unregister(id: CCNodePinId): Promise { const nodePin = nullthrows(this.objects.get(id)); - this.#markedAsDeleted.add(id); await this.store.transactionManager.runInTransaction(() => { this.emit("willUnregister", nodePin); - this.objects.delete(nodePin.id); - // Its removal is announced only when the transaction ends, but the bit widths - // derived from it are stale from now on - this.#clearBitWidthCache(); + this.deleteObject(nodePin.id); }); this.emit("didUnregister", nodePin); - this.#markedAsDeleted.delete(id); } update(id: CCNodePinId, value: Pick) { const existingNodePin = nullthrows(this.objects.get(id)); const newNodePin = { ...existingNodePin, ...value }; - this.objects.set(id, newNodePin); - this.#clearBitWidthCache(); + this.setObject(newNodePin); this.emit("didUpdate", newNodePin); } @@ -178,10 +161,14 @@ export class CCNodePinStore extends CCObjectStoreBase< /** * The bit width groups of the pins of the nodes inside a user-defined component. - * They are cached until the circuit changes. + * They are cached until the revision of the store changes. * @param componentId a user-defined component */ getComponentBitWidths(componentId: CCComponentId): ComponentBitWidths { + if (this.#bitWidthsRevision !== this.store.revision) { + this.#bitWidthsByComponent.clear(); + this.#bitWidthsRevision = this.store.revision; + } const cached = this.#bitWidthsByComponent.get(componentId); if (cached) return cached; invariant( @@ -226,10 +213,6 @@ export class CCNodePinStore extends CCObjectStoreBase< return this.#getBitWidthsAround(nodePinId).isInConflict(nodePinId); } - isMarkedAsDeleted(id: CCNodePinId) { - return this.#markedAsDeleted.has(id); - } - /** * Check whether two node pins can be connected. * diff --git a/src/store/react/error.tsx b/src/store/react/error.tsx deleted file mode 100644 index 555a62a..0000000 --- a/src/store/react/error.tsx +++ /dev/null @@ -1,14 +0,0 @@ -import { type ComponentType, createElement } from "react"; -import type CCStore from ".."; -import { useStore } from "."; - -export default function ensureStoreItem

>( - check: (props: P, store: CCStore) => unknown, - component: ComponentType

, -): ComponentType

{ - return function EnsureStoreItem(props: P) { - const { store } = useStore(); - if (!check(props, store)) return null; - return createElement(component, props); - }; -} diff --git a/src/store/react/selectors.ts b/src/store/react/selectors.ts index fcfd27a..c605821 100644 --- a/src/store/react/selectors.ts +++ b/src/store/react/selectors.ts @@ -1,6 +1,4 @@ -import memoizeOne from "memoize-one"; -import nullthrows from "nullthrows"; -import { useCallback, useMemo, useSyncExternalStore } from "react"; +import { useCallback, useRef, useSyncExternalStore } from "react"; import { type CCComponent, type CCComponentId, @@ -9,122 +7,114 @@ import { import type { CCComponentPin, CCComponentPinId } from "../componentPin"; import type { CCConnectionId } from "../connection"; import type { CCNode, CCNodeId } from "../node"; -import type { CCNodePin } from "../nodePin"; import { useStore } from "."; +import { reuseIfSameItems } from "./snapshot"; + +/** Read on every render, since a removal is announced only after its transaction. */ +function useStoreList( + read: () => readonly T[], + subscribe: (onStoreChange: () => void) => () => void, +): readonly T[] { + const previousRef = useRef(null); + const getSnapshot = useCallback(() => { + const list = reuseIfSameItems(previousRef.current, read()); + previousRef.current = list; + return list; + }, [read]); + return useSyncExternalStore(subscribe, getSnapshot); +} export function useComponents() { const { store } = useStore(); - const getSnapshot = useMemo( - () => memoizeOne(() => store.components.getMany()), - [store], - ); + const read = useCallback(() => store.components.getMany(), [store]); const subscribe = useCallback( (onStoreChange: () => void) => { - const handler = () => { - getSnapshot.clear(); - onStoreChange(); - }; - store.components.on("didRegister", handler); - store.components.on("didUpdate", handler); - store.components.on("didUnregister", handler); + store.components.on("didRegister", onStoreChange); + store.components.on("didUpdate", onStoreChange); + store.components.on("didUnregister", onStoreChange); return () => { - store.components.off("didRegister", handler); - store.components.off("didUpdate", handler); - store.components.off("didUnregister", handler); + store.components.off("didRegister", onStoreChange); + store.components.off("didUpdate", onStoreChange); + store.components.off("didUnregister", onStoreChange); }; }, - [getSnapshot, store.components], + [store], ); - return useSyncExternalStore(subscribe, getSnapshot); + return useStoreList(read, subscribe); } -export function useComponent(componentId: CCComponentId) { +export function useNodeIds(parentComponentId: CCComponentId) { const { store } = useStore(); - const getSnapshot = useCallback( - () => nullthrows(store.components.get(componentId)), - [store, componentId], + const read = useCallback( + () => + store.nodes + .getManyByParentComponentId(parentComponentId) + .map((node) => node.id), + [store, parentComponentId], ); const subscribe = useCallback( (onStoreChange: () => void) => { - const handler = (component: CCComponent) => { - if (component.id === componentId) onStoreChange(); - }; - store.components.on("didUpdate", handler); + store.nodes.on("didRegister", onStoreChange); + store.nodes.on("didUnregister", onStoreChange); return () => { - store.components.off("didUpdate", handler); + store.nodes.off("didRegister", onStoreChange); + store.nodes.off("didUnregister", onStoreChange); }; }, - [store, componentId], + [store], ); - return useSyncExternalStore(subscribe, getSnapshot); + return useStoreList(read, subscribe); } -export function useNodeIds(parentComponentId: CCComponentId) { +export function useConnectionIds(parentComponentId: CCComponentId) { const { store } = useStore(); - const getSnapshot = useMemo( + const read = useCallback( () => - memoizeOne(() => - store.nodes - .getMany() - .filter((node) => node.parentComponentId === parentComponentId) - .map((node) => node.id), - ), + store.connections + .getManyByParentComponentId(parentComponentId) + .map((connection) => connection.id), [store, parentComponentId], ); const subscribe = useCallback( (onStoreChange: () => void) => { - const handler = () => { - getSnapshot.clear(); - onStoreChange(); - }; - store.nodes.on("didRegister", handler); - store.nodes.on("didUnregister", handler); + store.connections.on("didRegister", onStoreChange); + store.connections.on("didUnregister", onStoreChange); return () => { - store.nodes.off("didRegister", handler); - store.nodes.off("didUnregister", handler); + store.connections.off("didRegister", onStoreChange); + store.connections.off("didUnregister", onStoreChange); }; }, - [getSnapshot, store.nodes], + [store], ); - return useSyncExternalStore(subscribe, getSnapshot); + return useStoreList(read, subscribe); } -export function useConnectionIds(parentComponentId: CCComponentId) { +export function useComponent(componentId: CCComponentId | undefined) { const { store } = useStore(); - const getSnapshot = useMemo( + const getSnapshot = useCallback( () => - memoizeOne(() => - store.connections - .getMany() - .filter( - (connection) => connection.parentComponentId === parentComponentId, - ) - .map((connection) => connection.id), - ), - [store, parentComponentId], + componentId === undefined ? undefined : store.components.get(componentId), + [store, componentId], ); const subscribe = useCallback( (onStoreChange: () => void) => { - const handler = () => { - getSnapshot.clear(); - onStoreChange(); + const handler = (component: CCComponent) => { + if (component.id === componentId) onStoreChange(); }; - store.connections.on("didRegister", handler); - store.connections.on("didUnregister", handler); + store.components.on("didUpdate", handler); return () => { - store.connections.off("didRegister", handler); - store.connections.off("didUnregister", handler); + store.components.off("didUpdate", handler); }; }, - [getSnapshot, store.connections], + [store, componentId], ); return useSyncExternalStore(subscribe, getSnapshot); } -export function useNode(nodeId: CCNodeId) { +export function useNode(nodeId: CCNodeId | undefined) { const { store } = useStore(); const getSnapshot = useCallback( - () => nullthrows(store.nodes.get(nodeId)), + () => (nodeId === undefined ? undefined : store.nodes.get(nodeId)), [store, nodeId], ); const subscribe = useCallback( @@ -142,10 +132,13 @@ export function useNode(nodeId: CCNodeId) { return useSyncExternalStore(subscribe, getSnapshot); } -export function useComponentPin(componentPinId: CCComponentPinId) { +export function useComponentPin(componentPinId: CCComponentPinId | undefined) { const { store } = useStore(); const getSnapshot = useCallback( - () => nullthrows(store.componentPins.get(componentPinId)), + () => + componentPinId === undefined + ? undefined + : store.componentPins.get(componentPinId), [store, componentPinId], ); const subscribe = useCallback( @@ -163,61 +156,6 @@ export function useComponentPin(componentPinId: CCComponentPinId) { return useSyncExternalStore(subscribe, getSnapshot); } -export function useComponentPins(componentId: CCComponentId) { - const { store } = useStore(); - const getSnapshot = useMemo( - () => - memoizeOne(() => store.componentPins.getManyByComponentId(componentId)), - [store, componentId], - ); - const subscribe = useCallback( - (onStoreChange: () => void) => { - const handler = (componentPin: CCComponentPin) => { - if (componentPin.componentId === componentId) { - getSnapshot.clear(); - onStoreChange(); - } - }; - store.componentPins.on("didRegister", handler); - store.componentPins.on("didUpdate", handler); - store.componentPins.on("didUnregister", handler); - return () => { - store.componentPins.off("didRegister", handler); - store.componentPins.off("didUpdate", handler); - store.componentPins.off("didUnregister", handler); - }; - }, - [getSnapshot, store, componentId], - ); - return useSyncExternalStore(subscribe, getSnapshot); -} - -export function useNodePins(nodeId: CCNodeId) { - const { store } = useStore(); - const getSnapshot = useMemo( - () => memoizeOne(() => store.nodePins.getManyByNodeId(nodeId)), - [store, nodeId], - ); - const subscribe = useCallback( - (onStoreChange: () => void) => { - const handler = (nodePin: CCNodePin) => { - if (nodePin.nodeId === nodeId) { - getSnapshot.clear(); - onStoreChange(); - } - }; - store.nodePins.on("didRegister", handler); - store.nodePins.on("didUnregister", handler); - return () => { - store.nodePins.off("didRegister", handler); - store.nodePins.off("didUnregister", handler); - }; - }, - [getSnapshot, store, nodeId], - ); - return useSyncExternalStore(subscribe, getSnapshot); -} - export function useCanSimulate(componentId: CCComponentId) { const { store } = useStore(); const getSnapshot = useCallback( diff --git a/src/store/react/snapshot.test.ts b/src/store/react/snapshot.test.ts new file mode 100644 index 0000000..cc9f452 --- /dev/null +++ b/src/store/react/snapshot.test.ts @@ -0,0 +1,31 @@ +import { describe, expect, it } from "vitest"; +import { reuseIfSameItems } from "./snapshot"; + +describe("reuseIfSameItems", () => { + it("hands out the previous list while it holds the same items", () => { + const item = { id: "a" }; + const previous = [item, "b"]; + + expect(reuseIfSameItems(previous, [item, "b"])).toBe(previous); + }); + + it("hands out the next list when an item is added, removed, replaced or moved", () => { + const previous = ["a", "b"]; + for (const next of [["a", "b", "c"], ["a"], ["a", "c"], ["b", "a"]]) { + expect(reuseIfSameItems(previous, next)).toBe(next); + } + }); + + it("tells an updated object from the one it replaced", () => { + const previous = [{ id: "a", name: "old" }]; + const next = [{ id: "a", name: "old" }]; + + expect(reuseIfSameItems(previous, next)).toBe(next); + }); + + it("hands out the next list on the first read", () => { + const next = ["a"]; + + expect(reuseIfSameItems(null, next)).toBe(next); + }); +}); diff --git a/src/store/react/snapshot.ts b/src/store/react/snapshot.ts new file mode 100644 index 0000000..be353fd --- /dev/null +++ b/src/store/react/snapshot.ts @@ -0,0 +1,14 @@ +/** The previous list if it holds the same items in the same order, else the next. */ +export function reuseIfSameItems( + previous: readonly T[] | null, + next: readonly T[], +): readonly T[] { + if ( + previous !== null && + previous.length === next.length && + previous.every((item, index) => Object.is(item, next[index])) + ) { + return previous; + } + return next; +} diff --git a/src/store/revision.test.ts b/src/store/revision.test.ts new file mode 100644 index 0000000..7f78b28 --- /dev/null +++ b/src/store/revision.test.ts @@ -0,0 +1,67 @@ +import { describe, expect, it } from "vitest"; +import { const_, not, true_ } from "./intrinsics/definitions"; +import { + connectTestNodePins, + createTestStore, + registerTestComponent, + registerTestInputNode, + registerTestNode, + testBits, + testNodePin, +} from "./testUtils"; + +describe("Store revision", () => { + it("counts a removal as soon as it happens, before it is announced", async () => { + const store = createTestStore(); + const root = registerTestComponent(store, "Root"); + const source = registerTestNode(store, root.id, true_.component.id); + const sink = registerTestNode(store, root.id, not.component.id); + connectTestNodePins( + store, + root.id, + testNodePin(store, source, true_.outputPin.Out), + testNodePin(store, sink, not.inputPin.In), + ); + let announced = false; + store.connections.on("didUnregister", () => { + announced = true; + }); + const before = store.revision; + + const removal = store.nodes.unregister([sink.id]); + + expect(announced).toBe(false); + expect(store.revision).toBeGreaterThan(before); + await removal; + expect(announced).toBe(true); + }); + + it("counts a config change but not a move of a node", () => { + const store = createTestStore(); + const root = registerTestComponent(store, "Root"); + const node = registerTestNode(store, root.id, const_.component.id); + const before = store.revision; + + store.nodes.update(node.id, { position: { x: 1, y: 2 } }); + expect(store.revision).toBe(before); + + store.nodes.update(node.id, { + config: { mode: "binary", data: testBits(0, 4) }, + }); + expect(store.revision).toBeGreaterThan(before); + }); + + it("announces an update with the updated object", () => { + const store = createTestStore(); + const root = registerTestComponent(store, "Root"); + const input = registerTestInputNode(store, root.id, "In"); + const names: string[] = []; + store.components.on("didUpdate", (component) => names.push(component.name)); + store.componentPins.on("didUpdate", (pin) => names.push(pin.name)); + + store.components.update(root.id, { name: "Renamed" }); + store.componentPins.update(input.componentPinId, { name: "Renamed In" }); + + expect(names).toEqual(["Renamed", "Renamed In"]); + }); +});