Skip to content
Merged
Show file tree
Hide file tree
Changes from 11 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.
Comment thread
b-yogesh marked this conversation as resolved.
Outdated

## Version 0.1.4 (from 2025/03/06)

Expand Down
9 changes: 8 additions & 1 deletion chartlets.js/package-lock.json

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

1 change: 1 addition & 0 deletions chartlets.js/packages/lib/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
"preview": "vite preview"
},
"dependencies": {
"fast-memoize": "^2.5.2",
"microdiff": "^1.4",
"zustand": "^5.0"
},
Expand Down
80 changes: 66 additions & 14 deletions chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,17 @@ 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 memoize from "fast-memoize";

/**
* A reference to a property of an input of a callback of a contribution.
*/
export interface PropertyRef extends ContribRef, CallbackRef, InputRef {
/** The property. */
property: string;
/** Property ID for memoization */
id: string;
Comment thread
b-yogesh marked this conversation as resolved.
Outdated
}

export function handleHostStoreChange() {
Expand All @@ -43,29 +47,76 @@ 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[]> | 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);
Comment thread
b-yogesh marked this conversation as resolved.
Outdated
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 {
Comment thread
b-yogesh marked this conversation as resolved.
Outdated
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.
Expand All @@ -91,6 +142,7 @@ function getHostStorePropertyRefs(): PropertyRef[] {
callbackIndex,
inputIndex,
property: formatObjPath(input.property),
id: `${contribPoint}-${contribIndex}-${callbackIndex}-${inputIndex}`,
Comment thread
b-yogesh marked this conversation as resolved.
Outdated
});
}
}),
Expand Down
142 changes: 140 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,138 @@ 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[] = [
{
id: "panel-0-0-0",
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);
});
});
Loading