diff --git a/.changeset/tidy-toolbar-subscriptions.md b/.changeset/tidy-toolbar-subscriptions.md new file mode 100644 index 000000000..5c8b68fe6 --- /dev/null +++ b/.changeset/tidy-toolbar-subscriptions.md @@ -0,0 +1,5 @@ +--- +"@reflag/browser-sdk": patch +--- + +Unsubscribe toolbar flag listeners when the toolbar is unmounted or switches clients. diff --git a/packages/browser-sdk/src/toolbar/Toolbar.tsx b/packages/browser-sdk/src/toolbar/Toolbar.tsx index 363074469..8c8ac5200 100644 --- a/packages/browser-sdk/src/toolbar/Toolbar.tsx +++ b/packages/browser-sdk/src/toolbar/Toolbar.tsx @@ -68,7 +68,7 @@ export default function Toolbar({ useEffect(() => { updateFlags(); - reflagClient.on("flagsUpdated", updateFlags); + return reflagClient.on("flagsUpdated", updateFlags); }, [reflagClient, updateFlags]); const [search, setSearch] = useState(null); diff --git a/packages/browser-sdk/test/cleanupUi.test.ts b/packages/browser-sdk/test/cleanupUi.test.ts new file mode 100644 index 000000000..d3e5dd326 --- /dev/null +++ b/packages/browser-sdk/test/cleanupUi.test.ts @@ -0,0 +1,89 @@ +import { options } from "preact"; +import { afterEach, beforeEach, expect, test, vi } from "vitest"; + +import { ReflagClient } from "../src/client"; +import { showToolbarToggle } from "../src/toolbar"; +import { toolbarContainerId } from "../src/ui/constants"; +import { cleanupUi } from "./cleanupUi"; + +let effects: (() => void)[]; +let renders: (() => void)[]; +const originalRaf = options.requestAnimationFrame; +const originalDebounce = options.debounceRendering; + +beforeEach(() => { + effects = []; + renders = []; + // Hold the actual Preact callbacks so teardown timing is deterministic. + options.requestAnimationFrame = (callback) => effects.push(callback); + options.debounceRendering = (callback) => renders.push(callback); +}); + +function drainCallbacks() { + while (effects.length || renders.length) { + effects.shift()?.(); + renders.shift()?.(); + } +} + +function withoutSessionStorage(callback: () => void) { + const descriptor = Object.getOwnPropertyDescriptor( + globalThis, + "sessionStorage", + )!; + Reflect.deleteProperty(globalThis, "sessionStorage"); + try { + callback(); + } finally { + Object.defineProperty(globalThis, "sessionStorage", descriptor); + } +} + +function mountToolbar() { + const client = new ReflagClient({ + publishableKey: "test-key", + offline: true, + toolbar: false, + }); + const unsubscribe = vi.fn(); + const on = vi.spyOn(client, "on").mockReturnValue(unsubscribe); + showToolbarToggle({ reflagClient: client }); + expect(document.getElementById(toolbarContainerId)).not.toBeNull(); + return { on, unsubscribe }; +} + +afterEach(() => { + cleanupUi(); + drainCallbacks(); + options.requestAnimationFrame = originalRaf; + options.debounceRendering = originalDebounce; + vi.restoreAllMocks(); +}); + +test("unmounts before a delayed toolbar effect can run after jsdom teardown", () => { + const { on } = mountToolbar(); + expect(effects.length).toBeGreaterThan(0); + + cleanupUi(); + + withoutSessionStorage(() => { + expect(drainCallbacks).not.toThrow(); + }); + expect(on).not.toHaveBeenCalled(); + expect(document.getElementById(toolbarContainerId)).toBeNull(); +}); + +test("cancels queued toolbar renders and unsubscribes mounted effects", () => { + const { unsubscribe } = mountToolbar(); + // Run useEffect: updateFlags queues a render and subscribes to flagsUpdated. + while (effects.length) effects.shift()!(); + expect(renders.length).toBeGreaterThan(0); + + cleanupUi(); + + withoutSessionStorage(() => { + expect(drainCallbacks).not.toThrow(); + }); + expect(unsubscribe).toHaveBeenCalledOnce(); + expect(document.getElementById(toolbarContainerId)).toBeNull(); +}); diff --git a/packages/browser-sdk/test/cleanupUi.ts b/packages/browser-sdk/test/cleanupUi.ts new file mode 100644 index 000000000..387180d99 --- /dev/null +++ b/packages/browser-sdk/test/cleanupUi.ts @@ -0,0 +1,16 @@ +import { render } from "preact"; + +import { feedbackContainerId, toolbarContainerId } from "../src/ui/constants"; + +/** Unmount Preact before removing its hosts or tearing down jsdom globals. */ +export function cleanupUi() { + if (typeof document === "undefined") return; + + for (const id of [feedbackContainerId, toolbarContainerId]) { + const host = document.getElementById(id); + if (host?.shadowRoot) { + render(null, host.shadowRoot); + } + host?.remove(); + } +} diff --git a/packages/browser-sdk/test/usage.test.ts b/packages/browser-sdk/test/usage.test.ts index 86879005d..44e001f70 100644 --- a/packages/browser-sdk/test/usage.test.ts +++ b/packages/browser-sdk/test/usage.test.ts @@ -37,9 +37,6 @@ const clientNotInitializedError = { }; vi.mock("../src/sse"); -// These tests cover SDK usage, not toolbar rendering. Avoid scheduling Preact -// renders that can outlive the test's browser environment. -vi.mock("../src/toolbar", () => ({ showToolbarToggle: vi.fn() })); vi.mock("../src/feedback/promptStorage", () => { return { markPromptMessageCompleted: vi.fn(), diff --git a/packages/browser-sdk/vitest.setup.ts b/packages/browser-sdk/vitest.setup.ts index 9161f5f54..92decc881 100644 --- a/packages/browser-sdk/vitest.setup.ts +++ b/packages/browser-sdk/vitest.setup.ts @@ -1,5 +1,6 @@ import { afterAll, afterEach, beforeAll } from "vitest"; +import { cleanupUi } from "./test/cleanupUi"; import { server } from "./test/mocks/server.js"; beforeAll(() => { @@ -11,6 +12,7 @@ beforeAll(() => { }); afterEach(() => { + cleanupUi(); server.resetHandlers(); });