From b09629c654f0b01428168179e27aebb60de4b866 Mon Sep 17 00:00:00 2001 From: Kris McGinnes Date: Fri, 14 Aug 2026 14:40:37 -0500 Subject: [PATCH 1/2] Share one memoized label text color helper across canvas and previews `LabelPreview` inlined its own `new Color(...).isDark()` with no guard for the empty `labelColor` an imported style file can carry, so the preview could throw where the canvas does not. Both now call `labelTextColorFor`, memoized because parsing a color is the only non-trivial work here. Also reads the fallback from `appDefaultEdgeStyle.labelColor` rather than repeating the hex, and looks up line dash patterns through a `Map` so a `lineStyle` colliding with `Object.prototype` cannot resolve to a function. --- .../src/components/LabelPreview.test.tsx | 3 +- .../src/components/LabelPreview.tsx | 13 +++---- .../graphElementStyleData.test.ts | 11 ++++++ .../StateProvider/graphElementStyleData.ts | 35 ++++++++++++++----- 4 files changed, 44 insertions(+), 18 deletions(-) diff --git a/packages/graph-explorer/src/components/LabelPreview.test.tsx b/packages/graph-explorer/src/components/LabelPreview.test.tsx index 12b71bc2b..08c59e9ff 100644 --- a/packages/graph-explorer/src/components/LabelPreview.test.tsx +++ b/packages/graph-explorer/src/components/LabelPreview.test.tsx @@ -20,9 +20,10 @@ function renderLabel(style: LabelVisualStyle, scale = 2) { describe("LabelPreview", () => { describe("text color follows label darkness for contrast", () => { + // Casing follows `labelTextColorFor`, the helper the canvas shares. it("uses white text on a dark label color", () => { const el = renderLabel(labelStyle({ labelColor: "#1d2531" })); - expect(el.style.color).toBe("#ffffff"); + expect(el.style.color).toBe("#FFFFFF"); }); it("uses black text on a light label color", () => { diff --git a/packages/graph-explorer/src/components/LabelPreview.tsx b/packages/graph-explorer/src/components/LabelPreview.tsx index fed3f794a..33f8985c0 100644 --- a/packages/graph-explorer/src/components/LabelPreview.tsx +++ b/packages/graph-explorer/src/components/LabelPreview.tsx @@ -1,9 +1,6 @@ import type React from "react"; -import Color from "color"; - -import type { LabelVisualStyle } from "@/core"; - +import { type LabelVisualStyle, labelTextColorFor } from "@/core"; import { cn } from "@/utils"; /** @@ -31,8 +28,8 @@ interface LabelPreviewProps { /** * A label badge preview that faithfully matches cytoscape's canvas rendering - * at any scale. Text color is derived from `labelColor` darkness (white on dark, - * black on light) — same logic as `useGraphStyles.ts`. + * at any scale. Text color comes from `labelTextColorFor`, the same helper the + * canvas uses, so a preview cannot drift from what gets drawn. * * Used for both vertex and edge label previews. */ @@ -53,9 +50,7 @@ export function LabelPreview({ fontSize: FONT_SIZE * scale, padding: PADDING * scale, borderRadius: BORDER_RADIUS * scale, - color: new Color(labelStyle.labelColor).isDark() - ? "#ffffff" - : "#000000", + color: labelTextColorFor(labelStyle.labelColor), backgroundColor: `color-mix(in srgb, ${labelStyle.labelColor} ${labelStyle.labelBackgroundOpacity * 100}%, transparent)`, borderWidth: labelStyle.labelBorderWidth * scale || undefined, borderStyle: diff --git a/packages/graph-explorer/src/core/StateProvider/graphElementStyleData.test.ts b/packages/graph-explorer/src/core/StateProvider/graphElementStyleData.test.ts index d27b18948..bbdbdd5da 100644 --- a/packages/graph-explorer/src/core/StateProvider/graphElementStyleData.test.ts +++ b/packages/graph-explorer/src/core/StateProvider/graphElementStyleData.test.ts @@ -117,4 +117,15 @@ describe("labelTextColorFor", () => { expect(() => labelTextColorFor("")).not.toThrow(); expect(labelTextColorFor("")).toBe("#FFFFFF"); }); + + // The result is memoized in a module-level map, so a repeat call must not be + // able to return a different answer than the first. + it("returns a stable answer across repeated calls", () => { + expect(labelTextColorFor("#123456")).toBe(labelTextColorFor("#123456")); + expect(labelTextColorFor("")).toBe(labelTextColorFor("")); + }); + + it("keys the memo per color rather than sharing one answer", () => { + expect(labelTextColorFor("#000000")).not.toBe(labelTextColorFor("#ffffff")); + }); }); diff --git a/packages/graph-explorer/src/core/StateProvider/graphElementStyleData.ts b/packages/graph-explorer/src/core/StateProvider/graphElementStyleData.ts index 602f90a5b..366e60c1c 100644 --- a/packages/graph-explorer/src/core/StateProvider/graphElementStyleData.ts +++ b/packages/graph-explorer/src/core/StateProvider/graphElementStyleData.ts @@ -1,6 +1,11 @@ import Color from "color"; -import type { EdgeStyle, LineStyle, VertexStyle } from "./graphStyles"; +import { + appDefaultEdgeStyle, + type EdgeStyle, + type LineStyle, + type VertexStyle, +} from "./graphStyles"; /** * Per-element style data pushed onto cytoscape `ele.data()` so a single @@ -11,11 +16,11 @@ import type { EdgeStyle, LineStyle, VertexStyle } from "./graphStyles"; * dash-pattern remap so the style loop stays pure `data()`. */ -const LINE_PATTERN: Record = { - solid: undefined, - dashed: [5, 6], - dotted: [1, 2], -}; +/** A `Map` so a type name colliding with `Object.prototype` cannot resolve to a function. */ +const LINE_PATTERN = new Map([ + ["dashed", [5, 6]], + ["dotted", [1, 2]], +]); /** Data-mapper fields set on every rendered vertex. Feeds the single `node` rule. */ export type VertexStyleData = { @@ -47,13 +52,27 @@ export type EdgeStyleData = { ge_lineThickness: number; }; +/** + * Memoized because parsing a color is the one non-trivial computation in this + * module, and the number of distinct label colors in a graph is tiny next to + * the number of edges asking about them. + */ +const labelTextColors = new Map(); + /** * Picks white-on-dark / black-on-light for a label against its background color. * Falls back to the default label color when unset: an imported style file can * carry an empty `labelColor`, and `new Color("")` throws. */ export function labelTextColorFor(labelColor: string): "#FFFFFF" | "#000000" { - return new Color(labelColor || "#17457b").isDark() ? "#FFFFFF" : "#000000"; + let textColor = labelTextColors.get(labelColor); + if (textColor === undefined) { + textColor = new Color(labelColor || appDefaultEdgeStyle.labelColor).isDark() + ? "#FFFFFF" + : "#000000"; + labelTextColors.set(labelColor, textColor); + } + return textColor; } /** Precomputed cytoscape data-mapper fields for a rendered vertex. */ @@ -80,7 +99,7 @@ export function vertexStyleData( export function edgeStyleData(style: EdgeStyle): EdgeStyleData { const lineStyle: LineStyle = style.lineStyle === "dotted" ? "dashed" : style.lineStyle; - const dashPattern = LINE_PATTERN[style.lineStyle]; + const dashPattern = LINE_PATTERN.get(style.lineStyle); const data: EdgeStyleData = { ge_lineColor: style.lineColor, ge_lineStyle: lineStyle, From 4db11d9cf4f347b3ca36b08a0c2aeb0d4e5dfca3 Mon Sep 17 00:00:00 2001 From: Kris McGinnes Date: Fri, 14 Aug 2026 14:41:01 -0500 Subject: [PATCH 2/2] Resolve graph style data once per type instead of once per element Style data varies only by type, so within a render pass N elements of a type now cost one `vertexStyleData` call rather than N. The memoizing closure lives in a plain `create*Resolver` factory because the React Compiler lint rules reject a hook that returns a closure mutating its own captured cache. The vertex resolver takes the styles whose icons are in scope, since the canvas needs only the types it draws while the schema view needs every type. Names the atom lookups (`VertexStyleLookup`, `EdgeStyleLookup`) so that contract is explicit, drops the now-callerless `useAllEdgeStyles`, and tightens the style-context test to assert the exact selector set. --- .../src/core/StateProvider/graphStyles.ts | 23 ++-- .../src/core/StateProvider/index.ts | 1 + .../StateProvider/styleDataResolvers.test.ts | 129 ++++++++++++++++++ .../core/StateProvider/styleDataResolvers.ts | 82 +++++++++++ .../useGraphStyles.contextCount.test.tsx | 16 ++- .../modules/SchemaGraph/useSchemaGraphData.ts | 22 +-- 6 files changed, 243 insertions(+), 30 deletions(-) create mode 100644 packages/graph-explorer/src/core/StateProvider/styleDataResolvers.test.ts create mode 100644 packages/graph-explorer/src/core/StateProvider/styleDataResolvers.ts diff --git a/packages/graph-explorer/src/core/StateProvider/graphStyles.ts b/packages/graph-explorer/src/core/StateProvider/graphStyles.ts index 8026c7a37..619767924 100644 --- a/packages/graph-explorer/src/core/StateProvider/graphStyles.ts +++ b/packages/graph-explorer/src/core/StateProvider/graphStyles.ts @@ -199,8 +199,14 @@ export type LegacyUserStylingStorage = { * seam in that parser, not here. */ +/** Resolves the full style of any vertex type. */ +export type VertexStyleLookup = { get(type: VertexType): VertexStyle }; + +/** Resolves the full style of any edge type. */ +export type EdgeStyleLookup = { get(type: EdgeType): EdgeStyle }; + /** Vertex styles indexed by type for O(1) lookup, resolved against defaults. */ -export const vertexStyleAtom = atom(get => { +export const vertexStyleAtom = atom(get => { const userStyles = get(userVertexStylesAtom); return { get(type: VertexType) { @@ -210,7 +216,7 @@ export const vertexStyleAtom = atom(get => { }); /** Edge styles indexed by type for O(1) lookup, resolved against defaults. */ -export const edgeStyleAtom = atom(get => { +export const edgeStyleAtom = atom(get => { const userStyles = get(userEdgeStylesAtom); return { get(type: EdgeType) { @@ -244,8 +250,10 @@ export function resolveEdgeStyle( } /** Returns an array of vertex styles based on the known vertex types in the schema. - * Always includes an entry for `LABELS.MISSING_TYPE` so that blank nodes (which are - * assigned that synthetic type at runtime) receive icon styling on the canvas. + * For the schema view, which draws every type; the canvas scopes itself to the + * types it draws via `canvasVertexStylesAtom`. Always includes an entry for + * `LABELS.MISSING_TYPE` so blank nodes (assigned that synthetic type at runtime) + * are styled rather than skipped. */ export function useAllVertexStyles(): VertexStyle[] { const styles = useAtomValue(vertexStyleAtom); @@ -260,13 +268,6 @@ export function useAllVertexStyles(): VertexStyle[] { return [...schemaStyles, styles.get(missingType)]; } -/** Returns an array of edge styles based on the known edge types in the schema. */ -export function useAllEdgeStyles(): EdgeStyle[] { - const styles = useAtomValue(edgeStyleAtom); - const { edges: allSchemas } = useActiveSchema(); - return allSchemas.map(({ type }) => styles.get(type)); -} - /** Returns the resolved style for the specified vertex type. */ export function useVertexStyle(type: VertexType): VertexStyle { return useDeferredValue(useAtomValue(vertexStyleByTypeAtom(type))); diff --git a/packages/graph-explorer/src/core/StateProvider/index.ts b/packages/graph-explorer/src/core/StateProvider/index.ts index 17ba9bb7b..2f5145f14 100644 --- a/packages/graph-explorer/src/core/StateProvider/index.ts +++ b/packages/graph-explorer/src/core/StateProvider/index.ts @@ -12,6 +12,7 @@ export * from "./nodes"; export * from "./renderedEntities"; export * from "./graphStyles"; export * from "./graphElementStyleData"; +export * from "./styleDataResolvers"; export * from "./schema"; export * from "./storageAtoms"; export * from "./graphSession"; diff --git a/packages/graph-explorer/src/core/StateProvider/styleDataResolvers.test.ts b/packages/graph-explorer/src/core/StateProvider/styleDataResolvers.test.ts new file mode 100644 index 000000000..574fa0b57 --- /dev/null +++ b/packages/graph-explorer/src/core/StateProvider/styleDataResolvers.test.ts @@ -0,0 +1,129 @@ +// @vitest-environment happy-dom +import { act, waitFor } from "@testing-library/react"; + +import type { AppStore } from "@/core"; + +import { createEdgeType, createVertexType } from "@/core/entities"; +import { DbState, renderHookWithJotai } from "@/utils/testing"; + +import { userEdgeStylesAtom, userVertexStylesAtom } from "./storageAtoms"; +import { + useEdgeStyleDataResolver, + useVertexStyleDataResolver, +} from "./styleDataResolvers"; + +describe("useVertexStyleDataResolver", () => { + it("should return the same object for repeated lookups of a type", () => { + const type = createVertexType("Person"); + const { result } = renderHookWithJotai(() => + useVertexStyleDataResolver([]), + ); + + expect(result.current(type)).toBe(result.current(type)); + }); + + // The cache is keyed only by type, so a style edit has to replace the whole + // resolver. If it doesn't, the canvas keeps the old colors. + it("should reflect a style edited after the first lookup", async () => { + const type = createVertexType("Person"); + const dbState = new DbState(); + dbState.addVertexStyle(type, { color: "#111111" }); + + let store!: AppStore; + const { result } = renderHookWithJotai( + () => useVertexStyleDataResolver([]), + s => { + store = s; + dbState.applyTo(s); + }, + ); + expect(result.current(type).ge_color).toBe("#111111"); + + act(() => + store.set(userVertexStylesAtom, prev => + new Map(prev).set(type, { type, color: "#222222" }), + ), + ); + + await waitFor(() => { + expect(result.current(type).ge_color).toBe("#222222"); + }); + }); + + it("should resolve distinct style data per type", () => { + const dbState = new DbState(); + dbState.addVertexStyle(createVertexType("Person"), { color: "#111111" }); + dbState.addVertexStyle(createVertexType("City"), { color: "#222222" }); + + const { result } = renderHookWithJotai( + () => useVertexStyleDataResolver([]), + store => dbState.applyTo(store), + ); + + expect(result.current(createVertexType("Person")).ge_color).toBe("#111111"); + expect(result.current(createVertexType("City")).ge_color).toBe("#222222"); + }); + + // The canvas passes only the types it draws, so a type outside that scope must + // still resolve — without an icon. + it("should resolve a type outside the icon scope without an icon", () => { + const { result } = renderHookWithJotai(() => + useVertexStyleDataResolver([]), + ); + + expect( + result.current(createVertexType("Person")).__iconUrl, + ).toBeUndefined(); + }); +}); + +describe("useEdgeStyleDataResolver", () => { + it("should reflect a style edited after the first lookup", async () => { + const type = createEdgeType("route"); + const dbState = new DbState(); + dbState.addEdgeStyle(type, { lineColor: "#111111" }); + + let store!: AppStore; + const { result } = renderHookWithJotai( + () => useEdgeStyleDataResolver(), + s => { + store = s; + dbState.applyTo(s); + }, + ); + expect(result.current(type).ge_lineColor).toBe("#111111"); + + act(() => + store.set(userEdgeStylesAtom, prev => + new Map(prev).set(type, { type, lineColor: "#222222" }), + ), + ); + + await waitFor(() => { + expect(result.current(type).ge_lineColor).toBe("#222222"); + }); + }); + + it("should return the same object for repeated lookups of a type", () => { + const type = createEdgeType("route"); + const { result } = renderHookWithJotai(() => useEdgeStyleDataResolver()); + + expect(result.current(type)).toBe(result.current(type)); + }); + + it("should resolve distinct style data per type", () => { + const dbState = new DbState(); + dbState.addEdgeStyle(createEdgeType("route"), { lineColor: "#111111" }); + dbState.addEdgeStyle(createEdgeType("owns"), { lineColor: "#222222" }); + + const { result } = renderHookWithJotai( + () => useEdgeStyleDataResolver(), + store => dbState.applyTo(store), + ); + + expect(result.current(createEdgeType("route")).ge_lineColor).toBe( + "#111111", + ); + expect(result.current(createEdgeType("owns")).ge_lineColor).toBe("#222222"); + }); +}); diff --git a/packages/graph-explorer/src/core/StateProvider/styleDataResolvers.ts b/packages/graph-explorer/src/core/StateProvider/styleDataResolvers.ts new file mode 100644 index 000000000..4a60444ad --- /dev/null +++ b/packages/graph-explorer/src/core/StateProvider/styleDataResolvers.ts @@ -0,0 +1,82 @@ +import { useAtomValue } from "jotai"; + +import { useBackgroundImageMap } from "@/core/icons"; + +import type { EdgeType, VertexType } from "../entities"; + +import { + type EdgeStyleData, + edgeStyleData, + type VertexStyleData, + vertexStyleData, +} from "./graphElementStyleData"; +import { + type EdgeStyleLookup, + edgeStyleAtom, + type VertexStyle, + type VertexStyleLookup, + vertexStyleAtom, +} from "./graphStyles"; + +/** + * Style data varies only by type, so within one render pass the graph surfaces + * resolve it once per type rather than once per element — N nodes of a type cost + * one `vertexStyleData` call. + * + * The cache is keyed only by type; the styles and icons it was built from are + * pinned by resolver identity alone. It is discarded whenever those inputs + * change identity, which includes every node add or remove, so treat it as a + * per-render dedupe and not a cross-render cache. + */ + +/** Resolves the cytoscape data-mapper fields for a vertex type, memoized per type. */ +export type VertexStyleDataResolver = (type: VertexType) => VertexStyleData; + +/** Resolves the cytoscape data-mapper fields for an edge type, memoized per type. */ +export type EdgeStyleDataResolver = (type: EdgeType) => EdgeStyleData; + +/** + * @param iconStyles The styles whose icons should be resolved — the caller's + * scope, since the canvas needs only the types it draws while the schema view + * needs every type. A type outside this set resolves without an icon. + */ +export function useVertexStyleDataResolver( + iconStyles: VertexStyle[], +): VertexStyleDataResolver { + const styles = useAtomValue(vertexStyleAtom); + const backgroundImages = useBackgroundImageMap(iconStyles); + return createVertexStyleDataResolver(styles, backgroundImages); +} + +export function useEdgeStyleDataResolver(): EdgeStyleDataResolver { + return createEdgeStyleDataResolver(useAtomValue(edgeStyleAtom)); +} + +function createVertexStyleDataResolver( + styles: VertexStyleLookup, + backgroundImages: Map, +): VertexStyleDataResolver { + const cache = new Map(); + return type => { + let data = cache.get(type); + if (data === undefined) { + data = vertexStyleData(styles.get(type), backgroundImages.get(type)); + cache.set(type, data); + } + return data; + }; +} + +function createEdgeStyleDataResolver( + styles: EdgeStyleLookup, +): EdgeStyleDataResolver { + const cache = new Map(); + return type => { + let data = cache.get(type); + if (data === undefined) { + data = edgeStyleData(styles.get(type)); + cache.set(type, data); + } + return data; + }; +} diff --git a/packages/graph-explorer/src/modules/GraphViewer/useGraphStyles.contextCount.test.tsx b/packages/graph-explorer/src/modules/GraphViewer/useGraphStyles.contextCount.test.tsx index 61e03af06..75b4c4f49 100644 --- a/packages/graph-explorer/src/modules/GraphViewer/useGraphStyles.contextCount.test.tsx +++ b/packages/graph-explorer/src/modules/GraphViewer/useGraphStyles.contextCount.test.tsx @@ -49,9 +49,17 @@ describe("useGraphStyles style-context count", () => { expect(large).toBe(small); }); - it("emits a small fixed number of selectors, not per-type", async () => { - const count = await selectorCountFor(50); - // node rule + edge rule + at most a couple of gated rules - expect(count).toBeLessThanOrEqual(6); + it("emits exactly the fixed selector set, not per-type", async () => { + const { result } = renderHookWithState( + () => useGraphStyles(), + seedWithTypes(50), + ); + await waitFor(() => expect(result.current).toBeDefined()); + + expect(Object.keys(result.current).sort()).toStrictEqual([ + "edge", + "edge[ge_lineDashPattern]", + "node", + ]); }); }); diff --git a/packages/graph-explorer/src/modules/SchemaGraph/useSchemaGraphData.ts b/packages/graph-explorer/src/modules/SchemaGraph/useSchemaGraphData.ts index c69bf8c2e..1ce3cd675 100644 --- a/packages/graph-explorer/src/modules/SchemaGraph/useSchemaGraphData.ts +++ b/packages/graph-explorer/src/modules/SchemaGraph/useSchemaGraphData.ts @@ -1,24 +1,19 @@ -import { useAtomValue } from "jotai"; - import type { GraphEdge, GraphNode } from "@/components/Graph"; import { createEdgeConnectionId, type EdgeConnectionId, - edgeStyleAtom, - edgeStyleData, type EdgeStyleData, type EdgeType, useActiveSchema, useAllVertexStyles, useDisplayEdgeTypeConfigs, useDisplayVertexTypeConfigs, - vertexStyleAtom, - vertexStyleData, + useEdgeStyleDataResolver, + useVertexStyleDataResolver, type VertexStyleData, type VertexType, } from "@/core"; -import { useBackgroundImageMap } from "@/core/icons"; type SchemaGraphNode = GraphNode & { data: { @@ -54,20 +49,18 @@ export function useSchemaGraphData() { /** Transforms vertex type configs into schema graph nodes. */ function useSchemaGraphNodes(): SchemaGraphNode[] { const vtConfigs = useDisplayVertexTypeConfigs(); - const vertexStyles = useAtomValue(vertexStyleAtom); - const backgroundImages = useBackgroundImageMap(useAllVertexStyles()); + // The schema view draws every type, so every type's icon is in scope here. + const resolveStyleData = useVertexStyleDataResolver(useAllVertexStyles()); const nodes: SchemaGraphNode[] = []; for (const config of vtConfigs.values()) { - const style = vertexStyles.get(config.type); - const backgroundImage = backgroundImages.get(config.type); nodes.push({ data: { id: config.type, type: config.type, displayLabel: config.displayLabel, - ...vertexStyleData(style, backgroundImage), + ...resolveStyleData(config.type), }, }); } @@ -82,7 +75,7 @@ function useSchemaGraphEdges( const schema = useActiveSchema(); const edgeConnections = schema.edgeConnections ?? []; const etConfigs = useDisplayEdgeTypeConfigs(); - const edgeStyles = useAtomValue(edgeStyleAtom); + const resolveStyleData = useEdgeStyleDataResolver(); const edges: SchemaGraphEdge[] = []; @@ -93,7 +86,6 @@ function useSchemaGraphEdges( const edgeConfig = etConfigs.get(connection.edgeType); const displayLabel = edgeConfig?.displayLabel ?? connection.edgeType; - const style = edgeStyles.get(connection.edgeType); edges.push({ data: { @@ -102,7 +94,7 @@ function useSchemaGraphEdges( target: connection.targetVertexType, type: connection.edgeType, displayLabel, - ...edgeStyleData(style), + ...resolveStyleData(connection.edgeType), }, }); }