use shallow comparison and some refactoring

Author: b-yogesh

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<string, ContributionState[]>,
   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<string, unknown[]> | undefined,
+  contributionsRecord: Record<string, ContributionState[]>,
+  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.

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

@@ -21,12 +21,12 @@ describe("handleHostStoreChange", () => {
       listeners.push(_l);
     },
   };
-  let lastInputValues: Record<string, unknown[]> = {};
+  let lastCallbackInputValues: Record<string, unknown[]> = {};
 
   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]);
   });
 });

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,

chartlets.js/packages/lib/src/store.ts

@@ -7,5 +7,5 @@ export const store = create<StoreState>(() => ({
   extensions: [],
   contributionsResult: {},
   contributionsRecord: {},
-  lastInputValues: {},
+  lastCallbackInputValues: {},
 }));

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<string, unknown[]>;
+  lastCallbackInputValues?: Record<string, unknown[]>;
 }

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]);
 }