From 4d2e50b55b97062b4db2de4cd49014d9b57cf559 Mon Sep 17 00:00:00 2001 From: Jannis Braun <151788261+TheZwiss@users.noreply.github.com> Date: Thu, 23 Apr 2026 02:46:19 +0200 Subject: [PATCH] fix(web): extract cross-store resolvers into neutral utility to break TDZ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit instanceStore registers three resolver functions at module load — setApiForOriginResolver, setUserIdForOriginResolver, setOriginFromHostnameResolver — whose backing `let` bindings used to live in spaceStore. When the module graph was entered from instanceStore (e.g. JoinSpaceModal importing useInstanceStore) the order became spaceStore → chatStore → useWebSocket → socialStore → instanceStore (top-level setter call) while spaceStore was still paused on its line-8 chatStore import, so the backing `let` had not been reached yet and the setter crashed with `Cannot access '_getApiForOrigin' before initialization`. This left InviteModal.test.tsx and JoinSpace.test.tsx unable to even load their suites once AudioManager was mocked away. Move the three `let` bindings, their setters, their pure getters, plus the WS-populated user-ID cache (`_myUserIdByOrigin`, setMyUserIdForOrigin, getCachedUserIdForOrigin, clearMyUserIdCache) into `packages/web/src/utils/crossStoreResolvers.ts`. The utility imports nothing from `./stores/*`, so no back-edge exists. spaceStore re-exports the public surface for backward compatibility with the many existing import sites; instanceStore imports the setters directly from the utility (the in-cycle re-export path does not resolve at module-init time under vite-ssr, so a direct import is required for the top-level setter calls). spaceStore's remaining wrappers (resolveUserOrigin, getLayoutHomeOrigin, getMyUserIdForOrigin) stay where they are — they combine the utility's pure lookups with authStore state — but now delegate to the utility. Also adds the AudioManager mock to InviteModal.test.tsx and JoinSpace.test.tsx so their suites actually load (same pattern already used in 5 other test files). Net test-suite result: 127/131 pass (up from 121/121 — +6 newly unlockable). The 4 remaining JoinSpace failures are pre-existing stale UI-text assertions (the placeholder was expanded and the submit button was made disable-when-empty) made visible by the suite now loading; they're orthogonal to this change and handed back for a separate triage. Closes backlog #27. --- docs/systems/client-federation.md | 12 ++- .../components/modals/InviteModal.test.tsx | 12 +++ .../src/components/modals/JoinSpace.test.tsx | 12 +++ packages/web/src/stores/instanceStore.ts | 7 +- packages/web/src/stores/spaceStore.ts | 79 +++++---------- packages/web/src/utils/crossStoreResolvers.ts | 99 +++++++++++++++++++ 6 files changed, 164 insertions(+), 57 deletions(-) create mode 100644 packages/web/src/utils/crossStoreResolvers.ts diff --git a/docs/systems/client-federation.md b/docs/systems/client-federation.md index b149c65b..cc9b34b3 100644 --- a/docs/systems/client-federation.md +++ b/docs/systems/client-federation.md @@ -5,7 +5,8 @@ Source files: - `packages/web/src/stores/instanceStore.ts` — Core multi-instance connection management, token caching, topology sync - `packages/web/src/hooks/useWebSocket.ts` — WebSocket multiplexing (one connection per instance), origin-aware event routing -- `packages/web/src/stores/spaceStore.ts` — Origin-aware space/channel store, `channelOriginMap`, `getChannelOrigin()`, `getApiForOrigin()`, DM deduplication +- `packages/web/src/stores/spaceStore.ts` — Origin-aware space/channel store, `channelOriginMap`, `getChannelOrigin()`, `resolveUserOrigin()`, `getLayoutHomeOrigin()`, `getMyUserIdForOrigin()`, DM deduplication +- `packages/web/src/utils/crossStoreResolvers.ts` — Neutral module holding the cross-store resolver bindings (`_getApiForOrigin`, `_resolveOriginFromHostname`, `_getUserIdForOrigin`) + the WS-populated user-ID cache. Breaks a TDZ cycle between spaceStore and instanceStore; see "API Client Resolution" below - `packages/web/src/utils/identity.ts` — Cross-instance user identity resolution (`isSelf`, `canonicalUserMatch`, self-ID registry) - `packages/web/src/hooks/useInstanceConnect.ts` — Connection flow hook for the Connections UI - `packages/web/src/components/modals/ConnectedInstances.tsx` — Connections settings panel @@ -184,10 +185,15 @@ getApiForOrigin(origin: string): BackspaceApiClient Returns the correct API client for the given origin. Uses a resolver pattern to break circular dependencies between stores: -- `instanceStore` registers the resolver at module init -- `spaceStore` exposes `getApiForOrigin()` which calls the registered resolver +- The resolver backing, its setter (`setApiForOriginResolver`), and the getter (`getApiForOrigin`) live in `packages/web/src/utils/crossStoreResolvers.ts` — a neutral module with no store imports +- `instanceStore` imports the setter from the utility directly (not from `spaceStore`) and registers the resolver at module init +- `spaceStore` re-exports `getApiForOrigin` (and its sibling setters) from the utility for backward compatibility with existing import sites - Consumers call `getApiForOrigin(getChannelOrigin(channelId))` to get the right client +The same pattern covers `resolveOriginFromHostname` (for `resolveUserOrigin`), the user-ID resolver (`resolveUserIdFromInstances`), and the WS-populated user-ID cache (`setMyUserIdForOrigin` / `getCachedUserIdForOrigin` / `clearMyUserIdCache`). + +**Why the utility exists:** `instanceStore` runs top-level `setXResolver` calls at module load. If spaceStore holds the backing `let _getApiForOrigin` declaration AND the import chain reaches instanceStore while spaceStore is mid-load (e.g. via `JoinSpaceModal` importing `useInstanceStore` directly), the setter crashes with TDZ: `Cannot access '_getApiForOrigin' before initialization`. Hoisting the mutable bindings into a module that has no back-edges into the stores eliminates the cycle. Do NOT add imports from `./stores/*` into `crossStoreResolvers.ts` — doing so re-creates the exact cycle that module was carved out to break. + ### User Origin Resolution ```typescript diff --git a/packages/web/src/components/modals/InviteModal.test.tsx b/packages/web/src/components/modals/InviteModal.test.tsx index dff97485..0ef1ecd9 100644 --- a/packages/web/src/components/modals/InviteModal.test.tsx +++ b/packages/web/src/components/modals/InviteModal.test.tsx @@ -1,6 +1,18 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; + +// Stub AudioManager to avoid AudioWorkletNode reference error in jsdom. +// Reached transitively via spaceStore → chatStore → useWebSocket → voiceStore. +vi.mock('../../audio/AudioManager', () => ({ + AudioManager: { + getInstance: vi.fn().mockReturnValue({ + setOutputDevice: vi.fn(), + setVolume: vi.fn(), + }), + }, +})); + import { InviteModal } from './InviteModal'; import { useUIStore } from '../../stores/uiStore'; import { useSpaceStore } from '../../stores/spaceStore'; diff --git a/packages/web/src/components/modals/JoinSpace.test.tsx b/packages/web/src/components/modals/JoinSpace.test.tsx index 23c8aefb..f86cb364 100644 --- a/packages/web/src/components/modals/JoinSpace.test.tsx +++ b/packages/web/src/components/modals/JoinSpace.test.tsx @@ -2,6 +2,18 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { MemoryRouter } from 'react-router-dom'; + +// Stub AudioManager to avoid AudioWorkletNode reference error in jsdom. +// Reached transitively via spaceStore → chatStore → useWebSocket → voiceStore. +vi.mock('../../audio/AudioManager', () => ({ + AudioManager: { + getInstance: vi.fn().mockReturnValue({ + setOutputDevice: vi.fn(), + setVolume: vi.fn(), + }), + }, +})); + import { JoinSpaceModal } from './JoinSpace'; import { useUIStore } from '../../stores/uiStore'; import { useSpaceStore } from '../../stores/spaceStore'; diff --git a/packages/web/src/stores/instanceStore.ts b/packages/web/src/stores/instanceStore.ts index 7dcbfa26..05af419b 100644 --- a/packages/web/src/stores/instanceStore.ts +++ b/packages/web/src/stores/instanceStore.ts @@ -2,7 +2,12 @@ import { create } from 'zustand'; import type { User, InstanceInfoResponse, ReplicatedInstance, AuthResponse, FederationRegistryEntry } from '@backspace/shared'; import { BackspaceApiClient, createApiClient, api } from '../api/client'; import { useAuthStore } from './authStore'; -import { setApiForOriginResolver, setUserIdForOriginResolver, setOriginFromHostnameResolver, useSpaceStore } from './spaceStore'; +import { + setApiForOriginResolver, + setUserIdForOriginResolver, + setOriginFromHostnameResolver, +} from '../utils/crossStoreResolvers'; +import { useSpaceStore } from './spaceStore'; import { connectInstance, disconnectInstance as disconnectWs, disconnectAllRemote } from '../hooks/useWebSocket'; // Circular dependency: federationOps imports useInstanceStore, instanceStore imports this. // Safe because both modules access each other lazily (at call time, not import time). diff --git a/packages/web/src/stores/spaceStore.ts b/packages/web/src/stores/spaceStore.ts index c64023b3..df4f424b 100644 --- a/packages/web/src/stores/spaceStore.ts +++ b/packages/web/src/stores/spaceStore.ts @@ -4,6 +4,13 @@ import { api, BackspaceApiClient } from '../api/client'; import { resolveAssetUrl, normalizeUserAssets } from '../utils/assetUrls'; import { isSelf } from '../utils/identity'; import { sortDmChannels } from '../utils/dmSorting'; +import { + getApiForOrigin, + resolveOriginFromHostname, + resolveUserIdFromInstances, + getCachedUserIdForOrigin, + clearMyUserIdCache, +} from '../utils/crossStoreResolvers'; import { useAuthStore } from './authStore'; import { useChatStore } from './chatStore'; @@ -139,7 +146,7 @@ export const useSpaceStore = create((set, get) => ({ _layoutUpdatedAt: 0, reset: () => { - _myUserIdByOrigin.clear(); + clearMyUserIdCache(); set({ spaces: [], currentSpaceId: null, @@ -951,36 +958,20 @@ export function resolveDmChannelId(rawId: string): string | null { return null; } -// ─── API client resolution ──────────────────────────────────────────────────── -// The actual resolver is registered by instanceStore on import, avoiding a -// circular dependency (instanceStore → useWebSocket → chatStore → spaceStore). - -let _getApiForOrigin: ((origin: string) => BackspaceApiClient) | null = null; - -export function setApiForOriginResolver(resolver: (origin: string) => BackspaceApiClient): void { - _getApiForOrigin = resolver; -} - -/** - * Returns the correct API client for a given instance origin. - * '' or falsy = home instance, 'https://...' = remote instance. - * The resolver is registered by instanceStore on import. - */ -export function getApiForOrigin(origin: string): BackspaceApiClient { - if (!origin || !_getApiForOrigin) return api; - return _getApiForOrigin(origin); -} - -// ─── Hostname → origin resolution (federation) ──────────────────────────────── -// Converts a user's `homeInstance` hostname (e.g. "remote.example.com") to a -// full origin URL (e.g. "https://remote.example.com") by looking up connected -// instances. Registered by instanceStore on import. - -let _resolveOriginFromHostname: ((hostname: string) => string) | null = null; - -export function setOriginFromHostnameResolver(resolver: (hostname: string) => string): void { - _resolveOriginFromHostname = resolver; -} +// ─── Cross-store resolvers (federation) ─────────────────────────────────────── +// The resolver/setter pairs and the WS-populated user-ID cache live in +// `utils/crossStoreResolvers.ts` — a neutral module with no store imports — +// to break a TDZ cycle: instanceStore registers these at top-level load, but +// a spaceStore-rooted import chain leaves spaceStore mid-load when that code +// runs. Re-exported here for backward compatibility with existing import +// sites. See the header comment in crossStoreResolvers.ts for details. +export { + setApiForOriginResolver, + getApiForOrigin, + setOriginFromHostnameResolver, + setUserIdForOriginResolver, + setMyUserIdForOrigin, +} from '../utils/crossStoreResolvers'; /** * Returns the instance origin for a federated user based on their homeInstance. @@ -989,7 +980,7 @@ export function setOriginFromHostnameResolver(resolver: (hostname: string) => st export function resolveUserOrigin(user: { homeInstance?: string | null }): string { const host = user.homeInstance; if (!host || host === window.location.host) return ''; - return _resolveOriginFromHostname?.(host) ?? ''; + return resolveOriginFromHostname(host); } /** @@ -1000,25 +991,7 @@ export function resolveUserOrigin(user: { homeInstance?: string | null }): strin export function getLayoutHomeOrigin(): string { const user = useAuthStore.getState().user; if (!user?.homeInstance) return ''; - return _resolveOriginFromHostname?.(user.homeInstance) ?? ''; -} - -// ─── User ID resolution (federation) ────────────────────────────────────────── -// Same resolver pattern as getApiForOrigin — registered by instanceStore on -// import to break the circular dependency chain. - -let _getUserIdForOrigin: ((origin: string) => string | undefined) | null = null; - -export function setUserIdForOriginResolver(resolver: (origin: string) => string | undefined): void { - _getUserIdForOrigin = resolver; -} - -// Direct identity cache populated from WS ready events. -// Bypasses the instanceStore resolver for reliable federation support. -const _myUserIdByOrigin = new Map(); - -export function setMyUserIdForOrigin(origin: string, userId: string): void { - _myUserIdByOrigin.set(origin, userId); + return resolveOriginFromHostname(user.homeInstance); } /** @@ -1029,8 +1002,8 @@ export function setMyUserIdForOrigin(origin: string, userId: string): void { export function getMyUserIdForOrigin(origin: string): string | undefined { if (!origin) return useAuthStore.getState().user?.id; // Direct cache (populated from WS ready) takes priority — always correct - const cached = _myUserIdByOrigin.get(origin); + const cached = getCachedUserIdForOrigin(origin); if (cached) return cached; // Fallback to instanceStore resolver (may have placeholder user during connection) - return _getUserIdForOrigin?.(origin); + return resolveUserIdFromInstances(origin); } diff --git a/packages/web/src/utils/crossStoreResolvers.ts b/packages/web/src/utils/crossStoreResolvers.ts new file mode 100644 index 00000000..8b385b26 --- /dev/null +++ b/packages/web/src/utils/crossStoreResolvers.ts @@ -0,0 +1,99 @@ +// Cross-store resolvers for federation identity and API client routing. +// +// This module exists SOLELY to break a temporal-dead-zone (TDZ) cycle between +// spaceStore and instanceStore. instanceStore runs top-level `setXResolver` +// calls at module load; the backing `let` bindings used to live in spaceStore. +// When the import graph is entered from instanceStore (e.g. via a component +// like JoinSpaceModal that imports `useInstanceStore` directly) — or any chain +// that resolves in an order where spaceStore is mid-load when instanceStore's +// top-level code runs — the `let _getApiForOrigin = null` declaration has not +// been evaluated yet, so the setter crashes with +// `Cannot access '_getApiForOrigin' before initialization`. +// +// The fix: hold the resolvers, their setters, and the local user-ID cache +// in this neutral module. It imports nothing from any store, so no back-edge +// exists and module load order between spaceStore / instanceStore / anything +// else cannot cause TDZ. spaceStore re-exports the public surface for +// backward compatibility with existing import sites. +// +// IMPORTANT: do not add imports from any `./stores/*` module here. Doing so +// re-creates the exact cycle this module was carved out to break. + +import { api, BackspaceApiClient } from '../api/client'; + +// ─── API client resolution ──────────────────────────────────────────────────── +// Registered by instanceStore on import; maps an origin to the instance- +// specific API client. `'' | null | undefined` origin always returns the home +// `api` client. + +let _getApiForOrigin: ((origin: string) => BackspaceApiClient) | null = null; + +export function setApiForOriginResolver( + resolver: (origin: string) => BackspaceApiClient +): void { + _getApiForOrigin = resolver; +} + +export function getApiForOrigin(origin: string): BackspaceApiClient { + if (!origin || !_getApiForOrigin) return api; + return _getApiForOrigin(origin); +} + +// ─── Hostname → origin resolution (federation) ──────────────────────────────── +// Registered by instanceStore on import; maps a federated user's `homeInstance` +// hostname (e.g. "remote.example.com") to a full origin URL +// (e.g. "https://remote.example.com") by looking up connected instances. + +let _resolveOriginFromHostname: ((hostname: string) => string) | null = null; + +export function setOriginFromHostnameResolver( + resolver: (hostname: string) => string +): void { + _resolveOriginFromHostname = resolver; +} + +/** + * Pure hostname→origin lookup. Returns '' if the resolver is not yet + * registered or the hostname is unknown. Callers that need to combine this + * with `window.location.host` or `authStore` state should layer on top + * (see `resolveUserOrigin` / `getLayoutHomeOrigin` in spaceStore). + */ +export function resolveOriginFromHostname(hostname: string): string { + return _resolveOriginFromHostname?.(hostname) ?? ''; +} + +// ─── User ID resolution (federation) ────────────────────────────────────────── +// Registered by instanceStore on import; maps an origin to the local user's +// ID on that instance. Used as a fallback for `getMyUserIdForOrigin` when the +// direct WS-populated cache below has not yet been filled. + +let _getUserIdForOrigin: ((origin: string) => string | undefined) | null = null; + +export function setUserIdForOriginResolver( + resolver: (origin: string) => string | undefined +): void { + _getUserIdForOrigin = resolver; +} + +export function resolveUserIdFromInstances(origin: string): string | undefined { + return _getUserIdForOrigin?.(origin); +} + +// ─── Direct user-ID cache (populated from WS ready events) ──────────────────── +// Bypasses the instanceStore resolver for reliable federation support even +// while `instanceStore.instances[].user` is still a placeholder mid-connection. + +const _myUserIdByOrigin = new Map(); + +export function setMyUserIdForOrigin(origin: string, userId: string): void { + _myUserIdByOrigin.set(origin, userId); +} + +export function getCachedUserIdForOrigin(origin: string): string | undefined { + return _myUserIdByOrigin.get(origin); +} + +/** Clears the WS-populated user-ID cache. Called by spaceStore.reset() on logout. */ +export function clearMyUserIdCache(): void { + _myUserIdByOrigin.clear(); +}