Skip to content

Commit 2e44753

Browse files
fix stale values in ensemble.user (#1096)
Co-authored-by: Ehmad Saeed <justehmadsaeed@gmail.com>
1 parent 4eee2e1 commit 2e44753

File tree

11 files changed

+260
-15
lines changed

11 files changed

+260
-15
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@ensembleui/react-framework": patch
3+
---
4+
5+
fix stale values in ensemble.user

packages/framework/jest.config.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
module.exports = {
44
preset: "ts-jest",
55
testEnvironment: "jsdom",
6+
setupFilesAfterEnv: ["<rootDir>/setupTests.ts"],
67
moduleNameMapper: {
78
"^lodash-es$": "lodash",
89
},

packages/framework/setupTests.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// setup i18n to avoid react-i18next warnings during tests
2+
import i18n from "i18next";
3+
import { initReactI18next } from "react-i18next";
4+
5+
void i18n.use(initReactI18next).init({
6+
lng: "en",
7+
fallbackLng: "en",
8+
resources: { en: { translation: {} } },
9+
interpolation: { escapeValue: false },
10+
});
11+
12+
// jsdom does not implement ResizeObserver; provide a polyfill for tests
13+
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
14+
global.ResizeObserver = require("resize-observer-polyfill");
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import { render, screen, waitFor } from "@testing-library/react";
2+
import React from "react";
3+
import { Provider } from "jotai";
4+
import { ApplicationContextProvider } from "../useApplicationContext";
5+
import { ScreenContextProvider } from "../useScreenContext";
6+
import type { EnsembleAppModel } from "../../shared";
7+
import { useEnsembleUser } from "../useEnsembleUser";
8+
9+
const Probe: React.FC<{ id: string }> = ({ id }) => {
10+
const user = useEnsembleUser();
11+
return <div data-testid={`u-${id}`}>{String(user.accessToken ?? "")}</div>;
12+
};
13+
14+
const BootstrapUser: React.FC = () => {
15+
const user = useEnsembleUser();
16+
React.useEffect(() => {
17+
user.set({ accessToken: "fresh" });
18+
// eslint-disable-next-line react-hooks/exhaustive-deps
19+
}, []);
20+
return null;
21+
};
22+
23+
const app: EnsembleAppModel = {
24+
id: "app",
25+
screens: [],
26+
customWidgets: [],
27+
scripts: [],
28+
home: {
29+
id: "home",
30+
name: "home",
31+
body: { name: "Text", properties: { text: "home" } },
32+
},
33+
themes: { default: { name: "default" } },
34+
};
35+
36+
describe("ScreenContextProvider does not clobber user atom", () => {
37+
test("user stays latest when screen mounts", async () => {
38+
render(
39+
<Provider>
40+
<ApplicationContextProvider app={app}>
41+
<BootstrapUser />
42+
<Probe id="before" />
43+
<ScreenContextProvider screen={{ name: "home" }}>
44+
<Probe id="after" />
45+
</ScreenContextProvider>
46+
</ApplicationContextProvider>
47+
</Provider>,
48+
);
49+
50+
await waitFor(() => {
51+
expect(screen.getByTestId("u-before").textContent).toBe("fresh");
52+
expect(screen.getByTestId("u-after").textContent).toBe("fresh");
53+
});
54+
});
55+
});

packages/framework/src/hooks/__tests__/useEnsembleUser.test.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@ import { useEnsembleUser } from "../useEnsembleUser";
77
const store = getDefaultStore();
88

99
describe("useEnsembleUser", () => {
10+
beforeEach(() => {
11+
sessionStorage.removeItem("ensemble.user");
12+
store.set(userAtom, {} as unknown as { [key: string]: unknown });
13+
});
14+
1015
test("fetches variable from ensemble.user", () => {
1116
store.set(userAtom, {
1217
accessToken: "eyJabcd",
@@ -58,4 +63,30 @@ describe("useEnsembleUser", () => {
5863
},
5964
});
6065
});
66+
67+
test("returns sessionStorage snapshot on first render when atom empty", () => {
68+
sessionStorage.setItem(
69+
"ensemble.user",
70+
JSON.stringify({ accessToken: "seed", userDetails: { foo: "bar" } }),
71+
);
72+
73+
const { result } = renderHook(() => useEnsembleUser());
74+
const user = result.current;
75+
76+
expect(get(user, "accessToken")).toBe("seed");
77+
expect(get(user, "userDetails.foo")).toBe("bar");
78+
});
79+
80+
test("prefers atom value over sessionStorage when atom populated", () => {
81+
sessionStorage.setItem(
82+
"ensemble.user",
83+
JSON.stringify({ accessToken: "seed" }),
84+
);
85+
store.set(userAtom, { accessToken: "live" });
86+
87+
const { result } = renderHook(() => useEnsembleUser());
88+
const user = result.current;
89+
90+
expect(get(user, "accessToken")).toBe("live");
91+
});
6192
});
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
import { act, render, screen, waitFor } from "@testing-library/react";
2+
import React, { useEffect } from "react";
3+
import { Provider } from "jotai";
4+
import { useEnsembleUser } from "../useEnsembleUser";
5+
6+
type Api = {
7+
setToken: (token?: string) => void;
8+
};
9+
10+
const UserTokenProbe: React.FC<{
11+
id: string;
12+
onReady?: (api: Api) => void;
13+
}> = ({ id, onReady }) => {
14+
const user = useEnsembleUser();
15+
16+
useEffect(() => {
17+
onReady?.({
18+
setToken: (token?: string) => user.set({ accessToken: token }),
19+
});
20+
}, [onReady, user]);
21+
22+
return (
23+
<div data-testid={`token-${id}`}>{String(user.accessToken ?? "")}</div>
24+
);
25+
};
26+
27+
describe("userAtom cross-store sync", () => {
28+
beforeEach(() => {
29+
sessionStorage.removeItem("ensemble.user");
30+
});
31+
32+
test("initializes from sessionStorage on first mount", async () => {
33+
sessionStorage.setItem(
34+
"ensemble.user",
35+
JSON.stringify({ accessToken: "seed" }),
36+
);
37+
38+
render(
39+
<Provider>
40+
<UserTokenProbe id="one" />
41+
</Provider>,
42+
);
43+
44+
await waitFor(() => {
45+
expect(screen.getByTestId("token-one").textContent).toBe("seed");
46+
});
47+
});
48+
49+
test("propagates updates between nested Providers via storage sync", async () => {
50+
let apiA: Api | undefined;
51+
let apiB: Api | undefined;
52+
53+
render(
54+
<Provider>
55+
<UserTokenProbe
56+
id="a"
57+
onReady={(api) => {
58+
apiA = api;
59+
}}
60+
/>
61+
<Provider>
62+
<UserTokenProbe
63+
id="b"
64+
onReady={(api) => {
65+
apiB = api;
66+
}}
67+
/>
68+
</Provider>
69+
</Provider>,
70+
);
71+
72+
// outer updates, inner should reflect
73+
act(() => {
74+
apiA?.setToken("t1");
75+
});
76+
77+
await waitFor(() => {
78+
expect(screen.getByTestId("token-a").textContent).toBe("t1");
79+
expect(screen.getByTestId("token-b").textContent).toBe("t1");
80+
});
81+
82+
// inner updates, outer should reflect
83+
act(() => {
84+
apiB?.setToken("t2");
85+
});
86+
87+
await waitFor(() => {
88+
expect(screen.getByTestId("token-a").textContent).toBe("t2");
89+
expect(screen.getByTestId("token-b").textContent).toBe("t2");
90+
});
91+
});
92+
});

packages/framework/src/hooks/useCommandCallback.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { useAtomCallback } from "jotai/utils";
22
import type { FC, ReactNode } from "react";
33
import { useCallback } from "react";
4-
import { mapKeys } from "lodash-es";
4+
import { mapKeys, assign } from "lodash-es";
55
import { createEvaluationContext } from "../evaluate";
66
import type { EnsembleUser } from "../state";
77
import { appAtom, screenAtom, themeAtom, userAtom } from "../state";
@@ -84,6 +84,8 @@ export const useCommandCallback = <
8484
setTheme: (name: string) => set(themeAtom, name),
8585
user: {
8686
...user,
87+
set: (userUpdate: EnsembleUser) =>
88+
set(userAtom, assign({}, user, userUpdate)),
8789
setUser: (userUpdate: EnsembleUser) => set(userAtom, userUpdate),
8890
},
8991
storage: storageApi,

packages/framework/src/hooks/useEnsembleUser.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { useAtom } from "jotai";
2-
import { assign } from "lodash-es";
2+
import { assign, isEmpty } from "lodash-es";
33
import { useMemo } from "react";
44
import { userAtom, type EnsembleUser } from "../state";
55

@@ -26,5 +26,18 @@ export const useEnsembleUser = (): EnsembleUser & EnsembleUserBuffer => {
2626
[setUser, user],
2727
);
2828

29-
return { ...storageBuffer, ...user };
29+
// ensure first render on direct loads sees latest value from sessionStorage
30+
const sessionSnapshot = useMemo<EnsembleUser>(() => {
31+
try {
32+
const raw = sessionStorage.getItem("ensemble.user");
33+
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
34+
return raw ? JSON.parse(raw) : {};
35+
} catch {
36+
return {};
37+
}
38+
}, []);
39+
40+
const effectiveUser = isEmpty(user) ? sessionSnapshot : user;
41+
42+
return { ...storageBuffer, ...effectiveUser };
3043
};

packages/framework/src/hooks/useScreenContext.tsx

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import {
1010
screenDataAtom,
1111
screenModelAtom,
1212
themeModelAtom,
13-
userAtom,
1413
} from "../state";
1514
import type {
1615
ApplicationContextDefinition,
@@ -77,16 +76,10 @@ const HydrateAtoms: React.FC<
7776

7877
// initialising on state with prop on render here
7978
useHydrateAtoms([[screenAtom, screenContext]]);
80-
useHydrateAtoms(
81-
[
82-
[appAtom, appContext],
83-
[themeModelAtom, themeScope.theme],
84-
[userAtom, appContext.user],
85-
],
86-
{
87-
dangerouslyForceHydrate: true,
88-
},
89-
);
79+
useHydrateAtoms([
80+
[appAtom, appContext],
81+
[themeModelAtom, themeScope.theme],
82+
]);
9083

9184
// initiate device resizer observer
9285
useDeviceObserver();

packages/framework/src/state/user.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,42 @@ const atomWithSessionStorage = <T = unknown>(
1717
}
1818
return initialValue;
1919
};
20+
// initialise from sessionStorage so first render sees the latest value
2021
const baseAtom = atom<T>(getInitialValue());
22+
23+
// keep all jotai stores in sync by listening to storage events
24+
// this is required because we create nested Providers (separate stores)
25+
// and without syncing, a later Provider may see stale values
26+
baseAtom.onMount = (setAtom) => {
27+
// set initial value from storage on first mount for this store
28+
try {
29+
setAtom(getInitialValue());
30+
} catch {
31+
setAtom(initialValue);
32+
}
33+
34+
const handler = (event: StorageEvent): void => {
35+
if (event.key !== key) return;
36+
try {
37+
const next = event.newValue
38+
? (JSON.parse(event.newValue) as T)
39+
: getInitialValue();
40+
setAtom(next);
41+
} catch {
42+
setAtom(getInitialValue());
43+
}
44+
};
45+
46+
if (typeof window !== "undefined") {
47+
window.addEventListener("storage", handler);
48+
}
49+
50+
return () => {
51+
if (typeof window !== "undefined") {
52+
window.removeEventListener("storage", handler);
53+
}
54+
};
55+
};
2156
const derivedAtom = atom<T, unknown[], unknown>(
2257
(get) => get(baseAtom),
2358
(get, set, update) => {
@@ -26,6 +61,10 @@ const atomWithSessionStorage = <T = unknown>(
2661
) as T;
2762
set(baseAtom, nextValue);
2863
sessionStorage.setItem(key, JSON.stringify(nextValue));
64+
// notify other stores in this tab to update immediately
65+
if (typeof window !== "undefined") {
66+
window.dispatchEvent(new StorageEvent("storage", { key }));
67+
}
2968
},
3069
);
3170
return derivedAtom;

0 commit comments

Comments
 (0)