diff --git a/docs/adr/20260813-element-data-style-mappers.md b/docs/adr/20260813-element-data-style-mappers.md index 203efcc67..40a5def7a 100644 --- a/docs/adr/20260813-element-data-style-mappers.md +++ b/docs/adr/20260813-element-data-style-mappers.md @@ -21,3 +21,4 @@ Two structured/optional properties can't be a plain always-present mapper, so th - **A stylesheet consumer must merge into the base `node`/`edge` rules, never replace them.** `useSchemaGraphStyles` adds a schema label by spreading `{ ...baseStyles.node, label: … }`; overwriting the `node`/`edge` keys wholesale would discard every `ge_*` mapper and render the graph unstyled. Guarded by `useSchemaGraphStyles.test.tsx`. - Context-count regression guards (`useGraphStyles.contextCount.test.tsx`, `useSchemaGraphStyles.test.tsx`) assert the selector count stays O(1) regardless of type count, so the per-type-selector approach can't creep back in unnoticed. - A future reader seeing `data(ge_*)` mappers and no per-type selectors should not "restore" per-type selectors — that is the exact regression this avoids. +- **The `ge_*` producers must be fed a type set scoped to what is drawn, not the whole schema.** Moving styling into element data moves the per-type cost from the stylesheet into the render path, so resolving a style and an icon for every schema type became the new bottleneck (10,044 types resolved to draw 3). Canvas consumers take their scope from `canvasVertexStylesAtom`; only the schema view uses `useAllVertexStyles`, because drawing every type is its actual job. Edges have no canvas-scoped equivalent on purpose — edge style data needs no icon resolution, so scoping would buy nothing. diff --git a/docs/agents/react.md b/docs/agents/react.md index 62984c288..c988841aa 100644 --- a/docs/agents/react.md +++ b/docs/agents/react.md @@ -2,6 +2,7 @@ - This project uses React 19 - The React Compiler is enabled — it auto-memoizes components and hooks, so manual `useMemo`, `useCallback`, and `React.memo` are unnecessary in most cases and should be avoided unless profiling shows a specific need +- When per-call memoization genuinely is needed, a hook must not build and return a closure that mutates its own captured cache — the `react-compiler/*` lint rules reject it. Put the cache in a plain `create*` factory and have the hook call it, as `core/StateProvider/styleDataResolvers.ts` does. The two-line hook is not a pointless wrapper: collapsing it back inline fails lint. The factory also ends up store-free and directly unit-testable. - Official React docs: https://react.dev ## General @@ -16,6 +17,11 @@ - Server state goes in TanStack Query - **Exception: vertex icons.** They resolve through the `core/icons/` registry, read via `useSyncExternalStore`, because a per-hook subscription scaled with vertex-type count and locked up the schema view at 10k types. Don't move icon resolution back into TanStack Query — see `docs/adr/20260813-icon-registry-not-react-query.md` +## Client state (Jotai) + +- When a derivation is consumed by more than one hook or pipeline, define it as a derived atom so the store computes it once, rather than a hook each caller re-runs. `visibleVertexIdsAtom` is shared by `useRenderedVertices` and `useRenderedEdges` for exactly this reason; as a hook it ran the filter loop once per call site +- `atomFamily` never evicts unless you call `remove`/`setShouldRemove`, and nothing in this codebase does. Key a family on a stable branded ID, never on a freshly allocated object or array — the latter interns a new entry per recomputation that can never be reached again + ## Feature Modules - Feature modules in `src/modules/` contain all related components, hooks, and utilities diff --git a/packages/graph-explorer/src/core/StateProvider/displayVertex.ts b/packages/graph-explorer/src/core/StateProvider/displayVertex.ts index 5ca4560eb..03e5eac06 100644 --- a/packages/graph-explorer/src/core/StateProvider/displayVertex.ts +++ b/packages/graph-explorer/src/core/StateProvider/displayVertex.ts @@ -141,6 +141,6 @@ const displayVerticesSelector = atomFamily((vertices: Vertex[]) => }), ); -const displayVerticesInCanvasSelector = atom(get => { +export const displayVerticesInCanvasSelector = atom(get => { return get(displayVerticesSelector(get(nodesAtom).values().toArray())); }); diff --git a/packages/graph-explorer/src/core/StateProvider/renderedEntities.test.ts b/packages/graph-explorer/src/core/StateProvider/renderedEntities.test.ts index 6e9c90e6b..dd66fb031 100644 --- a/packages/graph-explorer/src/core/StateProvider/renderedEntities.test.ts +++ b/packages/graph-explorer/src/core/StateProvider/renderedEntities.test.ts @@ -1,11 +1,15 @@ // @vitest-environment happy-dom import { waitFor } from "@testing-library/react"; +import { createStore } from "jotai"; import { createEdgeId, + createVertex, createVertexId, createVertexType, } from "@/core/entities"; +import { iconRegistry } from "@/core/icons"; +import { LABELS } from "@/utils"; import { createRandomEdge, createRandomVertex, @@ -15,6 +19,7 @@ import { } from "@/utils/testing"; import { + canvasVertexStylesAtom, createRenderedEdgeId, createRenderedVertexId, getEdgeIdFromRenderedEdgeId, @@ -22,6 +27,7 @@ import { type RenderedEdgeId, type RenderedVertexId, useRenderedEntities, + visibleVertexIdsAtom, } from "./renderedEntities"; describe("createRenderedVertexId", () => { @@ -90,6 +96,125 @@ describe("getEdgeIdFromRenderedEdgeId", () => { }); }); +describe("visibleVertexIdsAtom", () => { + it("should exclude vertices filtered by ID and by type", () => { + const dbState = new DbState(); + const kept = createTestableVertex(); + const filteredById = createTestableVertex(); + const filteredByType = createTestableVertex(); + + dbState.addTestableVertexToGraph(kept); + dbState.addTestableVertexToGraph(filteredById); + dbState.addTestableVertexToGraph(filteredByType); + dbState.filterVertex(filteredById.id); + dbState.filterVertexType(createVertexType(filteredByType.types[0])); + + const store = createStore(); + dbState.applyTo(store); + + expect([...store.get(visibleVertexIdsAtom)]).toStrictEqual([kept.id]); + }); +}); + +describe("canvasVertexStylesAtom", () => { + // The point of the atom: a schema can carry far more types than the canvas + // draws, and resolving an icon for every one of them dominated render cost. + it("should cover only the types drawn on the canvas", () => { + const dbState = new DbState(); + const onCanvas = createTestableVertex(); + dbState.addTestableVertexToGraph(onCanvas); + dbState.addVertexStyle(createVertexType("NotOnCanvas"), { + color: "#123456", + }); + + const store = createStore(); + dbState.applyTo(store); + + expect(store.get(canvasVertexStylesAtom).map(s => s.type)).toStrictEqual([ + createVertexType(onCanvas.types[0]), + ]); + }); + + it("should exclude the types of filtered-out vertices", () => { + const dbState = new DbState(); + const kept = createTestableVertex(); + const filtered = createTestableVertex(); + dbState.addTestableVertexToGraph(kept); + dbState.addTestableVertexToGraph(filtered); + dbState.filterVertex(filtered.id); + + const store = createStore(); + dbState.applyTo(store); + + expect(store.get(canvasVertexStylesAtom).map(s => s.type)).toStrictEqual([ + createVertexType(kept.types[0]), + ]); + }); + + // `useAllVertexStyles` states this guarantee explicitly; here it has to hold + // via `createVertex` defaulting an untyped vertex to `LABELS.MISSING_TYPE`. + // Without it, blank nodes lose their icon silently. + it("should cover a blank node's synthetic missing type", () => { + const dbState = new DbState(); + dbState.addVertexToGraph( + createVertex({ id: "blank", isBlankNode: true, types: [] }), + ); + + const store = createStore(); + dbState.applyTo(store); + + expect(store.get(canvasVertexStylesAtom).map(s => s.type)).toStrictEqual([ + LABELS.MISSING_TYPE, + ]); + }); + + it("should list a shared type once", () => { + const dbState = new DbState(); + const first = createTestableVertex(); + const second = createTestableVertex().with({ types: first.types }); + dbState.addTestableVertexToGraph(first); + dbState.addTestableVertexToGraph(second); + + const store = createStore(); + dbState.applyTo(store); + + expect(store.get(canvasVertexStylesAtom)).toHaveLength(1); + }); +}); + +// The canvas resolves icons only for the types it draws, so the scope and the +// per-element lookup have to agree. A miss is silent — the node just renders +// without an icon — so assert the hit case on a real element. +describe("useRenderedVertices icon coverage", () => { + beforeEach(() => { + iconRegistry.reset(); + }); + + it("should carry the resolved icon url onto the element data", async () => { + const dbState = new DbState(); + const vertex = createTestableVertex(); + dbState.addTestableVertexToGraph(vertex); + // A raster icon resolves synchronously in the registry, so no fetch stub. + dbState.addVertexStyle(createVertexType(vertex.types[0]), { + iconUrl: "https://example.test/icon.png", + iconImageType: "image/png", + color: "#abcdef", + }); + + const { result } = renderHookWithJotai( + () => useRenderedEntities(), + store => dbState.applyTo(store), + ); + + await waitFor(() => { + expect(result.current.vertices[0].data.__iconUrl).toBe( + "https://example.test/icon.png", + ); + expect(result.current.vertices[0].data.ge_color).toBe("#abcdef"); + }); + }); +}); + describe("useRenderedVertices", () => { it("should return the filtered vertices by ID", async () => { const dbState = new DbState(); diff --git a/packages/graph-explorer/src/core/StateProvider/renderedEntities.ts b/packages/graph-explorer/src/core/StateProvider/renderedEntities.ts index 80ccea67a..d76402a19 100644 --- a/packages/graph-explorer/src/core/StateProvider/renderedEntities.ts +++ b/packages/graph-explorer/src/core/StateProvider/renderedEntities.ts @@ -1,33 +1,32 @@ -import { useAtomValue } from "jotai"; +import { atom, useAtomValue } from "jotai"; import type { Branded } from "@/utils"; import { type DisplayEdge, type DisplayVertex, + displayVerticesInCanvasSelector, edgesFilteredIdsAtom, edgesTypesFilteredAtom, - edgeStyleAtom, type EntityRawId, nodesFilteredIdsAtom, nodesTypesFilteredAtom, useAllNeighbors, - useAllVertexStyles, useDisplayEdgesInCanvas, useDisplayVerticesInCanvas, - vertexStyleAtom, type VertexId, + type VertexStyle, + vertexStyleAtom, + type VertexType, } from "@/core"; -import { useBackgroundImageMap } from "@/core/icons"; import type { EdgeId } from "../entities/edge"; +import type { EdgeStyleData, VertexStyleData } from "./graphElementStyleData"; import { - type EdgeStyleData, - edgeStyleData, - type VertexStyleData, - vertexStyleData, -} from "./graphElementStyleData"; + useEdgeStyleDataResolver, + useVertexStyleDataResolver, +} from "./styleDataResolvers"; /** A string representation of a vertex ID that encodes the original type. Cytoscape requires IDs to be strings. */ export type RenderedVertexId = Branded; @@ -41,41 +40,83 @@ export type RenderedVertex = ReturnType; /** A representation of an edge that Cytoscape can use. */ export type RenderedEdge = ReturnType; -/** Returns the filtered array of `RenderedVertex` instances for use by Cytoscape. */ -export function useRenderedVertices(): RenderedVertex[] { - const filteredIds = useAtomValue(nodesFilteredIdsAtom); - const filteredTypes = useAtomValue(nodesTypesFilteredAtom); - const displayVerticesInGraph = useDisplayVerticesInCanvas(); - const neighborCounts = useAllNeighbors(); - const vertexStyles = useAtomValue(vertexStyleAtom); - const backgroundImages = useBackgroundImageMap(useAllVertexStyles()); +/** + * The IDs of the canvas vertices that survive filtering. + * + * An atom rather than a hook so both the vertex and edge pipelines share one + * computation per store — as a hook it ran once per call site. + * + * Note this still recomputes when a vertex style changes, because + * `displayVerticesInCanvasSelector` resolves display labels through + * `vertexStyleByTypeAtom`. Only `id` and `types` are actually needed, so + * sourcing the predicate from `nodesAtom` would decouple it. + */ +export const visibleVertexIdsAtom = atom(get => { + const filteredIds = get(nodesFilteredIdsAtom); + const filteredTypes = get(nodesTypesFilteredAtom); + const displayVerticesInGraph = get(displayVerticesInCanvasSelector); - const result: RenderedVertex[] = []; + const result = new Set(); for (const vertex of displayVerticesInGraph.values()) { // Filters the nodes added to the graph by: // - Individual nodes hidden using the table view // - Vertex types unselected in the filter sidebar if (filteredIds.has(vertex.id)) continue; + if (vertex.types.some(type => filteredTypes.has(type))) continue; - // Check if any vertex type is in the filtered types - let hasFilteredType = false; - for (const type of vertex.types) { - if (filteredTypes.has(type)) { - hasFilteredType = true; - break; - } + result.add(vertex.id); + } + + return result; +}); + +/** + * Vertex styles for only the types drawn on the canvas. + * + * The schema can carry tens of thousands of vertex types while the canvas shows + * a handful, and resolving a style plus an icon for every type in the schema on + * every render is the dominant render cost otherwise. The schema view, which + * genuinely draws every type, uses `useAllVertexStyles` instead. + */ +export const canvasVertexStylesAtom = atom(get => { + const styles = get(vertexStyleAtom); + const displayVerticesInGraph = get(displayVerticesInCanvasSelector); + const visibleIds = get(visibleVertexIdsAtom); + + const types = new Set(); + for (const vertex of displayVerticesInGraph.values()) { + if (visibleIds.has(vertex.id)) { + types.add(vertex.primaryType); } - if (hasFilteredType) continue; + } + + const result: VertexStyle[] = []; + for (const type of types) { + result.push(styles.get(type)); + } + return result; +}); + +/** Returns the filtered array of `RenderedVertex` instances for use by Cytoscape. */ +export function useRenderedVertices(): RenderedVertex[] { + const displayVerticesInGraph = useDisplayVerticesInCanvas(); + const visibleIds = useAtomValue(visibleVertexIdsAtom); + const neighborCounts = useAllNeighbors(); + const canvasVertexStyles = useAtomValue(canvasVertexStylesAtom); + const resolveStyleData = useVertexStyleDataResolver(canvasVertexStyles); + + const result: RenderedVertex[] = []; + + for (const vertex of displayVerticesInGraph.values()) { + if (!visibleIds.has(vertex.id)) continue; const neighborCount = neighborCounts.get(vertex.id)?.unfetched ?? 0; - const style = vertexStyles.get(vertex.primaryType); - const backgroundImage = backgroundImages.get(vertex.primaryType); result.push( createRenderedVertex( vertex, neighborCount, - vertexStyleData(style, backgroundImage), + resolveStyleData(vertex.primaryType), ), ); } @@ -88,11 +129,8 @@ export function useRenderedEdges(): RenderedEdge[] { const edges = useDisplayEdgesInCanvas(); const filteredEdgeIds = useAtomValue(edgesFilteredIdsAtom); const filteredEdgeTypes = useAtomValue(edgesTypesFilteredAtom); - const vertices = useRenderedVertices(); - const edgeStyles = useAtomValue(edgeStyleAtom); - - // Get the IDs of the existing vertices - const existingVertexIds = new Set(vertices.map(v => v.data.vertexId)); + const visibleVertexIds = useAtomValue(visibleVertexIdsAtom); + const resolveStyleData = useEdgeStyleDataResolver(); const result: RenderedEdge[] = []; @@ -103,11 +141,10 @@ export function useRenderedEdges(): RenderedEdge[] { // - Missing source or target vertex if (filteredEdgeTypes.has(edge.type)) continue; if (filteredEdgeIds.has(edge.id)) continue; - if (!existingVertexIds.has(edge.sourceId)) continue; - if (!existingVertexIds.has(edge.targetId)) continue; + if (!visibleVertexIds.has(edge.sourceId)) continue; + if (!visibleVertexIds.has(edge.targetId)) continue; - const style = edgeStyles.get(edge.type); - result.push(createRenderedEdge(edge, edgeStyleData(style))); + result.push(createRenderedEdge(edge, resolveStyleData(edge.type))); } return result;