From 206f537d1221d4d25f548d9d829b8058f7f6ad6c Mon Sep 17 00:00:00 2001 From: Rn86222 Date: Sat, 14 Nov 2026 13:35:43 +0900 Subject: [PATCH 1/8] refactor(store): read lists from the store on every render The hooks returning a list (components, node ids, connection ids) cached it with memoize-one and only dropped the cache when the event announcing a change arrived. Some removals are announced only when their transaction ends, so a component rendered in between got a list that still held an object that was gone. Read the list from the store on every render instead, and hand out the previous array while its items are the same, so that React still skips re-renders when nothing changed. Also drop `useComponentPins` and `useNodePins`, which nothing uses, and the memoize-one dependency. Co-Authored-By: Claude Opus 5.5 --- package-lock.json | 7 -- package.json | 1 - src/store/react/selectors.ts | 191 +++++++++++-------------------- src/store/react/snapshot.test.ts | 32 ++++++ src/store/react/snapshot.ts | 20 ++++ 5 files changed, 120 insertions(+), 131 deletions(-) create mode 100644 src/store/react/snapshot.test.ts create mode 100644 src/store/react/snapshot.ts 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/store/react/selectors.ts b/src/store/react/selectors.ts index fcfd27a..5844f1d 100644 --- a/src/store/react/selectors.ts +++ b/src/store/react/selectors.ts @@ -1,6 +1,5 @@ -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,114 +8,115 @@ 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"; + +/** + * A list read from the store, kept in step with it. + * + * The list is read again on every render instead of being cached until the event + * announcing a change arrives. Some removals are announced only when their transaction + * ends, and a list cached until then would still hand out an object that is gone to a + * component rendered in the meantime. The previous array is handed out again while its + * items are the same, so that React does not re-render for nothing. + * @param read reads the list from the store; must be stable across renders + * @param subscribe subscribes to the events that can change the list + */ +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) { const { store } = useStore(); - const getSnapshot = useMemo( - () => - memoizeOne(() => - store.connections - .getMany() - .filter( - (connection) => connection.parentComponentId === parentComponentId, - ) - .map((connection) => connection.id), - ), - [store, parentComponentId], + const getSnapshot = useCallback( + () => nullthrows(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); } @@ -163,61 +163,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..cd4c679 --- /dev/null +++ b/src/store/react/snapshot.test.ts @@ -0,0 +1,32 @@ +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", () => { + // The stores replace an object on update, so a new object means a change + 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..9763927 --- /dev/null +++ b/src/store/react/snapshot.ts @@ -0,0 +1,20 @@ +/** + * The previous list if it holds the same items in the same order, or else the next one. + * `useSyncExternalStore` re-renders whenever the snapshot is a different object, so a + * list read afresh from the store keeps its identity this way while nothing changed. + * @param previous the list handed out last time, or null on the first read + * @param next the list just read + */ +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; +} From d6d493d97ca4834ecf10fd0991ee1e5f19c90bfe Mon Sep 17 00:00:00 2001 From: Rn86222 Date: Sat, 14 Nov 2026 13:35:59 +0900 Subject: [PATCH 2/8] fix(editor): render nothing once the object of a renderer is gone A renderer can be re-rendered by its own subscription before the list it comes from drops it. The renderer of a connection, for one, follows the nodes at both ends, so moving one of them re-rendered it even after the connection was removed, and it failed to look the connection up. `ensureStoreItem` did not prevent this: it checked the object in a wrapper, but the subscription re-renders the inner component alone. Instead, the hooks reading a single object now return undefined once it is gone, and the renderers of connections, nodes and interface nodes call all their hooks first and then render nothing if their object is missing. `ensureStoreItem` is removed. Co-Authored-By: Claude Opus 5.5 --- .../edit/Editor/renderer/Connection/index.tsx | 132 ++++++------- .../Node/components/Interface/index.tsx | 9 +- src/pages/edit/Editor/renderer/Node/index.tsx | 178 +++++++++--------- src/store/react/error.tsx | 14 -- src/store/react/selectors.ts | 23 ++- 5 files changed, 174 insertions(+), 182 deletions(-) delete mode 100644 src/store/react/error.tsx diff --git a/src/pages/edit/Editor/renderer/Connection/index.tsx b/src/pages/edit/Editor/renderer/Connection/index.tsx index 4ff2b66..97fd5a7 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, @@ -98,73 +97,74 @@ 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); + // Subscribed to follow the nodes at both ends when they move + const fromNode = useNode(fromNodePin?.nodeId); + const toNode = useNode(toNodePin?.nodeId); + const isInBitWidthConflict = useIsConnectionInBitWidthConflict(connectionId); + const [isHovered, setIsHovered] = useState(false); + + // Moving a node re-renders this alone, which can happen after the connection is + // removed but before the list of connections drops it + 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..72ca2d8 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,12 @@ 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, ); + // Renaming the pin re-renders this alone, which can happen after the node is removed + 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..9f126dc 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,97 @@ 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); + // Re-rendered alone by its own subscriptions, this can run after the node is + // removed but before the list of nodes drops it + 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/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 5844f1d..aeac523 100644 --- a/src/store/react/selectors.ts +++ b/src/store/react/selectors.ts @@ -1,4 +1,3 @@ -import nullthrows from "nullthrows"; import { useCallback, useRef, useSyncExternalStore } from "react"; import { type CCComponent, @@ -100,10 +99,17 @@ export function useConnectionIds(parentComponentId: CCComponentId) { return useStoreList(read, subscribe); } -export function useComponent(componentId: CCComponentId) { +/* + * The hooks reading a single object return undefined once it is gone. A component can + * be re-rendered by its own subscription before the list it comes from drops it, so it + * renders nothing instead of failing. + */ + +export function useComponent(componentId: CCComponentId | undefined) { const { store } = useStore(); const getSnapshot = useCallback( - () => nullthrows(store.components.get(componentId)), + () => + componentId === undefined ? undefined : store.components.get(componentId), [store, componentId], ); const subscribe = useCallback( @@ -121,10 +127,10 @@ export function useComponent(componentId: CCComponentId) { 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 +148,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( From 6b649999f4ff43eddccfc50857b22c2320c70844 Mon Sep 17 00:00:00 2001 From: Rn86222 Date: Sat, 14 Nov 2026 13:36:02 +0900 Subject: [PATCH 3/8] fix(store): announce the removal of a connection when its transaction ends #75 made connections announce their removal right away, unlike the other stores, because a renderer could look up a removed connection before the list of connections dropped it. The lists are now read from the store on every render and the renderers tolerate a removed object, so that special case is no longer needed. Announce it after the transaction like the other stores. The bit widths derived from a connection are stale as soon as it is gone, so the connection store now clears the cache of the node pin store right after the removal instead of on the announcement. Co-Authored-By: Claude Opus 5.5 --- src/store/bitWidth.test.ts | 37 ++++++++++++++++++++++++++++++++++++ src/store/connection.test.ts | 37 ++++++++++++++++++++++++++++++++++++ src/store/connection.ts | 12 ++++-------- src/store/nodePin.ts | 21 +++++++++++--------- 4 files changed, 90 insertions(+), 17 deletions(-) create mode 100644 src/store/connection.test.ts diff --git a/src/store/bitWidth.test.ts b/src/store/bitWidth.test.ts index aa94d2b..11e74d2 100644 --- a/src/store/bitWidth.test.ts +++ b/src/store/bitWidth.test.ts @@ -331,6 +331,43 @@ 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, + ); + // Not awaited: renderers query the bit widths before the removal is announced + 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/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 7af4615..181d1ec 100644 --- a/src/store/connection.ts +++ b/src/store/connection.ts @@ -82,15 +82,11 @@ export class CCConnectionStore extends CCBaseStore< 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.store.nodePins.clearBitWidthCache(); }); + for (const connection of connections) { + this.emit("didUnregister", connection); + } } /** diff --git a/src/store/nodePin.ts b/src/store/nodePin.ts index 32a0ecf..e94ea04 100644 --- a/src/store/nodePin.ts +++ b/src/store/nodePin.ts @@ -39,7 +39,12 @@ export class CCNodePinStore extends CCBaseStore< /** components whose bit widths are being analyzed, to catch one containing itself */ #componentsBeingAnalyzed: Set = new Set(); - #clearBitWidthCache(): void { + /** + * Drop the cached bit widths. A store removing an object announces it only when its + * transaction ends, but the bit widths derived from the object are stale as soon as + * it is gone, so such a store calls this right after the removal. + */ + clearBitWidthCache(): void { this.#bitWidthsByComponent.clear(); } @@ -50,12 +55,10 @@ export class CCNodePinStore extends CCBaseStore< } mount() { - this.store.connections.on("didRegister", () => this.#clearBitWidthCache()); - this.store.connections.on("didUnregister", () => - this.#clearBitWidthCache(), - ); + // A removed connection clears the cache itself, before its removal is announced + this.store.connections.on("didRegister", () => 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("didUpdateConfig", () => this.clearBitWidthCache()); this.store.nodes.on("didRegister", (node) => { const componentPins = this.store.componentPins.getManyByComponentId( node.componentId, @@ -107,7 +110,7 @@ export class CCNodePinStore extends CCBaseStore< invariant(this.store.componentPins.get(nodePin.componentPinId)); invariant(this.store.nodes.get(nodePin.nodeId)); this.objects.set(nodePin.id, nodePin); - this.#clearBitWidthCache(); + this.clearBitWidthCache(); this.emit("didRegister", nodePin); } @@ -123,7 +126,7 @@ export class CCNodePinStore extends CCBaseStore< 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.clearBitWidthCache(); }); this.emit("didUnregister", nodePin); this.#markedAsDeleted.delete(id); @@ -133,7 +136,7 @@ export class CCNodePinStore extends CCBaseStore< const existingNodePin = nullthrows(this.objects.get(id)); const newNodePin = { ...existingNodePin, ...value }; this.objects.set(id, newNodePin); - this.#clearBitWidthCache(); + this.clearBitWidthCache(); this.emit("didUpdate", newNodePin); } From d2b90b02e5874c3aa2d09a74965b5dddc852645b Mon Sep 17 00:00:00 2001 From: Rn86222 Date: Sat, 14 Nov 2026 13:45:18 +0900 Subject: [PATCH 4/8] fix(editor): keep the selected connections when opening their context menu Right-clicking a connection always selected it alone, so several connections selected with shift-click could not be deleted together. Keep the selection when the clicked connection is part of it, as the context menu of a node already does. Co-Authored-By: Claude Opus 5.5 --- src/pages/edit/Editor/renderer/Connection/index.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/pages/edit/Editor/renderer/Connection/index.tsx b/src/pages/edit/Editor/renderer/Connection/index.tsx index 97fd5a7..3080ca1 100644 --- a/src/pages/edit/Editor/renderer/Connection/index.tsx +++ b/src/pages/edit/Editor/renderer/Connection/index.tsx @@ -84,7 +84,11 @@ export function CCComponentEditorRendererConnectionCore({ } e.preventDefault(); e.stopPropagation(); - componentEditorState.selectConnection([connectionId], true); + // Keep a selection containing it, so that the menu acts on all of them, like + // the context menu of a node does + if (!componentEditorState.selectedConnectionIds.has(connectionId)) { + componentEditorState.selectConnection([connectionId], true); + } componentEditorState.openContextMenu(e); }} id={connectionId} From 99525ba03807007215207ded8fa0b83fafc6b6a4 Mon Sep 17 00:00:00 2001 From: Rn86222 Date: Sat, 14 Nov 2026 18:28:31 +0900 Subject: [PATCH 5/8] refactor(store): count circuit changes as they happen and cache bit widths on them The bit width cache was dropped on the events announcing a change. A removal is announced only when its transaction ends, so the cache was stale in between, and the node pin and connection stores dropped it by hand right after a removal to make up for that. The transaction delays the announcement on purpose; the cache was listening to the wrong signal. Count the revision of the store at the moment an object is stored or removed instead of on the announcement. The object-specific stores now change their objects only through `setObject` and `deleteObject` of the base store, which count the change; the map itself is read-only to them. Moving a node still does not count. The bit width cache is rebuilt when the revision differs from the one it was built at, so it needs neither the event listeners nor the manual clearing. The simulation netlist, cached on the same revision, is now exact during a transaction as well. Also: - announce the updated object in `didUpdate` of the component and component pin stores, which announced the one it replaced, and - remove `isMarkedAsDeleted` of the node pin store, which nothing uses. Co-Authored-By: Claude Opus 5.5 --- src/store/base.ts | 19 +++++++++++- src/store/component.ts | 9 +++--- src/store/componentPin.ts | 11 ++++--- src/store/connection.ts | 7 ++--- src/store/index.ts | 11 ++++--- src/store/node.ts | 10 +++--- src/store/nodePin.ts | 40 ++++++------------------ src/store/revision.test.ts | 63 ++++++++++++++++++++++++++++++++++++++ 8 files changed, 118 insertions(+), 52 deletions(-) create mode 100644 src/store/revision.test.ts diff --git a/src/store/base.ts b/src/store/base.ts index 7a98445..af856b5 100644 --- a/src/store/base.ts +++ b/src/store/base.ts @@ -16,7 +16,24 @@ export abstract class CCBaseStore< > extends EventEmitter { protected readonly store: CCStore; - protected readonly objects: Map = new Map(); + readonly #objects: Map = new Map(); + + protected get objects(): ReadonlyMap { + return this.#objects; + } + + protected setObject( + object: T, + { changesCircuit = true }: { changesCircuit?: boolean } = {}, + ): void { + this.#objects.set(object.id, object); + if (changesCircuit) this.store.countCircuitChange(); + } + + protected deleteObject(id: Id): void { + this.#objects.delete(id); + this.store.countCircuitChange(); + } constructor(store: CCStore) { super(); diff --git a/src/store/component.ts b/src/store/component.ts index 8f72a09..3acb1e2 100644 --- a/src/store/component.ts +++ b/src/store/component.ts @@ -42,7 +42,7 @@ export class CCComponentStore extends CCBaseStore< */ register(component: CCComponent): void { invariant(!this.objects.has(component.id)); - this.objects.set(component.id, component); + this.setObject(component); this.emit("didRegister", component); } @@ -55,7 +55,7 @@ export class CCComponentStore extends CCBaseStore< 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); } @@ -68,8 +68,9 @@ export class CCComponentStore extends CCBaseStore< 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 d17c690..ffc570b 100644 --- a/src/store/componentPin.ts +++ b/src/store/componentPin.ts @@ -60,7 +60,7 @@ export class CCComponentPinStore extends CCBaseStore< > { import(componentPins: CCComponentPin[]): void { for (const pin of componentPins) { - this.objects.set(pin.id, pin); + this.setObject(pin); } } @@ -86,7 +86,7 @@ export class CCComponentPinStore extends CCBaseStore< */ register(pin: CCComponentPin): void { invariant(this.store.components.get(pin.componentId)); - this.objects.set(pin.id, pin); + this.setObject(pin); this.emit("didRegister", pin); } @@ -120,7 +120,7 @@ export class CCComponentPinStore extends CCBaseStore< 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); } @@ -167,8 +167,9 @@ export class CCComponentPinStore extends CCBaseStore< ): 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.ts b/src/store/connection.ts index 181d1ec..dd13755 100644 --- a/src/store/connection.ts +++ b/src/store/connection.ts @@ -35,7 +35,7 @@ export class CCConnectionStore extends CCBaseStore< > { import(connections: CCConnection[]): void { for (const connection of connections) { - this.objects.set(connection.id, connection); + this.setObject(connection); } } @@ -67,7 +67,7 @@ export class CCConnectionStore extends CCBaseStore< ) { return; } - this.objects.set(connection.id, connection); + this.setObject(connection); this.emit("didRegister", connection); } @@ -80,9 +80,8 @@ export class CCConnectionStore extends CCBaseStore< await this.store.transactionManager.runInTransaction(() => { for (const connection of connections) { this.emit("willUnregister", connection); - this.objects.delete(connection.id); + this.deleteObject(connection.id); } - this.store.nodePins.clearBitWidthCache(); }); for (const connection of connections) { this.emit("didUnregister", connection); diff --git a/src/store/index.ts b/src/store/index.ts index fb79db7..6b2e8f2 100644 --- a/src/store/index.ts +++ b/src/store/index.ts @@ -40,11 +40,18 @@ export default class CCStore { * 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 @@ -65,10 +72,6 @@ export default class CCStore { this.componentPins.register(pin); } } - - this.subscribeToCircuitChanges(() => { - this.#revision += 1; - }); } /** diff --git a/src/store/node.ts b/src/store/node.ts index 905724a..86f8e8e 100644 --- a/src/store/node.ts +++ b/src/store/node.ts @@ -70,7 +70,7 @@ export class CCNodeStore extends CCBaseStore< 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); } @@ -83,7 +83,7 @@ export class CCNodeStore extends CCBaseStore< 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) { @@ -114,8 +114,10 @@ export class CCNodeStore extends CCBaseStore< ): 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 e94ea04..ae43831 100644 --- a/src/store/nodePin.ts +++ b/src/store/nodePin.ts @@ -32,22 +32,13 @@ export class CCNodePinStore extends CCBaseStore< CCNodePin, CCNodePinStoreEvents > { - #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(); - /** - * Drop the cached bit widths. A store removing an object announces it only when its - * transaction ends, but the bit widths derived from the object are stale as soon as - * it is gone, so such a store calls this right after the removal. - */ - clearBitWidthCache(): void { - this.#bitWidthsByComponent.clear(); - } - import(nodePins: CCNodePin[]) { for (const nodePin of nodePins) { this.register(nodePin); @@ -55,10 +46,6 @@ export class CCNodePinStore extends CCBaseStore< } mount() { - // A removed connection clears the cache itself, before its removal is announced - this.store.connections.on("didRegister", () => 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, @@ -109,8 +96,7 @@ export class CCNodePinStore extends CCBaseStore< 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); } @@ -120,23 +106,17 @@ export class CCNodePinStore extends CCBaseStore< */ 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); } @@ -177,10 +157,14 @@ export class CCNodePinStore extends CCBaseStore< /** * 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( @@ -225,10 +209,6 @@ export class CCNodePinStore extends CCBaseStore< 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/revision.test.ts b/src/store/revision.test.ts new file mode 100644 index 0000000..0102724 --- /dev/null +++ b/src/store/revision.test.ts @@ -0,0 +1,63 @@ +import { describe, expect, it } from "vitest"; +import { const_, not, true_ } from "./intrinsics/definitions"; +import { + connectTestNodePins, + createTestStore, + registerTestComponent, + 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 names: string[] = []; + store.components.on("didUpdate", (component) => names.push(component.name)); + + store.components.update(root.id, { name: "Renamed" }); + + expect(names).toEqual(["Renamed"]); + }); +}); From 4791034333df340177b643f68f15c284d7049b33 Mon Sep 17 00:00:00 2001 From: Rn86222 Date: Sat, 14 Nov 2026 18:28:52 +0900 Subject: [PATCH 6/8] docs: drop comments that restate the code Co-Authored-By: Claude Opus 5.5 --- .../edit/Editor/renderer/Connection/index.tsx | 5 ----- .../Node/components/Interface/index.tsx | 1 - src/pages/edit/Editor/renderer/Node/index.tsx | 2 -- src/store/bitWidth.test.ts | 1 - src/store/react/selectors.ts | 18 +----------------- src/store/react/snapshot.test.ts | 1 - src/store/react/snapshot.ts | 8 +------- 7 files changed, 2 insertions(+), 34 deletions(-) diff --git a/src/pages/edit/Editor/renderer/Connection/index.tsx b/src/pages/edit/Editor/renderer/Connection/index.tsx index 3080ca1..4b1c6e0 100644 --- a/src/pages/edit/Editor/renderer/Connection/index.tsx +++ b/src/pages/edit/Editor/renderer/Connection/index.tsx @@ -84,8 +84,6 @@ export function CCComponentEditorRendererConnectionCore({ } e.preventDefault(); e.stopPropagation(); - // Keep a selection containing it, so that the menu acts on all of them, like - // the context menu of a node does if (!componentEditorState.selectedConnectionIds.has(connectionId)) { componentEditorState.selectConnection([connectionId], true); } @@ -109,14 +107,11 @@ export default function CCComponentEditorRendererConnection({ const connection = store.connections.get(connectionId); const fromNodePin = connection && store.nodePins.get(connection.from); const toNodePin = connection && store.nodePins.get(connection.to); - // Subscribed to follow the nodes at both ends when they move const fromNode = useNode(fromNodePin?.nodeId); const toNode = useNode(toNodePin?.nodeId); const isInBitWidthConflict = useIsConnectionInBitWidthConflict(connectionId); const [isHovered, setIsHovered] = useState(false); - // Moving a node re-renders this alone, which can happen after the connection is - // removed but before the list of connections drops it if (!connection || !fromNodePin || !toNodePin || !fromNode || !toNode) { return null; } 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 72ca2d8..303bf22 100644 --- a/src/pages/edit/Editor/renderer/Node/components/Interface/index.tsx +++ b/src/pages/edit/Editor/renderer/Node/components/Interface/index.tsx @@ -21,7 +21,6 @@ export function CCComponentEditorRendererNodeInterfaceRenderer( const componentPin = useComponentPin( nodePin && store.componentPins.getByImplementation(nodePin.id)?.id, ); - // Renaming the pin re-renders this alone, which can happen after the node is removed if (!nodePin || !componentPin) return null; const isEditMode = editorState.editorMode === "edit"; diff --git a/src/pages/edit/Editor/renderer/Node/index.tsx b/src/pages/edit/Editor/renderer/Node/index.tsx index 9f126dc..3c79012 100644 --- a/src/pages/edit/Editor/renderer/Node/index.tsx +++ b/src/pages/edit/Editor/renderer/Node/index.tsx @@ -51,8 +51,6 @@ export default function CCComponentEditorRendererNode({ vector2.zero, ); - // Re-rendered alone by its own subscriptions, this can run after the node is - // removed but before the list of nodes drops it if (!node || !component) return null; const layout = getCCComponentEditorRendererNodeLayout(store, nodeId); diff --git a/src/store/bitWidth.test.ts b/src/store/bitWidth.test.ts index 11e74d2..b3c25d7 100644 --- a/src/store/bitWidth.test.ts +++ b/src/store/bitWidth.test.ts @@ -357,7 +357,6 @@ describe("bit width groups", () => { const [toScreen] = store.connections.getConnectionsByNodePinId( screen.pixels, ); - // Not awaited: renderers query the bit widths before the removal is announced void store.connections.unregister([nullthrows(toScreen).id]); expect(conflictingConnectionCount(store)).toBe(0); diff --git a/src/store/react/selectors.ts b/src/store/react/selectors.ts index aeac523..c605821 100644 --- a/src/store/react/selectors.ts +++ b/src/store/react/selectors.ts @@ -10,17 +10,7 @@ import type { CCNode, CCNodeId } from "../node"; import { useStore } from "."; import { reuseIfSameItems } from "./snapshot"; -/** - * A list read from the store, kept in step with it. - * - * The list is read again on every render instead of being cached until the event - * announcing a change arrives. Some removals are announced only when their transaction - * ends, and a list cached until then would still hand out an object that is gone to a - * component rendered in the meantime. The previous array is handed out again while its - * items are the same, so that React does not re-render for nothing. - * @param read reads the list from the store; must be stable across renders - * @param subscribe subscribes to the events that can change the list - */ +/** Read on every render, since a removal is announced only after its transaction. */ function useStoreList( read: () => readonly T[], subscribe: (onStoreChange: () => void) => () => void, @@ -99,12 +89,6 @@ export function useConnectionIds(parentComponentId: CCComponentId) { return useStoreList(read, subscribe); } -/* - * The hooks reading a single object return undefined once it is gone. A component can - * be re-rendered by its own subscription before the list it comes from drops it, so it - * renders nothing instead of failing. - */ - export function useComponent(componentId: CCComponentId | undefined) { const { store } = useStore(); const getSnapshot = useCallback( diff --git a/src/store/react/snapshot.test.ts b/src/store/react/snapshot.test.ts index cd4c679..cc9f452 100644 --- a/src/store/react/snapshot.test.ts +++ b/src/store/react/snapshot.test.ts @@ -17,7 +17,6 @@ describe("reuseIfSameItems", () => { }); it("tells an updated object from the one it replaced", () => { - // The stores replace an object on update, so a new object means a change const previous = [{ id: "a", name: "old" }]; const next = [{ id: "a", name: "old" }]; diff --git a/src/store/react/snapshot.ts b/src/store/react/snapshot.ts index 9763927..be353fd 100644 --- a/src/store/react/snapshot.ts +++ b/src/store/react/snapshot.ts @@ -1,10 +1,4 @@ -/** - * The previous list if it holds the same items in the same order, or else the next one. - * `useSyncExternalStore` re-renders whenever the snapshot is a different object, so a - * list read afresh from the store keeps its identity this way while nothing changed. - * @param previous the list handed out last time, or null on the first read - * @param next the list just read - */ +/** 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[], From 6ffe6cf319669a1e44bca6faa38cfa15d835c2c0 Mon Sep 17 00:00:00 2001 From: Rn86222 Date: Sat, 14 Nov 2026 18:36:27 +0900 Subject: [PATCH 7/8] fix(editor): act only on selected objects that still exist The selection can keep the ids of objects removed since, e.g. connections removed along with a node pin, and Delete itself kept the removed connections selected because it cleared the selection non-exclusively. Since a right-click now keeps a selection containing the clicked connection, such ids reached Delete, and removing a connection that was already gone threw in `unregister` and left the others in place. Filter the selection by what still exists before acting on it, and clear the selection exclusively after Delete. Co-Authored-By: Claude Opus 5.5 --- .../edit/Editor/components/ContextMenu.tsx | 62 ++++++++----------- 1 file changed, 25 insertions(+), 37 deletions(-) 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; From 9f3ac4f518f66f3a4ba03b4c0fc5fb3e623c3f11 Mon Sep 17 00:00:00 2001 From: Rn86222 Date: Sat, 14 Nov 2026 18:36:27 +0900 Subject: [PATCH 8/8] docs(store): update the comments the revision change made stale Also test that the component pin store announces an update with the updated pin. Co-Authored-By: Claude Opus 5.5 --- src/store/index.ts | 9 ++++----- src/store/revision.test.ts | 6 +++++- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/store/index.ts b/src/store/index.ts index 6b2e8f2..57e89d0 100644 --- a/src/store/index.ts +++ b/src/store/index.ts @@ -75,9 +75,9 @@ export default class CCStore { } /** - * 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 */ @@ -136,8 +136,7 @@ export default class CCStore { /** * 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/revision.test.ts b/src/store/revision.test.ts index 0102724..7f78b28 100644 --- a/src/store/revision.test.ts +++ b/src/store/revision.test.ts @@ -4,6 +4,7 @@ import { connectTestNodePins, createTestStore, registerTestComponent, + registerTestInputNode, registerTestNode, testBits, testNodePin, @@ -53,11 +54,14 @@ describe("Store revision", () => { 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"]); + expect(names).toEqual(["Renamed", "Renamed In"]); }); });