From 1e57a4e5e472044ed98a784f2944c0ecfd71c8b4 Mon Sep 17 00:00:00 2001 From: Bob Carmichael Date: Wed, 21 Jan 2026 05:20:07 -0500 Subject: [PATCH 1/3] Stabilize selector references to prevent unnecessary re-rendering; extract common methods --- src/features/effects/effectsSlice.js | 11 +- src/features/layers/layersSlice.js | 238 +++++++++++------------- src/features/layers/layersSlice.spec.js | 64 +++++++ 3 files changed, 172 insertions(+), 141 deletions(-) diff --git a/src/features/effects/effectsSlice.js b/src/features/effects/effectsSlice.js index f7bed60f..31f0d015 100644 --- a/src/features/effects/effectsSlice.js +++ b/src/features/effects/effectsSlice.js @@ -1,9 +1,9 @@ /* global localStorage */ import { createSlice, createEntityAdapter } from "@reduxjs/toolkit" -import { createSelector, createSelectorCreator, lruMemoize } from "reselect" +import { createSelector } from "reselect" import { createCachedSelector } from "re-reselect" -import { isEqual } from "lodash" +import { cachedByIdDeepEqual } from "@/common/selectors" import { insertOne, prepareAfterAdd, updateOne } from "@/common/slice" import { selectState } from "@/features/app/appSlice" import EffectLayer from "./EffectLayer" @@ -150,9 +150,4 @@ export const selectEffectSelectionVertices = createCachedSelector( const instance = new EffectLayer(effect.type) return instance.getSelectionVertices(effect) }, -)({ - keySelector: (state, id) => id, - selectorCreator: createSelectorCreator(lruMemoize, { - equalityCheck: isEqual, - }), -}) +)(cachedByIdDeepEqual) diff --git a/src/features/layers/layersSlice.js b/src/features/layers/layersSlice.js index 35074a55..9091cc9f 100644 --- a/src/features/layers/layersSlice.js +++ b/src/features/layers/layersSlice.js @@ -1,12 +1,16 @@ /* global localStorage */ import { createSlice, createEntityAdapter } from "@reduxjs/toolkit" -import { createSelector, createSelectorCreator, lruMemoize } from "reselect" +import { createSelector } from "reselect" import { createCachedSelector } from "re-reselect" import { v4 as uuidv4 } from "uuid" import Color from "color" import { arrayMoveImmutable } from "array-move" -import { isEqual } from "lodash" +import { + cachedByIdDeepEqual, + createDeepEqualSelector, + createResultEqualSelector, +} from "@/common/selectors" import { rotate, offset, @@ -193,6 +197,12 @@ const layersSlice = createSlice({ // used in slice const { selectById } = adapter.getSelectors((state) => state.layers) +// computes vertices for a layer with given effects and machine +const computeLayerVertices = (layer, effects, machine, options) => { + const instance = new Layer(layer.type) + return instance.getVertices({ layer, effects, machine, options }) +} + // returns vertices suitable for display in the preview window const previewVertices = (vertices, layer) => { return vertices.map((vertex) => { @@ -380,15 +390,9 @@ export const selectLayerVertices = createCachedSelector( return [] } // zombie child - const instance = new Layer(layer.type) - return instance.getVertices({ layer, effects, machine }) + return computeLayerVertices(layer, effects, machine) }, -)({ - keySelector: (state, id) => id, - selectorCreator: createSelectorCreator(lruMemoize, { - equalityCheck: isEqual, - }), -}) +)(cachedByIdDeepEqual) // returns the machine-bound vertices for a given layer const selectMachineVertices = createCachedSelector( @@ -428,21 +432,10 @@ export const selectShapePreviewVertices = createCachedSelector( return [] } // zombie child - const instance = new Layer(layer.type) - const vertices = instance.getVertices({ - layer, - effects: [], - machine, - }) - + const vertices = computeLayerVertices(layer, [], machine) return previewVertices(vertices, layer) }, -)({ - keySelector: (state, id) => id, - selectorCreator: createSelectorCreator(lruMemoize, { - equalityCheck: isEqual, - }), -}) +)(cachedByIdDeepEqual) // creates a selector that returns previewable vertices for a given layer export const selectPreviewVertices = createCachedSelector( @@ -472,31 +465,22 @@ export const selectDraggingEffectVertices = createCachedSelector( return [] } // zombie child - const instance = new Layer(layer.type) const effect = effects.find((effect) => effect.id == effectId) - if (!effect) { - // no longer visible return [] } const effectInstance = new EffectLayer(effect.type) const idx = effects.findIndex((effect) => effect.id == effectId) const vertices = effectInstance.model.dragPreview - ? instance.getVertices({ - layer, - effects: effects.slice(0, idx + 1), - machine, - }) + ? computeLayerVertices(layer, effects.slice(0, idx + 1), machine) : [] return previewVertices(previewVertices(vertices, layer), effect) }, )({ keySelector: (state, id, effectId) => id + effectId, - selectorCreator: createSelectorCreator(lruMemoize, { - equalityCheck: isEqual, - }), + selectorCreator: createDeepEqualSelector, }) // returns the preview vertices for a layer when an effect is being dragged; this includes all @@ -511,28 +495,19 @@ export const selectShapeWhileEffectDraggingVertices = createCachedSelector( return [] } // zombie child or inactive effect - const instance = new Layer(layer.type) const effect = effects.find((effect) => effect.id == effectId) - if (!effect) { - // no longer visible return [] } const idx = effects.findIndex((effect) => effect.id == effectId) - const vertices = instance.getVertices({ - layer, - effects: effects.slice(0, idx), - machine, - }) + const vertices = computeLayerVertices(layer, effects.slice(0, idx), machine) return previewVertices(vertices, layer) }, )({ keySelector: (state, id, effectId) => id + effectId, - selectorCreator: createSelectorCreator(lruMemoize, { - equalityCheck: isEqual, - }), + selectorCreator: createDeepEqualSelector, }) // returns whether an upstream effect from the given effect is being dragged @@ -556,77 +531,80 @@ export const selectIsUpstreamEffectDragging = createCachedSelector( return idx > draggingIdx }, -)({ - keySelector: (state, id) => id, - selectorCreator: createSelectorCreator(lruMemoize, { - equalityCheck: isEqual, - }), -}) +)(cachedByIdDeepEqual) // returns a array of all visible machine-bound vertices and the connections between them -export const selectConnectedVertices = createSelector(selectState, (state) => { - if (!selectFontsLoaded(state)) { - return [] - } +export const selectConnectedVertices = createResultEqualSelector( + selectFontsLoaded, + selectVisibleLayerIds, + selectState, + (fontsLoaded, visibleLayerIds, state) => { + if (!fontsLoaded) { + return [] + } - log("selectConnectedVertices") - const visibleLayerIds = selectVisibleLayerIds(state) + log("selectConnectedVertices") - return visibleLayerIds.reduce((acc, id) => { - const vertices = selectMachineVertices(state, id) - const connector = selectConnectingVertices(state, id) + return visibleLayerIds.reduce((acc, id) => { + const vertices = selectMachineVertices(state, id) + const connector = selectConnectingVertices(state, id) - acc.push(...vertices, ...connector.slice(1, -1)) + acc.push(...vertices, ...connector.slice(1, -1)) - return acc - }, []) -}) + return acc + }, []) + }, +) // returns an array of layers (and connectors) in an object structure designed to be exported by // an exporter -export const selectLayersForExport = createSelector(selectState, (state) => { - if (!selectFontsLoaded(state)) { - return [] - } +export const selectLayersForExport = createResultEqualSelector( + selectFontsLoaded, + selectVisibleLayerIds, + selectState, + (fontsLoaded, visibleLayerIds, state) => { + if (!fontsLoaded) { + return [] + } - log("selectLayersForExport") - const visibleLayerIds = selectVisibleLayerIds(state) - let connectorCnt = 0 - - return visibleLayerIds.reduce((acc, id, index) => { - const vertices = selectMachineVertices(state, id) - const connector = selectConnectingVertices(state, id) - const effects = selectEffectsByLayerId(state, id) - const info = selectLayerById(state, id) - const codeEffects = effects.filter( - (effect) => effect.type === "programCode", - ) + log("selectLayersForExport") + let connectorCnt = 0 - acc.push({ - name: info.name, - type: "LAYER", - index, - vertices, - code: codeEffects.map((effect) => { - return { - pre: effect.programCodePre, - post: effect.programCodePost, - } - }), - }) + return visibleLayerIds.reduce((acc, id, index) => { + const vertices = selectMachineVertices(state, id) + const connector = selectConnectingVertices(state, id) + const effects = selectEffectsByLayerId(state, id) + const info = selectLayerById(state, id) + const codeEffects = effects.filter( + (effect) => effect.type === "programCode", + ) - if (connector.length > 0) { acc.push({ - type: "CONNECTOR", - index: connectorCnt, - vertices: connector, + name: info.name, + type: "LAYER", + index, + vertices, + code: codeEffects.map((effect) => { + return { + pre: effect.programCodePre, + post: effect.programCodePost, + } + }), }) - connectorCnt += 1 - } - return acc - }, []) -}) + if (connector.length > 0) { + acc.push({ + type: "CONNECTOR", + index: connectorCnt, + vertices: connector, + }) + connectorCnt += 1 + } + + return acc + }, []) + }, +) // returns an array of vertices connecting a given layer to the next (if it exists) export const selectConnectingVertices = createCachedSelector( @@ -680,28 +658,31 @@ export const selectConnectingVertices = createCachedSelector( )((state, id) => id) // returns the starting offset for each layer, given previous layers -export const selectVertexOffsets = createSelector(selectState, (state) => { - log("selectVertexOffsets") - const visibleLayerIds = selectVisibleLayerIds(state) - let offsets = {} - let offset = 0 - - visibleLayerIds.forEach((id) => { - const vertices = selectMachineVertices(state, id) - const connector = selectConnectingVertices(state, id) - offsets[id] = { start: offset, end: offset + vertices.length - 1 } - - if (connector.length > 0) { - offsets[id + "-connector"] = { - start: offset + vertices.length, - end: offset + vertices.length + connector.length - 1, +export const selectVertexOffsets = createResultEqualSelector( + selectVisibleLayerIds, + selectState, + (visibleLayerIds, state) => { + log("selectVertexOffsets") + let offsets = {} + let offset = 0 + + visibleLayerIds.forEach((id) => { + const vertices = selectMachineVertices(state, id) + const connector = selectConnectingVertices(state, id) + offsets[id] = { start: offset, end: offset + vertices.length - 1 } + + if (connector.length > 0) { + offsets[id + "-connector"] = { + start: offset + vertices.length, + end: offset + vertices.length + connector.length - 1, + } + offset += vertices.length + connector.length } - offset += vertices.length + connector.length - } - }) + }) - return offsets -}) + return offsets + }, +) // returns statistics across all layers export const selectVerticesStats = createSelector( @@ -725,11 +706,9 @@ export const selectLayerPreviewBounds = createCachedSelector( (state, id, isCurrent) => isCurrent, (layer, machineVertices, effects, machine, isCurrent) => { if (!layer) { - // zombie child return [] - } + } // zombie child - const instance = new Layer(layer.type) const hasSelectableEffect = effects.find((effect) => ["transformer", "mask"].includes(effect.type), ) @@ -740,12 +719,7 @@ export const selectLayerPreviewBounds = createCachedSelector( !(isCurrent || hasInvertedMask) || !hasSelectableEffect const vertices = includeLayer - ? instance.getVertices({ - layer, - effects: [], - machine, - options: { bounds: true }, - }) + ? computeLayerVertices(layer, [], machine, { bounds: true }) : [] const effectVertices = includeEffects ? machineVertices : [] @@ -757,9 +731,7 @@ export const selectLayerPreviewBounds = createCachedSelector( }, )({ keySelector: (state, id, isCurrent) => `${id}-${isCurrent}`, - selectorCreator: createSelectorCreator(lruMemoize, { - equalityCheck: isEqual, - }), + selectorCreator: createDeepEqualSelector, }) // given a set of vertices and a slider value, returns the indices of the diff --git a/src/features/layers/layersSlice.spec.js b/src/features/layers/layersSlice.spec.js index e18b388a..ef199bbe 100644 --- a/src/features/layers/layersSlice.spec.js +++ b/src/features/layers/layersSlice.spec.js @@ -20,6 +20,8 @@ import layersReducer, { setSelectedLayer, updateLayer, selectLayerVertices, + selectConnectedVertices, + selectVerticesStats, } from "./layersSlice" import effectsReducer, { updateEffect } from "@/features/effects/effectsSlice" import machinesReducer from "@/features/machines/machinesSlice" @@ -687,4 +689,66 @@ describe("layers selectors", () => { expect(selectLayerVertices.recomputations()).toBe(2) }) }) + + describe("selectConnectedVertices reference stability", () => { + let store + beforeEach(() => { + store = configureStore({ + reducer: { + effects: effectsReducer, + layers: layersReducer, + machines: machinesReducer, + fonts: fontsReducer, + images: imagesReducer, + }, + preloadedState: { + ...initialState, + fonts: { loaded: true }, + }, + }) + resetUniqueId(3) + selectVerticesStats.resetRecomputations() + }) + + it("should return same reference when vertices unchanged", () => { + // First call + const result1 = selectConnectedVertices(store.getState()) + + // Dispatch unrelated change (layer name doesn't affect vertices) + store.dispatch(updateLayer({ id: "A", name: "renamed" })) + + // Second call after state change + const result2 = selectConnectedVertices(store.getState()) + + // Reference should be the same (result equality check) + expect(result1).toBe(result2) + }) + + it("should not recompute downstream selectors when reference stable", () => { + // First call to downstream selector + selectVerticesStats(store.getState()) + expect(selectVerticesStats.recomputations()).toBe(1) + + // Dispatch unrelated change + store.dispatch(updateLayer({ id: "A", name: "renamed" })) + + // Second call - should NOT recompute because input reference is stable + selectVerticesStats(store.getState()) + expect(selectVerticesStats.recomputations()).toBe(1) + }) + + it("should return new reference when vertices actually change", () => { + const result1 = selectConnectedVertices(store.getState()) + const initialLength = result1.length + + // Hide a layer - this definitely changes the output + store.dispatch(updateLayer({ id: "A", visible: false })) + + const result2 = selectConnectedVertices(store.getState()) + + // Reference should be different (fewer vertices now) + expect(result1).not.toBe(result2) + expect(result2.length).toBeLessThan(initialLength) + }) + }) }) From 570e9c73615a30b59c75ef29ffdb64d4e1d032f1 Mon Sep 17 00:00:00 2001 From: Bob Carmichael Date: Wed, 21 Jan 2026 06:21:19 -0500 Subject: [PATCH 2/3] a few more optimizations --- src/features/layers/layersSlice.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/features/layers/layersSlice.js b/src/features/layers/layersSlice.js index 9091cc9f..7e7213b0 100644 --- a/src/features/layers/layersSlice.js +++ b/src/features/layers/layersSlice.js @@ -296,7 +296,7 @@ export const selectSelectedLayer = createSelector( }, ) -export const selectVisibleLayerIds = createSelector( +export const selectVisibleLayerIds = createResultEqualSelector( [selectLayerIds, selectLayerEntities], (layerIds, layers) => { return layerIds.filter((id) => layers[id].visible) @@ -375,9 +375,12 @@ export const selectActiveEffect = createCachedSelector( }, )((state, id) => id) -export const selectAllImageIds = createSelector([selectAllLayers], (layers) => { - return layers.map((layer) => layer.imageId).filter((id) => id) -}) +export const selectAllImageIds = createResultEqualSelector( + [selectAllLayers], + (layers) => { + return layers.map((layer) => layer.imageId).filter((id) => id) + }, +) // returns the vertices for a given layer export const selectLayerVertices = createCachedSelector( From 6f48aba4d2cd23b4b41f4acd73fbce3523410e14 Mon Sep 17 00:00:00 2001 From: Bob Carmichael Date: Wed, 21 Jan 2026 06:32:00 -0500 Subject: [PATCH 3/3] add missing file --- src/common/selectors.js | 43 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 src/common/selectors.js diff --git a/src/common/selectors.js b/src/common/selectors.js new file mode 100644 index 00000000..1fc25a3a --- /dev/null +++ b/src/common/selectors.js @@ -0,0 +1,43 @@ +import { createSelectorCreator, lruMemoize } from "reselect" +import { isEqual } from "lodash" + +// Compares inputs deeply before deciding whether to recompute. +// Default reselect uses === for inputs, so a new object reference triggers +// recomputation even if the contents are identical. This version uses deep +// equality, skipping expensive computation when inputs are structurally equal. +// Example: selectLayerVertices receives a layer object. If another selector +// returns a new layer reference with identical values, we skip vertex +// computation entirely. +export const createDeepEqualSelector = createSelectorCreator(lruMemoize, { + equalityCheck: isEqual, +}) + +// Compares result deeply before deciding whether to return a new reference. +// The selector always runs, but if the new result deep-equals the cached +// result, the cached reference is returned. This preserves referential +// equality for downstream consumers (React, other selectors). +// Use for aggregation selectors that combine data from multiple sources, +// where the result often stays the same even when inputs change. +// Example: selectConnectedVertices collects vertices from all visible layers. +// When the user renames a layer, state changes and the selector runs, but +// the vertices are identical. Returning the cached array reference prevents +// React from re-rendering the canvas. +export const createResultEqualSelector = createSelectorCreator(lruMemoize, { + resultEqualityCheck: isEqual, +}) + +// re-reselect config for cached selectors keyed by id with deep input comparison. +// Standard reselect only caches the most recent call. re-reselect's +// createCachedSelector maintains separate caches per key, so selecting +// layer "A" then layer "B" then layer "A" again hits the cache for "A". +// The keySelector extracts the cache key from selector arguments. +// Combined with createDeepEqualSelector, this skips recomputation when +// a cached entry exists AND its inputs are deep-equal to the current inputs. +// Example: selectLayerVertices(state, "layer-1") caches vertices for layer-1. +// Selecting a different layer doesn't evict this cache. When layer-1 is +// selected again, deep comparison of inputs determines if recomputation +// is needed. +export const cachedByIdDeepEqual = { + keySelector: (state, id) => id, + selectorCreator: createDeepEqualSelector, +}