Skip to content

Commit 9da8720

Browse files
formanforman
committed
Merge branch 'main' into forman-113-memoize_getHostStorePropertyRefs
# Conflicts: # chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts
2 parents a9ff949 + f6118e4 commit 9da8720

File tree

8 files changed

+249
-17
lines changed

8 files changed

+249
-17
lines changed

‎chartlets.js/CHANGES.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
* Add `multiple` property for `Select` component to enable the selection
44
of multiple elements. The `default` mode is supported at the moment.
55

6+
* Callbacks will now only be invoked when there’s an actual change in state,
7+
reducing unnecessary processing and improving performance. (#112)
68

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

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

Lines changed: 51 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { invokeCallbacks } from "@/actions/helpers/invokeCallbacks";
1313
import type { ContributionState } from "@/types/state/contribution";
1414
import type { HostStore } from "@/types/state/host";
1515
import { store } from "@/store";
16+
import { shallowEqualArrays } from "@/utils/compare";
1617
import type { ContribPoint } from "@/types/model/extension";
1718

1819
/**
@@ -46,29 +47,65 @@ export function handleHostStoreChange() {
4647
contributionsRecord,
4748
hostStore,
4849
);
49-
if (callbackRequests && callbackRequests.length > 0) {
50-
invokeCallbacks(callbackRequests);
50+
51+
const filteredCallbackRequests = callbackRequests.filter(
52+
(callbackRequest): callbackRequest is CallbackRequest =>
53+
callbackRequest !== undefined,
54+
);
55+
if (filteredCallbackRequests && filteredCallbackRequests.length > 0) {
56+
invokeCallbacks(filteredCallbackRequests);
5157
}
5258
}
5359

54-
function getCallbackRequests(
60+
// Exporting for testing only
61+
export function getCallbackRequests(
5562
propertyRefs: PropertyRef[],
5663
contributionsRecord: Record<string, ContributionState[]>,
5764
hostStore: HostStore,
58-
): CallbackRequest[] {
59-
return propertyRefs.map((propertyRef) => {
60-
const contributions = contributionsRecord[propertyRef.contribPoint];
61-
const contribution = contributions[propertyRef.contribIndex];
62-
const callback = contribution.callbacks![propertyRef.callbackIndex];
63-
const inputValues = getInputValues(
64-
callback.inputs!,
65-
contribution,
65+
): (CallbackRequest | undefined)[] {
66+
const { lastCallbackInputValues } = store.getState();
67+
return propertyRefs.map((propertyRef) =>
68+
getCallbackRequest(
69+
propertyRef,
70+
lastCallbackInputValues,
71+
contributionsRecord,
6672
hostStore,
67-
);
68-
return { ...propertyRef, inputValues };
69-
});
73+
),
74+
);
7075
}
7176

77+
const getCallbackRequest = (
78+
propertyRef: PropertyRef,
79+
lastCallbackInputValues: Record<string, unknown[]>,
80+
contributionsRecord: Record<string, ContributionState[]>,
81+
hostStore: HostStore,
82+
) => {
83+
const contribPoint: string = propertyRef.contribPoint;
84+
const contribIndex: number = propertyRef.contribIndex;
85+
const callbackIndex: number = propertyRef.callbackIndex;
86+
const contributions = contributionsRecord[contribPoint];
87+
const contribution = contributions[contribIndex];
88+
const callback = contribution.callbacks![callbackIndex];
89+
const inputValues = getInputValues(callback.inputs!, contribution, hostStore);
90+
const callbackId = `${contribPoint}-${contribIndex}-${callbackIndex}`;
91+
const lastInputValues = lastCallbackInputValues[callbackId];
92+
if (shallowEqualArrays(lastInputValues, inputValues)) {
93+
// We no longer log, as the situation is quite common
94+
// Enable error logging for debugging only:
95+
// console.groupCollapsed("Skipping callback request");
96+
// console.debug("inputValues", inputValues);
97+
// console.groupEnd();
98+
return undefined;
99+
}
100+
store.setState({
101+
lastCallbackInputValues: {
102+
...lastCallbackInputValues,
103+
[callbackId]: inputValues,
104+
},
105+
});
106+
return { ...propertyRef, inputValues };
107+
};
108+
72109
/**
73110
* Get the static list of host state property references
74111
* for given contribution points.

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

Lines changed: 139 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
1-
import { describe, it, expect, beforeEach } from "vitest";
1+
import { beforeEach, describe, expect, it } from "vitest";
22
import { store } from "@/store";
3-
import { handleHostStoreChange } from "./handleHostStoreChange";
3+
import {
4+
getCallbackRequests,
5+
handleHostStoreChange,
6+
type PropertyRef,
7+
} from "./handleHostStoreChange";
8+
import type { ContribPoint } from "@/types/model/extension";
9+
import type { ContributionState } from "@/types/state/contribution";
410

511
describe("handleHostStoreChange", () => {
612
let listeners: (() => void)[] = [];
@@ -15,10 +21,12 @@ describe("handleHostStoreChange", () => {
1521
listeners.push(_l);
1622
},
1723
};
24+
let lastCallbackInputValues: Record<string, unknown[]> = {};
1825

1926
beforeEach(() => {
2027
listeners = [];
2128
hostState = {};
29+
lastCallbackInputValues = {};
2230
});
2331

2432
it("should do nothing without host store", () => {
@@ -78,8 +86,137 @@ describe("handleHostStoreChange", () => {
7886
},
7987
},
8088
},
89+
contributionsRecord: {
90+
panel: [
91+
{
92+
name: "p0",
93+
container: { title: "Panel A" },
94+
extension: "e0",
95+
componentResult: {},
96+
initialState: {},
97+
callbacks: [
98+
{
99+
function: {
100+
name: "callback",
101+
parameters: [],
102+
return: {},
103+
},
104+
inputs: [{ id: "@app", property: "variableName" }],
105+
outputs: [{ id: "select", property: "value" }],
106+
},
107+
],
108+
},
109+
],
110+
},
81111
});
82112
hostStore.set("variableName", "CHL");
83113
handleHostStoreChange();
114+
115+
// calling it second time for coverage. No state change changes the
116+
// control flow
117+
handleHostStoreChange();
118+
// TODO: Update this test to assert the generated callback request
119+
});
120+
121+
it("should memoize second call with same arguments", () => {
122+
const extensions = [{ name: "ext", version: "0", contributes: ["panels"] }];
123+
store.setState({
124+
configuration: { hostStore, logging: { enabled: false } },
125+
extensions,
126+
contributionsResult: {
127+
status: "ok",
128+
data: {
129+
extensions,
130+
contributions: {
131+
panels: [
132+
{
133+
name: "ext.p1",
134+
extension: "ext",
135+
layout: {
136+
function: {
137+
name: "layout",
138+
parameters: [],
139+
return: {},
140+
},
141+
inputs: [],
142+
outputs: [],
143+
},
144+
callbacks: [
145+
{
146+
function: {
147+
name: "callback",
148+
parameters: [],
149+
return: {},
150+
},
151+
inputs: [{ id: "@app", property: "variableName" }],
152+
},
153+
],
154+
initialState: {},
155+
},
156+
],
157+
},
158+
},
159+
},
160+
lastCallbackInputValues: lastCallbackInputValues,
161+
});
162+
const propertyRefs: PropertyRef[] = [
163+
{
164+
contribPoint: "panel",
165+
contribIndex: 0,
166+
callbackIndex: 0,
167+
property: "value",
168+
inputIndex: 0,
169+
},
170+
];
171+
const contributionsRecord: Record<ContribPoint, ContributionState[]> = {
172+
panel: [
173+
{
174+
name: "ext.p1",
175+
container: { title: "Panel A" },
176+
extension: "ext",
177+
componentResult: {},
178+
initialState: { title: "Panel A" },
179+
callbacks: [
180+
{
181+
function: {
182+
name: "callback",
183+
parameters: [{ name: "param1" }],
184+
return: {},
185+
},
186+
inputs: [{ id: "@app", property: "variableName" }],
187+
},
188+
],
189+
},
190+
],
191+
};
192+
193+
hostStore.set("variableName", "CHL");
194+
195+
// first call -> should create callback request
196+
let result = getCallbackRequests(
197+
propertyRefs,
198+
contributionsRecord,
199+
hostStore,
200+
);
201+
expect(result[0]).toEqual({
202+
...propertyRefs[0],
203+
inputValues: ["CHL"],
204+
});
205+
206+
// second call -> memoized -> should not create callback request
207+
result = getCallbackRequests(propertyRefs, contributionsRecord, hostStore);
208+
expect(result).toEqual([undefined]);
209+
210+
// Third call - change state -> should create callback request
211+
hostStore.set("variableName", "TMP");
212+
result = getCallbackRequests(propertyRefs, contributionsRecord, hostStore);
213+
expect(result[0]).toEqual({
214+
...propertyRefs[0],
215+
inputValues: ["TMP"],
216+
});
217+
218+
// fourth call -> memoized -> should not invoke callback
219+
result = getCallbackRequests(propertyRefs, contributionsRecord, hostStore);
220+
expect(result).toEqual([undefined]);
84221
});
85222
});

‎chartlets.js/packages/lib/src/actions/helpers/getInputValues.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ export function getInputValues(
2626

2727
const noValue = {};
2828

29-
export function getInputValue(
29+
function getInputValue(
3030
input: Input,
3131
contributionState: ContributionState,
3232
hostStore?: HostStore,

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,5 @@ export const store = create<StoreState>(() => ({
77
extensions: [],
88
contributionsResult: {},
99
contributionsRecord: {},
10+
lastCallbackInputValues: {},
1011
}));

‎chartlets.js/packages/lib/src/types/state/store.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,4 +44,9 @@ export interface StoreState {
4444
* See hook `useThemeMode()`.
4545
*/
4646
themeMode?: ThemeMode;
47+
/**
48+
* Store last input values for callback requests to avoid invoking them if
49+
* there are no state changes
50+
*/
51+
lastCallbackInputValues: Record<string, unknown[]>;
4752
}

