From 54d474c486213e7c046e56cc72cfc245ac1640d4 Mon Sep 17 00:00:00 2001 From: Yogesh Kumar Baljeet Singh Date: Tue, 11 Mar 2025 11:18:38 +0100 Subject: [PATCH 01/15] Add memoization for invoking callbacks --- .../lib/src/actions/handleHostStoreChange.ts | 27 ++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts b/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts index 31c7c55b..e453b08f 100644 --- a/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts +++ b/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts @@ -20,6 +20,8 @@ export interface PropertyRef extends ContribRef, CallbackRef, InputRef { property: string; } +const processedinputValuess = new Map(); + export function handleHostStoreChange() { const { extensions, configuration, contributionsRecord } = store.getState(); const { hostStore } = configuration; @@ -43,8 +45,11 @@ export function handleHostStoreChange() { contributionsRecord, hostStore, ); - if (callbackRequests && callbackRequests.length > 0) { - invokeCallbacks(callbackRequests); + const filtered_callbackRequests = callbackRequests.filter( + (req): req is CallbackRequest => req !== undefined, + ); + if (filtered_callbackRequests && filtered_callbackRequests.length > 0) { + invokeCallbacks(filtered_callbackRequests); } } @@ -52,7 +57,9 @@ function getCallbackRequests( propertyRefs: PropertyRef[], contributionsRecord: Record, hostStore: HostStore, -): CallbackRequest[] { +): (CallbackRequest | undefined)[] { + const { configuration } = store.getState(); + const loggingEnabled = configuration.logging?.enabled; return propertyRefs.map((propertyRef) => { const contributions = contributionsRecord[propertyRef.contribPoint]; const contribution = contributions[propertyRef.contribIndex]; @@ -62,6 +69,20 @@ function getCallbackRequests( contribution, hostStore, ); + const serializedInputValues = JSON.stringify(inputValues); + if (processedinputValuess.has(serializedInputValues)) { + if (loggingEnabled) { + console.groupCollapsed( + "Skipping this callback as no state has changed!", + ); + console.log("propertyRef", propertyRef); + console.log("inputValues", inputValues); + console.groupEnd(); + } + return; + } else { + processedinputValuess.set(serializedInputValues, true); + } return { ...propertyRef, inputValues }; }); } From 164dc487ace183038b0e82ef8e8b6b079e49066e Mon Sep 17 00:00:00 2001 From: Yogesh Kumar Baljeet Singh Date: Tue, 11 Mar 2025 14:26:50 +0100 Subject: [PATCH 02/15] Revert --- .../lib/src/actions/handleHostStoreChange.ts | 28 +++---------------- 1 file changed, 4 insertions(+), 24 deletions(-) diff --git a/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts b/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts index e453b08f..d53f0ad4 100644 --- a/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts +++ b/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts @@ -20,8 +20,6 @@ export interface PropertyRef extends ContribRef, CallbackRef, InputRef { property: string; } -const processedinputValuess = new Map(); - export function handleHostStoreChange() { const { extensions, configuration, contributionsRecord } = store.getState(); const { hostStore } = configuration; @@ -45,11 +43,9 @@ export function handleHostStoreChange() { contributionsRecord, hostStore, ); - const filtered_callbackRequests = callbackRequests.filter( - (req): req is CallbackRequest => req !== undefined, - ); - if (filtered_callbackRequests && filtered_callbackRequests.length > 0) { - invokeCallbacks(filtered_callbackRequests); + + if (callbackRequests && callbackRequests.length > 0) { + invokeCallbacks(callbackRequests); } } @@ -57,9 +53,7 @@ function getCallbackRequests( propertyRefs: PropertyRef[], contributionsRecord: Record, hostStore: HostStore, -): (CallbackRequest | undefined)[] { - const { configuration } = store.getState(); - const loggingEnabled = configuration.logging?.enabled; +): CallbackRequest[] { return propertyRefs.map((propertyRef) => { const contributions = contributionsRecord[propertyRef.contribPoint]; const contribution = contributions[propertyRef.contribIndex]; @@ -69,20 +63,6 @@ function getCallbackRequests( contribution, hostStore, ); - const serializedInputValues = JSON.stringify(inputValues); - if (processedinputValuess.has(serializedInputValues)) { - if (loggingEnabled) { - console.groupCollapsed( - "Skipping this callback as no state has changed!", - ); - console.log("propertyRef", propertyRef); - console.log("inputValues", inputValues); - console.groupEnd(); - } - return; - } else { - processedinputValuess.set(serializedInputValues, true); - } return { ...propertyRef, inputValues }; }); } From e6f239cd7d7178360eb88268d1abb91942b58179 Mon Sep 17 00:00:00 2001 From: Yogesh Kumar Baljeet Singh Date: Tue, 11 Mar 2025 17:35:52 +0100 Subject: [PATCH 03/15] Add fast-memoize --- chartlets.js/package-lock.json | 9 ++++++++- chartlets.js/packages/lib/package.json | 1 + 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/chartlets.js/package-lock.json b/chartlets.js/package-lock.json index ae12139d..b3214816 100644 --- a/chartlets.js/package-lock.json +++ b/chartlets.js/package-lock.json @@ -4352,6 +4352,12 @@ "dev": true, "license": "MIT" }, + "node_modules/fast-memoize": { + "version": "2.5.2", + "resolved": "https://registry.npmjs.org/fast-memoize/-/fast-memoize-2.5.2.tgz", + "integrity": "sha512-Ue0LwpDYErFbmNnZSF0UH6eImUwDmogUO1jyE+JbN2gsQz/jICm1Ve7t9QT0rNSsfJt+Hs4/S3GnsDVjL4HVrw==", + "license": "MIT" + }, "node_modules/fast-uri": { "version": "3.0.3", "resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.0.3.tgz", @@ -8275,6 +8281,7 @@ "version": "0.1.4", "license": "MIT", "dependencies": { + "fast-memoize": "^2.5.2", "microdiff": "^1.4", "zustand": "^5.0" }, @@ -8322,4 +8329,4 @@ } } } -} \ No newline at end of file +} diff --git a/chartlets.js/packages/lib/package.json b/chartlets.js/packages/lib/package.json index cc196cdc..e745513e 100644 --- a/chartlets.js/packages/lib/package.json +++ b/chartlets.js/packages/lib/package.json @@ -55,6 +55,7 @@ "preview": "vite preview" }, "dependencies": { + "fast-memoize": "^2.5.2", "microdiff": "^1.4", "zustand": "^5.0" }, From 0a75265c4672f4c78897ebf1113f8707184ef37d Mon Sep 17 00:00:00 2001 From: Yogesh Kumar Baljeet Singh Date: Tue, 11 Mar 2025 17:37:00 +0100 Subject: [PATCH 04/15] implement memoizing and avoid invoking callbacks when not required --- .../lib/src/actions/handleHostStoreChange.ts | 40 +++++++++++++++++-- .../lib/src/actions/helpers/getInputValues.ts | 5 ++- .../packages/lib/src/types/state/store.ts | 5 +++ .../packages/lib/src/utils/compare.ts | 8 ++++ 4 files changed, 53 insertions(+), 5 deletions(-) create mode 100644 chartlets.js/packages/lib/src/utils/compare.ts diff --git a/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts b/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts index d53f0ad4..eb4cf4d7 100644 --- a/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts +++ b/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts @@ -11,6 +11,7 @@ import { invokeCallbacks } from "@/actions/helpers/invokeCallbacks"; import type { ContributionState } from "@/types/state/contribution"; import type { HostStore } from "@/types/state/host"; import { store } from "@/store"; +import { shallowEqualArrays } from "@/utils/compare"; /** * A reference to a property of an input of a callback of a contribution. @@ -18,6 +19,8 @@ import { store } from "@/store"; export interface PropertyRef extends ContribRef, CallbackRef, InputRef { /** The property. */ property: string; + /** Property ID for memoization */ + id: string; } export function handleHostStoreChange() { @@ -44,16 +47,23 @@ export function handleHostStoreChange() { hostStore, ); - if (callbackRequests && callbackRequests.length > 0) { - invokeCallbacks(callbackRequests); + const filteredCallbackRequests = callbackRequests.filter( + (callbackRequest): callbackRequest is CallbackRequest => + callbackRequest !== undefined, + ); + if (filteredCallbackRequests && filteredCallbackRequests.length > 0) { + invokeCallbacks(filteredCallbackRequests); } } -function getCallbackRequests( +// Exporting for testing only +export function getCallbackRequests( propertyRefs: PropertyRef[], contributionsRecord: Record, hostStore: HostStore, -): CallbackRequest[] { +): (CallbackRequest | undefined)[] { + const { configuration, lastInputValues } = store.getState(); + const { logging } = configuration; return propertyRefs.map((propertyRef) => { const contributions = contributionsRecord[propertyRef.contribPoint]; const contribution = contributions[propertyRef.contribIndex]; @@ -63,6 +73,27 @@ function getCallbackRequests( contribution, hostStore, ); + const propRefId = propertyRef.id; + if ( + lastInputValues?.[propRefId] && + shallowEqualArrays(lastInputValues?.[propRefId], inputValues) + ) { + // Skip adding the inputValues if memoized values are returned. + if (logging?.enabled) { + console.groupCollapsed("Skipping callback request"); + console.log("inputValues", inputValues); + console.groupEnd(); + } + return; + } + if (lastInputValues) { + lastInputValues[propRefId] = inputValues; + store.setState({ + lastInputValues: { ...lastInputValues }, + }); + } else { + store.setState({ lastInputValues: { [propRefId]: inputValues } }); + } return { ...propertyRef, inputValues }; }); } @@ -92,6 +123,7 @@ function getHostStorePropertyRefs(): PropertyRef[] { callbackIndex, inputIndex, property: formatObjPath(input.property), + id: `${contribPoint}-${contribIndex}-${callbackIndex}-${inputIndex}`, }); } }), diff --git a/chartlets.js/packages/lib/src/actions/helpers/getInputValues.ts b/chartlets.js/packages/lib/src/actions/helpers/getInputValues.ts index 544a7e10..131e90a9 100644 --- a/chartlets.js/packages/lib/src/actions/helpers/getInputValues.ts +++ b/chartlets.js/packages/lib/src/actions/helpers/getInputValues.ts @@ -13,6 +13,7 @@ import { import { formatObjPath, getValue, type ObjPathLike } from "@/utils/objPath"; import { isObject } from "@/utils/isObject"; import type { HostStore } from "@/types/state/host"; +import memoize from "fast-memoize"; export function getInputValues( inputs: Input[], @@ -26,7 +27,9 @@ export function getInputValues( const noValue = {}; -export function getInputValue( +export const getInputValue = memoize(_getInputValue); + +function _getInputValue( input: Input, contributionState: ContributionState, hostStore?: HostStore, diff --git a/chartlets.js/packages/lib/src/types/state/store.ts b/chartlets.js/packages/lib/src/types/state/store.ts index 5edb8366..6dc7482a 100644 --- a/chartlets.js/packages/lib/src/types/state/store.ts +++ b/chartlets.js/packages/lib/src/types/state/store.ts @@ -25,4 +25,9 @@ export interface StoreState { * See hook `useThemeMode()`. */ themeMode?: ThemeMode; + /** + * Store last input values for callback requests to avoid invoking them if + * there are no changes + * */ + lastInputValues?: Record; } diff --git a/chartlets.js/packages/lib/src/utils/compare.ts b/chartlets.js/packages/lib/src/utils/compare.ts new file mode 100644 index 00000000..184bb0cc --- /dev/null +++ b/chartlets.js/packages/lib/src/utils/compare.ts @@ -0,0 +1,8 @@ +export function shallowEqualArrays( + arr1?: unknown[], + arr2?: unknown[], +): boolean { + if (!arr1 || !arr2) return false; + if (arr1.length !== arr2.length) return false; + return arr1.every((val, index) => val === arr2[index]); +} From 7bc63f115d334e730f89df427e131e6bb9db6a1b Mon Sep 17 00:00:00 2001 From: Yogesh Kumar Baljeet Singh Date: Tue, 11 Mar 2025 17:37:06 +0100 Subject: [PATCH 05/15] add tests --- .../actions/handleHostStoreChanges.test.tsx | 97 ++++++++++++++++++- .../packages/lib/src/utils/compare.test.ts | 16 +++ 2 files changed, 111 insertions(+), 2 deletions(-) create mode 100644 chartlets.js/packages/lib/src/utils/compare.test.ts diff --git a/chartlets.js/packages/lib/src/actions/handleHostStoreChanges.test.tsx b/chartlets.js/packages/lib/src/actions/handleHostStoreChanges.test.tsx index 49877aca..e514cf33 100644 --- a/chartlets.js/packages/lib/src/actions/handleHostStoreChanges.test.tsx +++ b/chartlets.js/packages/lib/src/actions/handleHostStoreChanges.test.tsx @@ -1,6 +1,12 @@ -import { describe, it, expect, beforeEach } from "vitest"; +import { beforeEach, describe, expect, it } from "vitest"; import { store } from "@/store"; -import { handleHostStoreChange } from "./handleHostStoreChange"; +import { + getCallbackRequests, + handleHostStoreChange, + type PropertyRef, +} from "./handleHostStoreChange"; +import type { ContribPoint } from "@/types/model/extension"; +import type { ContributionState } from "@/types/state/contribution"; describe("handleHostStoreChange", () => { let listeners: (() => void)[] = []; @@ -15,10 +21,12 @@ describe("handleHostStoreChange", () => { listeners.push(_l); }, }; + let lastInputValues: Record = {}; beforeEach(() => { listeners = []; hostState = {}; + lastInputValues = {}; }); it("should do nothing without host store", () => { @@ -82,4 +90,89 @@ describe("handleHostStoreChange", () => { hostStore.set("variableName", "CHL"); handleHostStoreChange(); }); + + it("should memoize second call with same arguments", () => { + const extensions = [{ name: "e0", version: "0", contributes: ["panels"] }]; + store.setState({ + configuration: { hostStore, logging: { enabled: false } }, + extensions, + contributionsResult: { + status: "ok", + data: { + extensions, + contributions: { + panels: [ + { + name: "ext.p1", + extension: "ext", + layout: { + function: { + name: "layout", + parameters: [], + return: {}, + }, + inputs: [], + outputs: [], + }, + callbacks: [ + { + function: { + name: "callback", + parameters: [], + return: {}, + }, + inputs: [{ id: "@app", property: "variableName" }], + outputs: [{ id: "select", property: "value" }], + }, + ], + initialState: {}, + }, + ], + }, + }, + }, + lastInputValues: lastInputValues, + }); + hostStore.set("variableName", "CHL"); + const propertyRefs: PropertyRef[] = [ + { + id: "panel-0-0-0", + contribPoint: "panel", + contribIndex: 0, + callbackIndex: 0, + property: "value", + inputIndex: 0, + }, + ]; + const contributionsRecord: Record = { + panel: [ + { + name: "ext.p1", + container: { title: "Panel A" }, + extension: "ext", + componentResult: {}, + initialState: { title: "Panel A" }, + callbacks: [ + { + function: { + name: "callback", + parameters: [{ name: "param1" }], + return: {}, + }, + inputs: [{ id: "@app", property: "variableName" }], + }, + ], + }, + ], + }; + const result = getCallbackRequests( + propertyRefs, + contributionsRecord, + hostStore, + ); + expect(result[0]).toEqual({ + ...propertyRefs[0], + inputValues: ["CHL"], + }); + }); }); diff --git a/chartlets.js/packages/lib/src/utils/compare.test.ts b/chartlets.js/packages/lib/src/utils/compare.test.ts new file mode 100644 index 00000000..1be9f808 --- /dev/null +++ b/chartlets.js/packages/lib/src/utils/compare.test.ts @@ -0,0 +1,16 @@ +import { describe, expect, it } from "vitest"; +import { shallowEqualArrays } from "@/utils/compare"; + +describe("Test shallowEqualArrays()", () => { + const arr_a: string[] = ["a", "b", "c"]; + const arr_b: string[] = ["a", "b", "c"]; + const arr_c: string[] = ["a", "b", "d"]; + const arr_d: string[] = []; + const arr_e: string[] = ["a", "b", "c", "d"]; + it("works", () => { + expect(shallowEqualArrays(arr_a, arr_b)).toBe(true); + expect(shallowEqualArrays(arr_a, arr_c)).toBe(false); + expect(shallowEqualArrays(arr_a, arr_d)).toBe(false); + expect(shallowEqualArrays(arr_a, arr_e)).toBe(false); + }); +}); From 2f8afc145e99d7b58548b6ce589ac5cecff20a49 Mon Sep 17 00:00:00 2001 From: Yogesh Kumar Baljeet Singh Date: Wed, 12 Mar 2025 11:13:15 +0100 Subject: [PATCH 06/15] update tests --- .../packages/lib/src/utils/compare.test.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/chartlets.js/packages/lib/src/utils/compare.test.ts b/chartlets.js/packages/lib/src/utils/compare.test.ts index 1be9f808..498e9340 100644 --- a/chartlets.js/packages/lib/src/utils/compare.test.ts +++ b/chartlets.js/packages/lib/src/utils/compare.test.ts @@ -7,10 +7,28 @@ describe("Test shallowEqualArrays()", () => { const arr_c: string[] = ["a", "b", "d"]; const arr_d: string[] = []; const arr_e: string[] = ["a", "b", "c", "d"]; + const arr_f: (string | null)[] = ["a", "b", "c", null]; + const arr_g: (string | null)[] = ["a", "b", "c", null]; + const arr_h = [1, [1, 2, 3], [4, 5, 6]]; + const arr_i = [1, [1, 2, 3], [4, 5, 6]]; + const arr_j: (number | string | null)[] = [1, 2, "c", null]; + const arr_k: (number | string | null)[] = [1, 3, "c", null]; + const arr_l: (number | string | null)[] = [1, 2, "c", null]; + const arr_m: number[] = [1, 2]; + const arr_n: number[] = [1, 2]; + const arr_o: null[] = [null]; + const arr_p: null[] = [null]; it("works", () => { expect(shallowEqualArrays(arr_a, arr_b)).toBe(true); expect(shallowEqualArrays(arr_a, arr_c)).toBe(false); expect(shallowEqualArrays(arr_a, arr_d)).toBe(false); expect(shallowEqualArrays(arr_a, arr_e)).toBe(false); + expect(shallowEqualArrays(arr_f, arr_g)).toBe(true); + expect(shallowEqualArrays(arr_h, arr_i)).toBe(false); + expect(shallowEqualArrays(arr_j, arr_k)).toBe(false); + expect(shallowEqualArrays(arr_j, arr_l)).toBe(true); + expect(shallowEqualArrays(arr_m, arr_n)).toBe(true); + expect(shallowEqualArrays(arr_m, arr_l)).toBe(false); + expect(shallowEqualArrays(arr_o, arr_p)).toBe(true); }); }); From 650bbf7dd0d7f11baed3bc99f70e0221a04c4783 Mon Sep 17 00:00:00 2001 From: Yogesh Kumar Baljeet Singh Date: Wed, 12 Mar 2025 11:13:32 +0100 Subject: [PATCH 07/15] update store and memoized function --- .../packages/lib/src/actions/helpers/getInputValues.ts | 8 ++++---- chartlets.js/packages/lib/src/store.ts | 1 + 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/chartlets.js/packages/lib/src/actions/helpers/getInputValues.ts b/chartlets.js/packages/lib/src/actions/helpers/getInputValues.ts index 131e90a9..9c5f3748 100644 --- a/chartlets.js/packages/lib/src/actions/helpers/getInputValues.ts +++ b/chartlets.js/packages/lib/src/actions/helpers/getInputValues.ts @@ -15,7 +15,9 @@ import { isObject } from "@/utils/isObject"; import type { HostStore } from "@/types/state/host"; import memoize from "fast-memoize"; -export function getInputValues( +export const getInputValues = memoize(_getInputValues); + +export function _getInputValues( inputs: Input[], contributionState: ContributionState, hostStore?: HostStore, @@ -27,9 +29,7 @@ export function getInputValues( const noValue = {}; -export const getInputValue = memoize(_getInputValue); - -function _getInputValue( +function getInputValue( input: Input, contributionState: ContributionState, hostStore?: HostStore, diff --git a/chartlets.js/packages/lib/src/store.ts b/chartlets.js/packages/lib/src/store.ts index 89ca4e91..6901c81c 100644 --- a/chartlets.js/packages/lib/src/store.ts +++ b/chartlets.js/packages/lib/src/store.ts @@ -7,4 +7,5 @@ export const store = create(() => ({ extensions: [], contributionsResult: {}, contributionsRecord: {}, + lastInputValues: {}, })); From c0fa4c2530946be7629b845ff76de565f75640fa Mon Sep 17 00:00:00 2001 From: Yogesh Kumar Baljeet Singh Date: Wed, 12 Mar 2025 11:20:08 +0100 Subject: [PATCH 08/15] update CHANGES.md --- chartlets.js/CHANGES.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/chartlets.js/CHANGES.md b/chartlets.js/CHANGES.md index 43cafa89..fcd57ea5 100644 --- a/chartlets.js/CHANGES.md +++ b/chartlets.js/CHANGES.md @@ -3,6 +3,8 @@ * Add `multiple` property for `Select` component to enable the selection of multiple elements. The `default` mode is supported at the moment. +* Callbacks will now only be invoked when there’s an actual change in state, reducing unnecessary processing and + improving performance. ## Version 0.1.4 (from 2025/03/06) From 8c02307bcc534be075746436b26ff6fb744249c0 Mon Sep 17 00:00:00 2001 From: Yogesh Kumar Baljeet Singh Date: Thu, 13 Mar 2025 14:59:23 +0100 Subject: [PATCH 09/15] use shallow comparison and some refactoring --- .../lib/src/actions/handleHostStoreChange.ts | 85 ++++++++++++------- .../actions/handleHostStoreChanges.test.tsx | 59 +++++++++++-- .../lib/src/actions/helpers/getInputValues.ts | 5 +- chartlets.js/packages/lib/src/store.ts | 2 +- .../packages/lib/src/types/state/store.ts | 4 +- .../packages/lib/src/utils/compare.ts | 8 +- 6 files changed, 114 insertions(+), 49 deletions(-) diff --git a/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts b/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts index eb4cf4d7..a1fcb24c 100644 --- a/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts +++ b/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts @@ -12,6 +12,7 @@ import type { ContributionState } from "@/types/state/contribution"; import type { HostStore } from "@/types/state/host"; import { store } from "@/store"; import { shallowEqualArrays } from "@/utils/compare"; +import memoize from "fast-memoize"; /** * A reference to a property of an input of a callback of a contribution. @@ -62,42 +63,60 @@ export function getCallbackRequests( contributionsRecord: Record, hostStore: HostStore, ): (CallbackRequest | undefined)[] { - const { configuration, lastInputValues } = store.getState(); - const { logging } = configuration; - return propertyRefs.map((propertyRef) => { - const contributions = contributionsRecord[propertyRef.contribPoint]; - const contribution = contributions[propertyRef.contribIndex]; - const callback = contribution.callbacks![propertyRef.callbackIndex]; - const inputValues = getInputValues( - callback.inputs!, - contribution, + const { lastCallbackInputValues } = store.getState(); + return propertyRefs.map((propertyRef) => + getCallbackRequest( + propertyRef, + lastCallbackInputValues, + contributionsRecord, hostStore, - ); - const propRefId = propertyRef.id; - if ( - lastInputValues?.[propRefId] && - shallowEqualArrays(lastInputValues?.[propRefId], inputValues) - ) { - // Skip adding the inputValues if memoized values are returned. - if (logging?.enabled) { - console.groupCollapsed("Skipping callback request"); - console.log("inputValues", inputValues); - console.groupEnd(); - } - return; - } - if (lastInputValues) { - lastInputValues[propRefId] = inputValues; - store.setState({ - lastInputValues: { ...lastInputValues }, - }); - } else { - store.setState({ lastInputValues: { [propRefId]: inputValues } }); - } - return { ...propertyRef, inputValues }; - }); + ), + ); } +const getCallbackRequest = ( + propertyRef: PropertyRef, + lastCallbackInputValues: Record | undefined, + contributionsRecord: Record, + hostStore: HostStore, +) => { + const contributions = contributionsRecord[propertyRef.contribPoint]; + const contribution = contributions[propertyRef.contribIndex]; + const callback = contribution.callbacks![propertyRef.callbackIndex]; + const _inputValues = getInputValues( + callback.inputs!, + contribution, + hostStore, + ); + + // This is a dummy function created to memoize the _inputValues values + const _getInputValues = (_inputValues: unknown[]): unknown[] => { + return _inputValues; + }; + const memoizedInputValues = memoize(_getInputValues); + const inputValues = memoizedInputValues(_inputValues); + + const propRefId = propertyRef.id; + if (lastCallbackInputValues) { + const lastInputValues = lastCallbackInputValues[propRefId]; + if (lastInputValues && shallowEqualArrays(lastInputValues, inputValues)) { + // We no longer log, as the situation is quite common + // Enable error logging for debugging only: + // console.groupCollapsed("Skipping callback request"); + // console.debug("inputValues", inputValues); + // console.groupEnd(); + return undefined; + } + lastCallbackInputValues[propRefId] = inputValues; + store.setState({ + lastCallbackInputValues: { ...lastCallbackInputValues }, + }); + } else { + store.setState({ lastCallbackInputValues: { [propRefId]: inputValues } }); + } + return { ...propertyRef, inputValues }; +}; + // TODO: use a memoized selector to get hostStorePropertyRefs // Note that this will only be effective and once we split the // static contribution infos and dynamic contribution states. diff --git a/chartlets.js/packages/lib/src/actions/handleHostStoreChanges.test.tsx b/chartlets.js/packages/lib/src/actions/handleHostStoreChanges.test.tsx index e514cf33..ed4e9e3e 100644 --- a/chartlets.js/packages/lib/src/actions/handleHostStoreChanges.test.tsx +++ b/chartlets.js/packages/lib/src/actions/handleHostStoreChanges.test.tsx @@ -21,12 +21,12 @@ describe("handleHostStoreChange", () => { listeners.push(_l); }, }; - let lastInputValues: Record = {}; + let lastCallbackInputValues: Record = {}; beforeEach(() => { listeners = []; hostState = {}; - lastInputValues = {}; + lastCallbackInputValues = {}; }); it("should do nothing without host store", () => { @@ -86,13 +86,40 @@ describe("handleHostStoreChange", () => { }, }, }, + contributionsRecord: { + panel: [ + { + name: "p0", + container: { title: "Panel A" }, + extension: "e0", + componentResult: {}, + initialState: {}, + callbacks: [ + { + function: { + name: "callback", + parameters: [], + return: {}, + }, + inputs: [{ id: "@app", property: "variableName" }], + outputs: [{ id: "select", property: "value" }], + }, + ], + }, + ], + }, }); hostStore.set("variableName", "CHL"); handleHostStoreChange(); + + // calling it second time for coverage. No state change changes the + // control flow + handleHostStoreChange(); + // TODO: Update this test to assert the generated callback request }); it("should memoize second call with same arguments", () => { - const extensions = [{ name: "e0", version: "0", contributes: ["panels"] }]; + const extensions = [{ name: "ext", version: "0", contributes: ["panels"] }]; store.setState({ configuration: { hostStore, logging: { enabled: false } }, extensions, @@ -122,7 +149,6 @@ describe("handleHostStoreChange", () => { return: {}, }, inputs: [{ id: "@app", property: "variableName" }], - outputs: [{ id: "select", property: "value" }], }, ], initialState: {}, @@ -131,9 +157,8 @@ describe("handleHostStoreChange", () => { }, }, }, - lastInputValues: lastInputValues, + lastCallbackInputValues: lastCallbackInputValues, }); - hostStore.set("variableName", "CHL"); const propertyRefs: PropertyRef[] = [ { id: "panel-0-0-0", @@ -165,7 +190,11 @@ describe("handleHostStoreChange", () => { }, ], }; - const result = getCallbackRequests( + + hostStore.set("variableName", "CHL"); + + // first call -> should create callback request + let result = getCallbackRequests( propertyRefs, contributionsRecord, hostStore, @@ -174,5 +203,21 @@ describe("handleHostStoreChange", () => { ...propertyRefs[0], inputValues: ["CHL"], }); + + // second call -> memoized -> should not create callback request + result = getCallbackRequests(propertyRefs, contributionsRecord, hostStore); + expect(result).toEqual([undefined]); + + // Third call - change state -> should create callback request + hostStore.set("variableName", "TMP"); + result = getCallbackRequests(propertyRefs, contributionsRecord, hostStore); + expect(result[0]).toEqual({ + ...propertyRefs[0], + inputValues: ["TMP"], + }); + + // fourth call -> memoized -> should not invoke callback + result = getCallbackRequests(propertyRefs, contributionsRecord, hostStore); + expect(result).toEqual([undefined]); }); }); diff --git a/chartlets.js/packages/lib/src/actions/helpers/getInputValues.ts b/chartlets.js/packages/lib/src/actions/helpers/getInputValues.ts index 9c5f3748..ed248f8f 100644 --- a/chartlets.js/packages/lib/src/actions/helpers/getInputValues.ts +++ b/chartlets.js/packages/lib/src/actions/helpers/getInputValues.ts @@ -13,11 +13,8 @@ import { import { formatObjPath, getValue, type ObjPathLike } from "@/utils/objPath"; import { isObject } from "@/utils/isObject"; import type { HostStore } from "@/types/state/host"; -import memoize from "fast-memoize"; -export const getInputValues = memoize(_getInputValues); - -export function _getInputValues( +export function getInputValues( inputs: Input[], contributionState: ContributionState, hostStore?: HostStore, diff --git a/chartlets.js/packages/lib/src/store.ts b/chartlets.js/packages/lib/src/store.ts index 6901c81c..683ac9d3 100644 --- a/chartlets.js/packages/lib/src/store.ts +++ b/chartlets.js/packages/lib/src/store.ts @@ -7,5 +7,5 @@ export const store = create(() => ({ extensions: [], contributionsResult: {}, contributionsRecord: {}, - lastInputValues: {}, + lastCallbackInputValues: {}, })); diff --git a/chartlets.js/packages/lib/src/types/state/store.ts b/chartlets.js/packages/lib/src/types/state/store.ts index 6dc7482a..b1c25b90 100644 --- a/chartlets.js/packages/lib/src/types/state/store.ts +++ b/chartlets.js/packages/lib/src/types/state/store.ts @@ -27,7 +27,7 @@ export interface StoreState { themeMode?: ThemeMode; /** * Store last input values for callback requests to avoid invoking them if - * there are no changes + * there are no state changes * */ - lastInputValues?: Record; + lastCallbackInputValues?: Record; } diff --git a/chartlets.js/packages/lib/src/utils/compare.ts b/chartlets.js/packages/lib/src/utils/compare.ts index 184bb0cc..eb917bc9 100644 --- a/chartlets.js/packages/lib/src/utils/compare.ts +++ b/chartlets.js/packages/lib/src/utils/compare.ts @@ -2,7 +2,11 @@ export function shallowEqualArrays( arr1?: unknown[], arr2?: unknown[], ): boolean { - if (!arr1 || !arr2) return false; - if (arr1.length !== arr2.length) return false; + if (!arr1 || !arr2) { + return false; + } + if (arr1.length !== arr2.length) { + return false; + } return arr1.every((val, index) => val === arr2[index]); } From 70452a68aed50fa6683659f5cb8b96f4785112b5 Mon Sep 17 00:00:00 2001 From: Yogesh Kumar Baljeet Singh Date: Fri, 14 Mar 2025 09:09:52 +0100 Subject: [PATCH 10/15] increase test coverage --- chartlets.js/packages/lib/src/utils/compare.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/chartlets.js/packages/lib/src/utils/compare.test.ts b/chartlets.js/packages/lib/src/utils/compare.test.ts index 498e9340..5889c4b9 100644 --- a/chartlets.js/packages/lib/src/utils/compare.test.ts +++ b/chartlets.js/packages/lib/src/utils/compare.test.ts @@ -18,11 +18,13 @@ describe("Test shallowEqualArrays()", () => { const arr_n: number[] = [1, 2]; const arr_o: null[] = [null]; const arr_p: null[] = [null]; + const arr_q: null[] = []; it("works", () => { expect(shallowEqualArrays(arr_a, arr_b)).toBe(true); expect(shallowEqualArrays(arr_a, arr_c)).toBe(false); expect(shallowEqualArrays(arr_a, arr_d)).toBe(false); expect(shallowEqualArrays(arr_a, arr_e)).toBe(false); + expect(shallowEqualArrays(arr_e, arr_c)).toBe(false); expect(shallowEqualArrays(arr_f, arr_g)).toBe(true); expect(shallowEqualArrays(arr_h, arr_i)).toBe(false); expect(shallowEqualArrays(arr_j, arr_k)).toBe(false); @@ -30,5 +32,7 @@ describe("Test shallowEqualArrays()", () => { expect(shallowEqualArrays(arr_m, arr_n)).toBe(true); expect(shallowEqualArrays(arr_m, arr_l)).toBe(false); expect(shallowEqualArrays(arr_o, arr_p)).toBe(true); + expect(shallowEqualArrays(arr_p, arr_q)).toBe(false); + expect(shallowEqualArrays(arr_p)).toBe(false); }); }); From b78cce9d796471d5ddc54b04fe5b31978cbc1ab7 Mon Sep 17 00:00:00 2001 From: Yogesh Kumar Baljeet Singh Date: Fri, 14 Mar 2025 10:57:10 +0100 Subject: [PATCH 11/15] update based on reviwers comments --- .../lib/src/actions/handleHostStoreChange.ts | 26 ++++++++++--------- .../actions/handleHostStoreChanges.test.tsx | 1 - 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts b/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts index a1fcb24c..682aeb41 100644 --- a/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts +++ b/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts @@ -20,8 +20,6 @@ import memoize from "fast-memoize"; export interface PropertyRef extends ContribRef, CallbackRef, InputRef { /** The property. */ property: string; - /** Property ID for memoization */ - id: string; } export function handleHostStoreChange() { @@ -74,29 +72,34 @@ export function getCallbackRequests( ); } +// This is a dummy function created to memoize the _inputValues values +const _getInputValues = (_inputValues: unknown[]): unknown[] => { + return _inputValues; +}; +const memoizedInputValues = memoize(_getInputValues); + const getCallbackRequest = ( propertyRef: PropertyRef, lastCallbackInputValues: Record | undefined, contributionsRecord: Record, hostStore: HostStore, ) => { - const contributions = contributionsRecord[propertyRef.contribPoint]; - const contribution = contributions[propertyRef.contribIndex]; - const callback = contribution.callbacks![propertyRef.callbackIndex]; + const contribPoint: string = propertyRef.contribPoint; + const contribIndex: number = propertyRef.contribIndex; + const callbackIndex: number = propertyRef.callbackIndex; + const inputIndex: number = propertyRef.inputIndex; + const contributions = contributionsRecord[contribPoint]; + const contribution = contributions[contribIndex]; + const callback = contribution.callbacks![callbackIndex]; const _inputValues = getInputValues( callback.inputs!, contribution, hostStore, ); - // This is a dummy function created to memoize the _inputValues values - const _getInputValues = (_inputValues: unknown[]): unknown[] => { - return _inputValues; - }; - const memoizedInputValues = memoize(_getInputValues); const inputValues = memoizedInputValues(_inputValues); - const propRefId = propertyRef.id; + const propRefId = `${contribPoint}-${contribIndex}-${callbackIndex}-${inputIndex}`; if (lastCallbackInputValues) { const lastInputValues = lastCallbackInputValues[propRefId]; if (lastInputValues && shallowEqualArrays(lastInputValues, inputValues)) { @@ -142,7 +145,6 @@ function getHostStorePropertyRefs(): PropertyRef[] { callbackIndex, inputIndex, property: formatObjPath(input.property), - id: `${contribPoint}-${contribIndex}-${callbackIndex}-${inputIndex}`, }); } }), diff --git a/chartlets.js/packages/lib/src/actions/handleHostStoreChanges.test.tsx b/chartlets.js/packages/lib/src/actions/handleHostStoreChanges.test.tsx index ed4e9e3e..712abaf6 100644 --- a/chartlets.js/packages/lib/src/actions/handleHostStoreChanges.test.tsx +++ b/chartlets.js/packages/lib/src/actions/handleHostStoreChanges.test.tsx @@ -161,7 +161,6 @@ describe("handleHostStoreChange", () => { }); const propertyRefs: PropertyRef[] = [ { - id: "panel-0-0-0", contribPoint: "panel", contribIndex: 0, callbackIndex: 0, From c95671e145ffc39c08d5e8ca45a58fdf2c546c73 Mon Sep 17 00:00:00 2001 From: b-yogesh Date: Fri, 14 Mar 2025 10:57:35 +0100 Subject: [PATCH 12/15] Update chartlets.js/CHANGES.md Co-authored-by: Norman Fomferra --- chartlets.js/CHANGES.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/chartlets.js/CHANGES.md b/chartlets.js/CHANGES.md index fcd57ea5..ae32b0e5 100644 --- a/chartlets.js/CHANGES.md +++ b/chartlets.js/CHANGES.md @@ -3,8 +3,8 @@ * Add `multiple` property for `Select` component to enable the selection of multiple elements. The `default` mode is supported at the moment. -* Callbacks will now only be invoked when there’s an actual change in state, reducing unnecessary processing and - improving performance. +* Callbacks will now only be invoked when there’s an actual change in state, + reducing unnecessary processing and improving performance. (#112) ## Version 0.1.4 (from 2025/03/06) From 3fab4e47ae58f2724c68981f090753b51b354fd1 Mon Sep 17 00:00:00 2001 From: Yogesh Kumar Baljeet Singh Date: Fri, 14 Mar 2025 16:12:31 +0100 Subject: [PATCH 13/15] small refactor --- .../packages/lib/src/actions/handleHostStoreChange.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts b/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts index 682aeb41..94310210 100644 --- a/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts +++ b/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts @@ -72,7 +72,8 @@ export function getCallbackRequests( ); } -// This is a dummy function created to memoize the _inputValues values +// This is a dummy function created to memoize the _inputValues values from +// getCallbackRequest() const _getInputValues = (_inputValues: unknown[]): unknown[] => { return _inputValues; }; @@ -99,9 +100,9 @@ const getCallbackRequest = ( const inputValues = memoizedInputValues(_inputValues); - const propRefId = `${contribPoint}-${contribIndex}-${callbackIndex}-${inputIndex}`; + const callbackId = `${contribPoint}-${contribIndex}-${callbackIndex}-${inputIndex}`; if (lastCallbackInputValues) { - const lastInputValues = lastCallbackInputValues[propRefId]; + const lastInputValues = lastCallbackInputValues[callbackId]; if (lastInputValues && shallowEqualArrays(lastInputValues, inputValues)) { // We no longer log, as the situation is quite common // Enable error logging for debugging only: @@ -110,12 +111,12 @@ const getCallbackRequest = ( // console.groupEnd(); return undefined; } - lastCallbackInputValues[propRefId] = inputValues; + lastCallbackInputValues[callbackId] = inputValues; store.setState({ lastCallbackInputValues: { ...lastCallbackInputValues }, }); } else { - store.setState({ lastCallbackInputValues: { [propRefId]: inputValues } }); + store.setState({ lastCallbackInputValues: { [callbackId]: inputValues } }); } return { ...propertyRef, inputValues }; }; From 86809c8cdf3353b86a9cf6bd7b7d0c9f67e7a44f Mon Sep 17 00:00:00 2001 From: Yogesh Kumar Baljeet Singh Date: Fri, 14 Mar 2025 16:13:14 +0100 Subject: [PATCH 14/15] update --- chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts b/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts index 94310210..3194b926 100644 --- a/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts +++ b/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts @@ -88,7 +88,6 @@ const getCallbackRequest = ( const contribPoint: string = propertyRef.contribPoint; const contribIndex: number = propertyRef.contribIndex; const callbackIndex: number = propertyRef.callbackIndex; - const inputIndex: number = propertyRef.inputIndex; const contributions = contributionsRecord[contribPoint]; const contribution = contributions[contribIndex]; const callback = contribution.callbacks![callbackIndex]; @@ -100,7 +99,7 @@ const getCallbackRequest = ( const inputValues = memoizedInputValues(_inputValues); - const callbackId = `${contribPoint}-${contribIndex}-${callbackIndex}-${inputIndex}`; + const callbackId = `${contribPoint}-${contribIndex}-${callbackIndex}`; if (lastCallbackInputValues) { const lastInputValues = lastCallbackInputValues[callbackId]; if (lastInputValues && shallowEqualArrays(lastInputValues, inputValues)) { From 7a49490d348f11abcdcbf7be15204d285d310ede Mon Sep 17 00:00:00 2001 From: Yogesh Kumar Baljeet Singh Date: Fri, 14 Mar 2025 17:26:41 +0100 Subject: [PATCH 15/15] update based on reviewers comments --- chartlets.js/package-lock.json | 7 --- chartlets.js/packages/lib/package.json | 1 - .../lib/src/actions/handleHostStoreChange.ts | 49 ++++++------------- .../packages/lib/src/types/state/store.ts | 4 +- 4 files changed, 18 insertions(+), 43 deletions(-) diff --git a/chartlets.js/package-lock.json b/chartlets.js/package-lock.json index b3214816..3b8e04fb 100644 --- a/chartlets.js/package-lock.json +++ b/chartlets.js/package-lock.json @@ -4352,12 +4352,6 @@ "dev": true, "license": "MIT" }, - "node_modules/fast-memoize": { - "version": "2.5.2", - "resolved": "https://registry.npmjs.org/fast-memoize/-/fast-memoize-2.5.2.tgz", - "integrity": "sha512-Ue0LwpDYErFbmNnZSF0UH6eImUwDmogUO1jyE+JbN2gsQz/jICm1Ve7t9QT0rNSsfJt+Hs4/S3GnsDVjL4HVrw==", - "license": "MIT" - }, "node_modules/fast-uri": { "version": "3.0.3", "resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.0.3.tgz", @@ -8281,7 +8275,6 @@ "version": "0.1.4", "license": "MIT", "dependencies": { - "fast-memoize": "^2.5.2", "microdiff": "^1.4", "zustand": "^5.0" }, diff --git a/chartlets.js/packages/lib/package.json b/chartlets.js/packages/lib/package.json index e745513e..cc196cdc 100644 --- a/chartlets.js/packages/lib/package.json +++ b/chartlets.js/packages/lib/package.json @@ -55,7 +55,6 @@ "preview": "vite preview" }, "dependencies": { - "fast-memoize": "^2.5.2", "microdiff": "^1.4", "zustand": "^5.0" }, diff --git a/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts b/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts index 3194b926..ad3f9c78 100644 --- a/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts +++ b/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts @@ -12,7 +12,6 @@ import type { ContributionState } from "@/types/state/contribution"; import type { HostStore } from "@/types/state/host"; import { store } from "@/store"; import { shallowEqualArrays } from "@/utils/compare"; -import memoize from "fast-memoize"; /** * A reference to a property of an input of a callback of a contribution. @@ -72,16 +71,9 @@ export function getCallbackRequests( ); } -// This is a dummy function created to memoize the _inputValues values from -// getCallbackRequest() -const _getInputValues = (_inputValues: unknown[]): unknown[] => { - return _inputValues; -}; -const memoizedInputValues = memoize(_getInputValues); - const getCallbackRequest = ( propertyRef: PropertyRef, - lastCallbackInputValues: Record | undefined, + lastCallbackInputValues: Record, contributionsRecord: Record, hostStore: HostStore, ) => { @@ -91,32 +83,23 @@ const getCallbackRequest = ( const contributions = contributionsRecord[contribPoint]; const contribution = contributions[contribIndex]; const callback = contribution.callbacks![callbackIndex]; - const _inputValues = getInputValues( - callback.inputs!, - contribution, - hostStore, - ); - - const inputValues = memoizedInputValues(_inputValues); - + const inputValues = getInputValues(callback.inputs!, contribution, hostStore); const callbackId = `${contribPoint}-${contribIndex}-${callbackIndex}`; - if (lastCallbackInputValues) { - const lastInputValues = lastCallbackInputValues[callbackId]; - if (lastInputValues && shallowEqualArrays(lastInputValues, inputValues)) { - // We no longer log, as the situation is quite common - // Enable error logging for debugging only: - // console.groupCollapsed("Skipping callback request"); - // console.debug("inputValues", inputValues); - // console.groupEnd(); - return undefined; - } - lastCallbackInputValues[callbackId] = inputValues; - store.setState({ - lastCallbackInputValues: { ...lastCallbackInputValues }, - }); - } else { - store.setState({ lastCallbackInputValues: { [callbackId]: inputValues } }); + const lastInputValues = lastCallbackInputValues[callbackId]; + if (shallowEqualArrays(lastInputValues, inputValues)) { + // We no longer log, as the situation is quite common + // Enable error logging for debugging only: + // console.groupCollapsed("Skipping callback request"); + // console.debug("inputValues", inputValues); + // console.groupEnd(); + return undefined; } + store.setState({ + lastCallbackInputValues: { + ...lastCallbackInputValues, + [callbackId]: inputValues, + }, + }); return { ...propertyRef, inputValues }; }; diff --git a/chartlets.js/packages/lib/src/types/state/store.ts b/chartlets.js/packages/lib/src/types/state/store.ts index b1c25b90..f913c203 100644 --- a/chartlets.js/packages/lib/src/types/state/store.ts +++ b/chartlets.js/packages/lib/src/types/state/store.ts @@ -28,6 +28,6 @@ export interface StoreState { /** * Store last input values for callback requests to avoid invoking them if * there are no state changes - * */ - lastCallbackInputValues?: Record; + */ + lastCallbackInputValues: Record; }