Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions chartlets.js/CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion chartlets.js/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

65 changes: 51 additions & 14 deletions chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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<string, ContributionState[]>,
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<string, unknown[]>,
contributionsRecord: Record<string, ContributionState[]>,
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.
Expand Down
141 changes: 139 additions & 2 deletions chartlets.js/packages/lib/src/actions/handleHostStoreChanges.test.tsx
Original file line number Diff line number Diff line change
@@ -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)[] = [];
Expand All @@ -15,10 +21,12 @@ describe("handleHostStoreChange", () => {
listeners.push(_l);
},
};
let lastCallbackInputValues: Record<string, unknown[]> = {};

beforeEach(() => {
listeners = [];
hostState = {};
lastCallbackInputValues = {};
});

it("should do nothing without host store", () => {
Expand Down Expand Up @@ -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<ContribPoint, ContributionState[]> = {
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]);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export function getInputValues(

const noValue = {};

export function getInputValue(
function getInputValue(
input: Input,
contributionState: ContributionState,
hostStore?: HostStore,
Expand Down
1 change: 1 addition & 0 deletions chartlets.js/packages/lib/src/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@ export const store = create<StoreState>(() => ({
extensions: [],
contributionsResult: {},
contributionsRecord: {},
lastCallbackInputValues: {},
}));
5 changes: 5 additions & 0 deletions chartlets.js/packages/lib/src/types/state/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown[]>;
}
38 changes: 38 additions & 0 deletions chartlets.js/packages/lib/src/utils/compare.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
12 changes: 12 additions & 0 deletions chartlets.js/packages/lib/src/utils/compare.ts
Original file line number Diff line number Diff line change
@@ -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]);
}