diff --git a/chartlets.js/CHANGES.md b/chartlets.js/CHANGES.md index 43cafa89..ae32b0e5 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. (#112) ## Version 0.1.4 (from 2025/03/06) diff --git a/chartlets.js/package-lock.json b/chartlets.js/package-lock.json index ae12139d..3b8e04fb 100644 --- a/chartlets.js/package-lock.json +++ b/chartlets.js/package-lock.json @@ -8322,4 +8322,4 @@ } } } -} \ No newline at end of file +} diff --git a/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts b/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts index 31c7c55b..ad3f9c78 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. @@ -43,29 +44,65 @@ export function handleHostStoreChange() { contributionsRecord, 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[] { - 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, +): (CallbackRequest | undefined)[] { + const { lastCallbackInputValues } = store.getState(); + return propertyRefs.map((propertyRef) => + getCallbackRequest( + propertyRef, + lastCallbackInputValues, + contributionsRecord, hostStore, - ); - return { ...propertyRef, inputValues }; - }); + ), + ); } +const getCallbackRequest = ( + propertyRef: PropertyRef, + lastCallbackInputValues: Record, + contributionsRecord: Record, + hostStore: HostStore, +) => { + const contribPoint: string = propertyRef.contribPoint; + const contribIndex: number = propertyRef.contribIndex; + const callbackIndex: number = propertyRef.callbackIndex; + const contributions = contributionsRecord[contribPoint]; + const contribution = contributions[contribIndex]; + const callback = contribution.callbacks![callbackIndex]; + const inputValues = getInputValues(callback.inputs!, contribution, hostStore); + const callbackId = `${contribPoint}-${contribIndex}-${callbackIndex}`; + 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 }; +}; + // 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 49877aca..712abaf6 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 lastCallbackInputValues: Record = {}; beforeEach(() => { listeners = []; hostState = {}; + lastCallbackInputValues = {}; }); it("should do nothing without host store", () => { @@ -78,8 +86,137 @@ 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: "ext", 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" }], + }, + ], + initialState: {}, + }, + ], + }, + }, + }, + lastCallbackInputValues: lastCallbackInputValues, + }); + const propertyRefs: PropertyRef[] = [ + { + 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" }], + }, + ], + }, + ], + }; + + hostStore.set("variableName", "CHL"); + + // first call -> should create callback request + let result = getCallbackRequests( + propertyRefs, + contributionsRecord, + hostStore, + ); + expect(result[0]).toEqual({ + ...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 544a7e10..ed248f8f 100644 --- a/chartlets.js/packages/lib/src/actions/helpers/getInputValues.ts +++ b/chartlets.js/packages/lib/src/actions/helpers/getInputValues.ts @@ -26,7 +26,7 @@ export function getInputValues( const noValue = {}; -export 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..683ac9d3 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: {}, + lastCallbackInputValues: {}, })); diff --git a/chartlets.js/packages/lib/src/types/state/store.ts b/chartlets.js/packages/lib/src/types/state/store.ts index 5edb8366..f913c203 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 state changes + */ + lastCallbackInputValues: Record; } 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..5889c4b9 --- /dev/null +++ b/chartlets.js/packages/lib/src/utils/compare.test.ts @@ -0,0 +1,38 @@ +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"]; + 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]; + 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); + 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); + expect(shallowEqualArrays(arr_p, arr_q)).toBe(false); + expect(shallowEqualArrays(arr_p)).toBe(false); + }); +}); 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..eb917bc9 --- /dev/null +++ b/chartlets.js/packages/lib/src/utils/compare.ts @@ -0,0 +1,12 @@ +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]); +}