Merge pull request #114 from bcdev/forman-113-memoize_getHostStorePropertyRefs

Memoize computation of property references

Author: forman

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)
 

chartlets.js/package-lock.json

@@ -55,6 +55,7 @@
     "preview": "vite preview"
   },
   "dependencies": {
+    "micro-memoize": "^4.1.3",
     "microdiff": "^1.4",
     "zustand": "^5.0"
   },

chartlets.js/packages/lib/package.json

@@ -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<ContribPoint, ContributionState[]>,
+): 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;

chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts

@@ -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<string, unknown> = {};
+  // 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<string, ContributionState[]> = {
+      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", () => {

chartlets.js/packages/lib/src/actions/handleHostStoreChanges.test.tsx

@@ -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<Contributions>;
-  /** A record that maps contribPoint --> ContributionState[].*/
+  /**
+   * A record that maps contribPoint --> ContributionState[].
+   */
   contributionsRecord: Record<ContribPoint, ContributionState[]>;
   /**
    * The app's current theme mode.