diff --git a/apps/web/src/components/structures/MatrixChat.test.tsx b/apps/web/src/components/structures/MatrixChat.test.tsx index d5eb98a4c7..8b3e7ef3aa 100644 --- a/apps/web/src/components/structures/MatrixChat.test.tsx +++ b/apps/web/src/components/structures/MatrixChat.test.tsx @@ -329,13 +329,16 @@ describe("", () => { localStorage.clear(); vi.clearAllTimers(); + // RTL cleanup won't touch roots we render ourselves so clean those up manually + await clearAllModals(); + // This is a massive hack, but a lot of these tests end up completing while the login flow is still proceeding. // So then, we start the next test while stuff is still ongoing from the previous test, which messes up the current test. // There is no obvious event we could wait for which indicates that everything has completed, // since each test does something different. Instead, we just let real timers and microtasks drain. await act(() => sleep(200)); - // RTL cleanup won't touch roots we render ourselves so clean those up manually + // Anything the drain kicked off may have opened a dialog again await clearAllModals(); }); diff --git a/apps/web/src/test/setupTests.ts b/apps/web/src/test/setupTests.ts index 13f8f64cb0..03f24c0185 100644 --- a/apps/web/src/test/setupTests.ts +++ b/apps/web/src/test/setupTests.ts @@ -6,7 +6,8 @@ Please see LICENSE files in the repository root for full details. */ import { vi, beforeEach, afterEach } from "vitest"; -import fetchMock, { manageFetchMockGlobally } from "@fetch-mock/vitest"; +import { act } from "react"; +import fetchMock from "@fetch-mock/vitest"; import SdkConfig, { DEFAULTS } from "../SdkConfig"; import "./setupGlobals.ts"; @@ -18,29 +19,19 @@ declare global { globalThis.IS_REACT_ACT_ENVIRONMENT = true; -// Ignore benign post-teardown exceptions as they cause flakes -const guardState = globalThis as unknown as { __vitestTestRunning?: boolean; __teardownGuardInstalled?: boolean }; -if (!guardState.__teardownGuardInstalled) { - guardState.__teardownGuardInstalled = true; - const isPostTeardownStraggler = (): boolean => !guardState.__vitestTestRunning || typeof window === "undefined"; - process.on("uncaughtException", (err) => { - if (isPostTeardownStraggler()) return; - throw err; - }); - process.on("unhandledRejection", (reason) => { - if (isPostTeardownStraggler()) return; - throw reason; - }); -} +// Captured before any test can install fake timers, so the drain in `afterEach` below always +// runs against a real immediate and cannot hang. +const realSetImmediate = globalThis.setImmediate; -manageFetchMockGlobally(); +// Deliberately *not* calling `manageFetchMockGlobally()` as it monkey-patches `vi.restoreAllMocks`, +// `vi.resetAllMocks` and `vi.unstubAllGlobals` such that they also tear the fetch mock down, putting the +// environment's real `fetch` back on the global. +// We re-set the mock before every test below, so the lifecycle integration buys us nothing. beforeEach(() => { - guardState.__vitestTestRunning = true; - vi.stubEnv("TZ", "UTC"); - // set up fetch API mock + // set up fetch API mock. Unmatched requests 404 rather than reaching the network. fetchMock.hardReset(); fetchMock.catch(404); fetchMock.mockGlobal(); @@ -48,9 +39,12 @@ beforeEach(() => { setupLanguageMock(); }); -afterEach(() => { - guardState.__vitestTestRunning = false; - return fetchMock.callHistory.flush(); +afterEach(async () => { + await fetchMock.callHistory.flush(); + + await act(async () => { + await new Promise((resolve) => realSetImmediate(resolve)); + }); }); // uninitialised SdkConfig causes lots of warnings in console, init with defaults diff --git a/apps/web/src/vector/init.test.ts b/apps/web/src/vector/init.test.ts index 70a9e8f67d..37d0582be8 100644 --- a/apps/web/src/vector/init.test.ts +++ b/apps/web/src/vector/init.test.ts @@ -57,6 +57,27 @@ describe("loadApp", () => { await waitFor(() => expect(window.matrixChat).toBeInstanceOf(MatrixChat)); }); + it("should replace the previous app rather than leaving it mounted", async () => { + await loadApp({}); + await waitFor(() => expect(window.matrixChat).toBeInstanceOf(MatrixChat)); + const first = window.matrixChat; + + // Count only what the second load does. We track the mounted/unmounted delta rather than raw + // mount count, as StrictMode's extra mount/unmount cycle cancels out in the difference. + const mounted = vi.spyOn(MatrixChat.prototype, "componentDidMount"); + const unmounted = vi.spyOn(MatrixChat.prototype, "componentWillUnmount"); + const delta = (): number => mounted.mock.calls.length - unmounted.mock.calls.length; + + setUpMatrixChatDiv(); + await loadApp({}); + await waitFor(() => expect(window.matrixChat).not.toBe(first)); + + // The new app replaces the old one, so the number of live apps is unchanged. A second root over + // the same container would instead leave the first tree mounted against a detached node, with both + // copies still driven by the dispatcher and the client peg. + await waitFor(() => expect(delta()).toBe(0)); + }); + it("should pass onTokenLoginCompleted which strips searchParams & fragment to MatrixChat", async () => { const spy = vi.spyOn(window.history, "replaceState"); diff --git a/apps/web/src/vector/init.tsx b/apps/web/src/vector/init.tsx index 90ebb2dde6..208f29d0bf 100644 --- a/apps/web/src/vector/init.tsx +++ b/apps/web/src/vector/init.tsx @@ -8,7 +8,7 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com Please see LICENSE files in the repository root for full details. */ -import { createRoot } from "react-dom/client"; +import { createRoot, type Root } from "react-dom/client"; import React, { StrictMode } from "react"; import { logger } from "matrix-js-sdk/src/logger"; import { ModuleLoader } from "@element-hq/element-web-module-api"; @@ -32,6 +32,25 @@ import { type URLParams } from "./url_utils.ts"; export const rageshakePromise = initRageshake(); +let root: Root | undefined; +let rootContainer: Element | undefined; + +/** + * Get the React root for the `#matrixchat` container. + * + * These views replace one another (`showError` may be called after `loadApp`, for instance), so they share a + * single root: calling `createRoot` again for the same container leaves the previous tree mounted and running + * against a detached DOM node, with both copies still subscribed to the dispatcher and the client peg. + */ +function getRoot(): Root { + const container = document.getElementById("matrixchat")!; + if (root && rootContainer === container) return root; + root?.unmount(); + rootContainer = container; + root = createRoot(container); + return root; +} + export function preparePlatform(): void { if (window.electron) { logger.log("Using Electron platform"); @@ -98,8 +117,7 @@ export async function loadApp(urlParams: URLParams): Promise { window.matrixChat = matrixChat; } const app = await module.loadApp(urlParams, setWindowMatrixChat); - const root = createRoot(document.getElementById("matrixchat")!); - root.render(app); + getRoot().render(app); } export async function showError(title: string, messages?: string[]): Promise { @@ -107,8 +125,7 @@ export async function showError(title: string, messages?: string[]): Promise , @@ -120,8 +137,7 @@ export async function showIncompatibleBrowser(onAccept: () => void): Promise ,