‎chartlets.js/packages/lib/src/utils/compare.test.ts‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import { describe, expect, it } from "vitest";
2+
import { shallowEqualArrays } from "@/utils/compare";
3+
4+
describe("Test shallowEqualArrays()", () => {
5+
const arr_a: string[] = ["a", "b", "c"];
6+
const arr_b: string[] = ["a", "b", "c"];
7+
const arr_c: string[] = ["a", "b", "d"];
8+
const arr_d: string[] = [];
9+
const arr_e: string[] = ["a", "b", "c", "d"];
10+
const arr_f: (string | null)[] = ["a", "b", "c", null];
11+
const arr_g: (string | null)[] = ["a", "b", "c", null];
12+
const arr_h = [1, [1, 2, 3], [4, 5, 6]];
13+
const arr_i = [1, [1, 2, 3], [4, 5, 6]];
14+
const arr_j: (number | string | null)[] = [1, 2, "c", null];
15+
const arr_k: (number | string | null)[] = [1, 3, "c", null];
16+
const arr_l: (number | string | null)[] = [1, 2, "c", null];
17+
const arr_m: number[] = [1, 2];
18+
const arr_n: number[] = [1, 2];
19+
const arr_o: null[] = [null];
20+
const arr_p: null[] = [null];
21+
const arr_q: null[] = [];
22+
it("works", () => {
23+
expect(shallowEqualArrays(arr_a, arr_b)).toBe(true);
24+
expect(shallowEqualArrays(arr_a, arr_c)).toBe(false);
25+
expect(shallowEqualArrays(arr_a, arr_d)).toBe(false);
26+
expect(shallowEqualArrays(arr_a, arr_e)).toBe(false);
27+
expect(shallowEqualArrays(arr_e, arr_c)).toBe(false);
28+
expect(shallowEqualArrays(arr_f, arr_g)).toBe(true);
29+
expect(shallowEqualArrays(arr_h, arr_i)).toBe(false);
30+
expect(shallowEqualArrays(arr_j, arr_k)).toBe(false);
31+
expect(shallowEqualArrays(arr_j, arr_l)).toBe(true);
32+
expect(shallowEqualArrays(arr_m, arr_n)).toBe(true);
33+
expect(shallowEqualArrays(arr_m, arr_l)).toBe(false);
34+
expect(shallowEqualArrays(arr_o, arr_p)).toBe(true);
35+
expect(shallowEqualArrays(arr_p, arr_q)).toBe(false);
36+
expect(shallowEqualArrays(arr_p)).toBe(false);
37+
});
38+
});

‎chartlets.js/packages/lib/src/utils/compare.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
export function shallowEqualArrays(
2+
arr1?: unknown[],
3+
arr2?: unknown[],
4+
): boolean {
5+
if (!arr1 || !arr2) {
6+
return false;
7+
}
8+
if (arr1.length !== arr2.length) {
9+
return false;
10+
}
11+
return arr1.every((val, index) => val === arr2[index]);
12+
}

0 commit comments

Comments
 (0)