diff --git a/chartlets.js/CHANGES.md b/chartlets.js/CHANGES.md index ae32b0e..5095876 100644 --- a/chartlets.js/CHANGES.md +++ b/chartlets.js/CHANGES.md @@ -3,6 +3,12 @@ * Add `multiple` property for `Select` component to enable the selection of multiple elements. The `default` mode is supported at the moment. +* Static information about callbacks retrieved from API is not cached + reducing unnecessary processing and improving performance. (#113) + +* Relaxed requirements for adding new components to Chartlets.js via + plugins. Implementing `ComponentProps` is now optional. (#115) + * Callbacks will now only be invoked when there’s an actual change in state, reducing unnecessary processing and improving performance. (#112) diff --git a/chartlets.js/package-lock.json b/chartlets.js/package-lock.json index 3b8e04f..78e4a13 100644 --- a/chartlets.js/package-lock.json +++ b/chartlets.js/package-lock.json @@ -5452,6 +5452,12 @@ "node": ">= 8" } }, + "node_modules/micro-memoize": { + "version": "4.1.3", + "resolved": "https://registry.npmjs.org/micro-memoize/-/micro-memoize-4.1.3.tgz", + "integrity": "sha512-DzRMi8smUZXT7rCGikRwldEh6eO6qzKiPPopcr1+2EY3AYKpy5fu159PKWwIS9A6IWnrvPKDMcuFtyrroZa8Bw==", + "license": "MIT" + }, "node_modules/microdiff": { "version": "1.4.0", "resolved": "https://registry.npmjs.org/microdiff/-/microdiff-1.4.0.tgz", @@ -8275,6 +8281,7 @@ "version": "0.1.4", "license": "MIT", "dependencies": { + "micro-memoize": "^4.1.3", "microdiff": "^1.4", "zustand": "^5.0" }, diff --git a/chartlets.js/packages/lib/package.json b/chartlets.js/packages/lib/package.json index cc196cd..3c38875 100644 --- a/chartlets.js/packages/lib/package.json +++ b/chartlets.js/packages/lib/package.json @@ -55,6 +55,7 @@ "preview": "vite preview" }, "dependencies": { + "micro-memoize": "^4.1.3", "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 ad3f9c7..30d00c1 100644 --- a/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts +++ b/chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts @@ -1,17 +1,20 @@ +import memoize from "micro-memoize"; + import type { + Callback, CallbackRef, CallbackRequest, ContribRef, InputRef, } from "@/types/model/callback"; -import type { Input } from "@/types/model/channel"; import { getInputValues } from "@/actions/helpers/getInputValues"; import { formatObjPath } from "@/utils/objPath"; 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"; +import { shallowEqualArrays } from "@/utils/shallowEqualArrays"; +import type { ContribPoint } from "@/types/model/extension"; /** * A reference to a property of an input of a callback of a contribution. @@ -21,23 +24,31 @@ export interface PropertyRef extends ContribRef, CallbackRef, InputRef { property: string; } -export function handleHostStoreChange() { +/** + * This action is called once a host store change has been detected. + * Note this will only create callback requests for callbacks whose input + * value are affected by the change. + * + * @returns The array of callback requests made or `undefined`, + * if no callback requests have been made. + */ +export function handleHostStoreChange(): CallbackRequest[] | undefined { const { extensions, configuration, contributionsRecord } = store.getState(); const { hostStore } = configuration; if (!hostStore) { // Exit if no host store configured. // Actually, we should not come here. - return; + return undefined; } synchronizeThemeMode(hostStore); if (extensions.length === 0) { // Exit if there are no extensions (yet) - return; + return undefined; } - const propertyRefs = getHostStorePropertyRefs(); + const propertyRefs = getPropertyRefsForContribPoints(contributionsRecord); if (!propertyRefs || propertyRefs.length === 0) { // Exit if there are is nothing to be changed - return; + return undefined; } const callbackRequests = getCallbackRequests( propertyRefs, @@ -49,9 +60,12 @@ export function handleHostStoreChange() { (callbackRequest): callbackRequest is CallbackRequest => callbackRequest !== undefined, ); - if (filteredCallbackRequests && filteredCallbackRequests.length > 0) { - invokeCallbacks(filteredCallbackRequests); + if (!filteredCallbackRequests || !filteredCallbackRequests.length) { + return undefined; } + + invokeCallbacks(filteredCallbackRequests); + return filteredCallbackRequests; } // Exporting for testing only @@ -103,36 +117,77 @@ const getCallbackRequest = ( 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. -// The hostStorePropertyRefs only depend on the static -// contribution infos. - /** - * Get the static list of host state property references for all contributions. + * Get the static list of host state property references + * for given contribution points. + * Note: the export exists only for testing. */ -function getHostStorePropertyRefs(): PropertyRef[] { - const { contributionsRecord } = store.getState(); +export const getPropertyRefsForContribPoints = memoize( + _getPropertyRefsForContribPoints, +); + +function _getPropertyRefsForContribPoints( + contributionsRecord: Record, +): PropertyRef[] { const propertyRefs: PropertyRef[] = []; Object.getOwnPropertyNames(contributionsRecord).forEach((contribPoint) => { const contributions = contributionsRecord[contribPoint]; - contributions.forEach((contribution, contribIndex) => { - (contribution.callbacks || []).forEach( - (callback, callbackIndex) => - (callback.inputs || []).forEach((input, inputIndex) => { - if (!input.noTrigger && input.id === "@app" && input.property) { - propertyRefs.push({ - contribPoint, - contribIndex, - callbackIndex, - inputIndex, - property: formatObjPath(input.property), - }); - } - }), - [] as Input[], - ); + propertyRefs.push( + ...getPropertyRefsForContributions(contribPoint, contributions), + ); + }); + return propertyRefs; +} + +/** + * Get the static list of host state property references + * for given contributions. + */ +const getPropertyRefsForContributions = memoize( + _getPropertyRefsForContributions, +); + +function _getPropertyRefsForContributions( + contribPoint: string, + contributions: ContributionState[], +): PropertyRef[] { + const propertyRefs: PropertyRef[] = []; + contributions.forEach((contribution, contribIndex) => { + propertyRefs.push( + ...getPropertyRefsForCallbacks( + contribPoint, + contribIndex, + contribution.callbacks, + ), + ); + }); + return propertyRefs; +} + +/** + * Get the static list of host state property references + * for given callbacks. + */ +const getPropertyRefsForCallbacks = memoize(_getPropertyRefsForCallbacks); + +function _getPropertyRefsForCallbacks( + contribPoint: string, + contribIndex: number, + callbacks: Callback[] | undefined, +) { + const propertyRefs: PropertyRef[] = []; + (callbacks || []).forEach((callback, callbackIndex) => { + const inputs = callback.inputs || []; + inputs.forEach((input, inputIndex) => { + if (!input.noTrigger && input.id === "@app" && input.property) { + propertyRefs.push({ + contribPoint, + contribIndex, + callbackIndex, + inputIndex, + property: formatObjPath(input.property), + }); + } }); }); return propertyRefs; diff --git a/chartlets.js/packages/lib/src/actions/handleHostStoreChanges.test.tsx b/chartlets.js/packages/lib/src/actions/handleHostStoreChanges.test.tsx index 712abaf..7ad89d9 100644 --- a/chartlets.js/packages/lib/src/actions/handleHostStoreChanges.test.tsx +++ b/chartlets.js/packages/lib/src/actions/handleHostStoreChanges.test.tsx @@ -2,6 +2,7 @@ import { beforeEach, describe, expect, it } from "vitest"; import { store } from "@/store"; import { getCallbackRequests, + getPropertyRefsForContribPoints, handleHostStoreChange, type PropertyRef, } from "./handleHostStoreChange"; @@ -11,6 +12,7 @@ import type { ContributionState } from "@/types/state/contribution"; describe("handleHostStoreChange", () => { let listeners: (() => void)[] = []; let hostState: Record = {}; + // noinspection JSUnusedGlobalSymbols const hostStore = { get: (key: string) => hostState[key], set: (key: string, value: unknown) => { @@ -46,6 +48,76 @@ describe("handleHostStoreChange", () => { expect(store.getState().themeMode).toEqual("light"); }); + it("should memoize computation of property refs", () => { + const contributionsRecord: Record = { + panels: [ + { + 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" }], + }, + { + function: { + name: "callback2", + parameters: [], + return: {}, + }, + inputs: [ + { id: "@app", property: "datasetId" }, + { id: "@app", property: "variableName" }, + ], + outputs: [{ id: "plot", property: "value" }], + }, + ], + }, + ], + }; + const propertyRefs1 = getPropertyRefsForContribPoints(contributionsRecord); + const propertyRefs2 = getPropertyRefsForContribPoints(contributionsRecord); + const propertyRefs3 = getPropertyRefsForContribPoints({ + ...contributionsRecord, + }); + expect(propertyRefs1).toBe(propertyRefs2); + expect(propertyRefs2).not.toBe(propertyRefs3); + expect(propertyRefs1).toEqual([ + { + callbackIndex: 0, + contribIndex: 0, + contribPoint: "panels", + inputIndex: 0, + property: "variableName", + }, + { + callbackIndex: 1, + contribIndex: 0, + contribPoint: "panels", + inputIndex: 0, + property: "datasetId", + }, + { + callbackIndex: 1, + contribIndex: 0, + contribPoint: "panels", + inputIndex: 1, + property: "variableName", + }, + ]); + expect(propertyRefs1).toEqual(propertyRefs2); + expect(propertyRefs1).toEqual(propertyRefs3); + expect(propertyRefs2).toEqual(propertyRefs3); + }); + it("should generate callback requests", () => { const extensions = [{ name: "e0", version: "0", contributes: ["panels"] }]; store.setState({ @@ -110,12 +182,20 @@ describe("handleHostStoreChange", () => { }, }); hostStore.set("variableName", "CHL"); - handleHostStoreChange(); + expect(handleHostStoreChange()).toEqual([ + { + contribPoint: "panel", + contribIndex: 0, + callbackIndex: 0, + inputIndex: 0, + inputValues: ["CHL"], + property: "variableName", + }, + ]); - // calling it second time for coverage. No state change changes the - // control flow - handleHostStoreChange(); - // TODO: Update this test to assert the generated callback request + // calling it second time for coverage, + // because the no-state-change changes the control flow + expect(handleHostStoreChange()).toBeUndefined(); }); it("should memoize second call with same arguments", () => { diff --git a/chartlets.js/packages/lib/src/types/state/store.ts b/chartlets.js/packages/lib/src/types/state/store.ts index f913c20..cb769bd 100644 --- a/chartlets.js/packages/lib/src/types/state/store.ts +++ b/chartlets.js/packages/lib/src/types/state/store.ts @@ -9,14 +9,33 @@ import type { FrameworkOptions } from "./options"; export type ThemeMode = "dark" | "light" | "system"; +// TODO: Split contributionsRecord into two fields comprising static +// contribution data and dynamic contribution states. +// This will allow memoizing the computation of property references +// (PropertyRef[]) on the level of the StoreState from static data only. +// The property references would then be just computed once. +// See function getPropertyRefsForContribPoints() +// in actions/handleHostStoreChange.ts + +/** + * The state of the Chartlets main store. + */ export interface StoreState { - /** Framework configuration */ + /** + * Framework configuration. + */ configuration: FrameworkOptions; - /** All extensions */ + /** + * All extensions. + */ extensions: Extension[]; - /** API call result from `GET /contributions`. */ + /** + * API call result from `GET /contributions`. + */ contributionsResult: ApiResult; - /** A record that maps contribPoint --> ContributionState[].*/ + /** + * A record that maps contribPoint --> ContributionState[]. + */ contributionsRecord: Record; /** * The app's current theme mode. diff --git a/chartlets.js/packages/lib/src/utils/compare.ts b/chartlets.js/packages/lib/src/utils/compare.ts deleted file mode 100644 index eb917bc..0000000 --- a/chartlets.js/packages/lib/src/utils/compare.ts +++ /dev/null @@ -1,12 +0,0 @@ -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]); -} diff --git a/chartlets.js/packages/lib/src/utils/compare.test.ts b/chartlets.js/packages/lib/src/utils/shallowEqualArrays.test.ts similarity index 71% rename from chartlets.js/packages/lib/src/utils/compare.test.ts rename to chartlets.js/packages/lib/src/utils/shallowEqualArrays.test.ts index 5889c4b..39b58c8 100644 --- a/chartlets.js/packages/lib/src/utils/compare.test.ts +++ b/chartlets.js/packages/lib/src/utils/shallowEqualArrays.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { shallowEqualArrays } from "@/utils/compare"; +import { shallowEqualArrays } from "@/utils/shallowEqualArrays"; describe("Test shallowEqualArrays()", () => { const arr_a: string[] = ["a", "b", "c"]; @@ -11,15 +11,16 @@ describe("Test shallowEqualArrays()", () => { 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_j: (number | string | null)[] = [1, 2.3, "c", null]; + const arr_k: (number | string | null)[] = [1, 3.1, "c", null]; + const arr_l: (number | string | null)[] = [1, 2.3, "c", null]; + const arr_m: number[] = [1, 2, NaN, Infinity]; + const arr_n: number[] = [1, 2, NaN, Infinity]; const arr_o: null[] = [null]; const arr_p: null[] = [null]; const arr_q: null[] = []; it("works", () => { + expect(shallowEqualArrays(arr_a, arr_a)).toBe(true); expect(shallowEqualArrays(arr_a, arr_b)).toBe(true); expect(shallowEqualArrays(arr_a, arr_c)).toBe(false); expect(shallowEqualArrays(arr_a, arr_d)).toBe(false); @@ -33,6 +34,7 @@ describe("Test shallowEqualArrays()", () => { 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); + expect(shallowEqualArrays(arr_p, undefined)).toBe(false); + expect(shallowEqualArrays(undefined, arr_p)).toBe(false); }); }); diff --git a/chartlets.js/packages/lib/src/utils/shallowEqualArrays.ts b/chartlets.js/packages/lib/src/utils/shallowEqualArrays.ts new file mode 100644 index 0000000..77c7af6 --- /dev/null +++ b/chartlets.js/packages/lib/src/utils/shallowEqualArrays.ts @@ -0,0 +1,20 @@ +export function shallowEqualArrays( + arr1: unknown[] | undefined, + arr2: unknown[] | undefined, +): boolean { + if (arr1 === arr2) { + return true; + } + if (!arr1 || !arr2) { + return false; + } + if (arr1.length !== arr2.length) { + return false; + } + for (let i = 0; i < arr1.length; i++) { + if (!Object.is(arr1[i], arr2[i])) { + return false; + } + } + return true; +}