From b57e4fb9bcfe7d247777abdfde04c8c708eb33b9 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Fri, 14 Aug 2026 02:37:28 +0200 Subject: [PATCH] fix(channels): re-hydrate per-account manager state once the account is pinned MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The provider tree mounts above , which pins this tab's account only after /api/me resolves — so on a genuinely fresh tab (empty sessionStorage) the manager's sort, filters, view and layout initialised from an unknown account and stayed at defaults until a second reload. Make the active account reactive (a subscribable store + useActiveAccountId via useSyncExternalStore; setActiveAccount/clearActiveAccount notify) and re-read the stored value when the account transitions null→id, via a shared useAccountRehydrate hook. A markDirty guard (called by each owning setter) prevents clobbering a value the user changed in the brief pre-pin window; the effect is keyed on the account id only, so the user's own writes can't loop it. useSortState, useFilterState and ChannelsProvider (view/layout) all adopt it. Affects sort identically — this was a long-standing caveat, not new to R13. --- frontend/src/components/ChannelsProvider.tsx | 56 ++++++++++++------- frontend/src/lib/api/core.ts | 5 +- frontend/src/lib/storage.ts | 21 ++++++- .../src/lib/useAccountRehydrate.dom.test.ts | 45 +++++++++++++++ frontend/src/lib/useAccountRehydrate.ts | 30 ++++++++++ frontend/src/lib/useFilterState.dom.test.ts | 3 + frontend/src/lib/useFilterState.ts | 16 ++++-- frontend/src/lib/useSortState.dom.test.ts | 3 + frontend/src/lib/useSortState.ts | 8 ++- 9 files changed, 160 insertions(+), 27 deletions(-) create mode 100644 frontend/src/lib/useAccountRehydrate.dom.test.ts create mode 100644 frontend/src/lib/useAccountRehydrate.ts diff --git a/frontend/src/components/ChannelsProvider.tsx b/frontend/src/components/ChannelsProvider.tsx index 6e9a1425..68510534 100644 --- a/frontend/src/components/ChannelsProvider.tsx +++ b/frontend/src/components/ChannelsProvider.tsx @@ -13,6 +13,7 @@ import { parseChannelLayout, type ChannelLayout } from "../lib/channelLayout"; import { type SortState } from "../lib/columnSort"; import { useSortState } from "../lib/useSortState"; import { useFilterState } from "../lib/useFilterState"; +import { useAccountRehydrate } from "../lib/useAccountRehydrate"; import type { FilterMap } from "./tableHeader"; import { useNavigation, useNavigationActions } from "./NavigationProvider"; import type { ChannelsView } from "./Channels"; @@ -129,6 +130,15 @@ export function ChannelsProvider({ children }: { children: ReactNode }) { const [focusName, setFocusName] = useState(null); const [focusToken, setFocusToken] = useState(0); + // view + layout (like sort/filters) initialise from an unknown account on first load, because this + // provider mounts above , which pins the account only after /api/me resolves. Re-read them once + // the account is known. Uses the raw state setters (not setView/setLayout) so it's a silent + // correction, not a history-pushing navigation; markManagerDirty guards a user switch made pre-pin. + const markManagerDirty = useAccountRehydrate(() => { + setViewState(clampView(getAccountRaw(LS.channelsView))); + setLayoutState(parseChannelLayout(getAccountRaw(LS.channelLayout))); + }); + // Latest-value ref so the STABLE setView can compare against the current tab without depending on // it (a dep would recreate the action on every switch and re-render every actions consumer). // Written during render — the sanctioned pattern NavigationProvider uses for `pageRef`. @@ -139,25 +149,33 @@ export function ChannelsProvider({ children }: { children: ReactNode }) { // The active tab rides in history.state (`_chTab`), the same way the channel page (`_chan`) and the // live YouTube search (`_yt`) do — so Back steps back through the tabs you actually visited instead // of jumping straight out to the previous module. - const setView = useCallback((v: ChannelsView) => { - // Re-selecting the tab you are already on is not a navigation — bail before pushing, or clicking - // the active tab (and goToFullHistory forcing "subscribed") stacks Back entries that do nothing. - if (v === viewRef.current) return; - setViewState(v); - setAccountRaw(LS.channelsView, v); - // Only own an entry while the manager is already on screen. Arriving from elsewhere - // (focusChannel / goToFullHistory) lets setPage push the entry — the stamp effect labels it — so - // one navigation never costs two Back presses. - if (window.history.state?.sfPage === "channels") { - // A FRESH state, like setPage's: carrying the old entry's `_chan`/`_yt` markers over would make - // Back re-open the channel overlay or a spent search on top of the tab. - window.history.pushState({ sfPage: "channels", _chTab: v }, ""); - } - }, []); - const setLayout = useCallback((l: ChannelLayout) => { - setLayoutState(l); - setAccountRaw(LS.channelLayout, l); - }, []); + const setView = useCallback( + (v: ChannelsView) => { + // Re-selecting the tab you are already on is not a navigation — bail before pushing, or clicking + // the active tab (and goToFullHistory forcing "subscribed") stacks Back entries that do nothing. + if (v === viewRef.current) return; + markManagerDirty(); + setViewState(v); + setAccountRaw(LS.channelsView, v); + // Only own an entry while the manager is already on screen. Arriving from elsewhere + // (focusChannel / goToFullHistory) lets setPage push the entry — the stamp effect labels it — so + // one navigation never costs two Back presses. + if (window.history.state?.sfPage === "channels") { + // A FRESH state, like setPage's: carrying the old entry's `_chan`/`_yt` markers over would make + // Back re-open the channel overlay or a spent search on top of the tab. + window.history.pushState({ sfPage: "channels", _chTab: v }, ""); + } + }, + [markManagerDirty], + ); + const setLayout = useCallback( + (l: ChannelLayout) => { + markManagerDirty(); + setLayoutState(l); + setAccountRaw(LS.channelLayout, l); + }, + [markManagerDirty], + ); const focusChannel = useCallback( (name: string) => { setFocusName(name); diff --git a/frontend/src/lib/api/core.ts b/frontend/src/lib/api/core.ts index 815dae6c..4074d8fb 100644 --- a/frontend/src/lib/api/core.ts +++ b/frontend/src/lib/api/core.ts @@ -5,7 +5,7 @@ import { notify, remove } from "../notifications"; import i18n from "../../i18n"; import { reportError } from "../errorDialog"; -import { activeAccountId, ACTIVE_ACCOUNT_KEY } from "../storage"; +import { activeAccountId, ACTIVE_ACCOUNT_KEY, notifyActiveAccountChange } from "../storage"; export class HttpError extends Error { status: number; @@ -136,6 +136,8 @@ export function setActiveAccount(id: number): void { } catch { /* ignore */ } + // Let per-account UI state re-hydrate now the account is known (see notifyActiveAccountChange). + notifyActiveAccountChange(); } export function clearActiveAccount(): void { try { @@ -143,6 +145,7 @@ export function clearActiveAccount(): void { } catch { /* ignore */ } + notifyActiveAccountChange(); } export async function req( diff --git a/frontend/src/lib/storage.ts b/frontend/src/lib/storage.ts index 2e12bc97..16e4bdcc 100644 --- a/frontend/src/lib/storage.ts +++ b/frontend/src/lib/storage.ts @@ -1,4 +1,4 @@ -import { useCallback, useState } from "react"; +import { useCallback, useState, useSyncExternalStore } from "react"; // Central registry of every localStorage key the app uses. One place to see them all, rename // safely, and avoid collisions — instead of `"siftlode.x"` string literals scattered across @@ -83,6 +83,25 @@ export function activeAccountId(): number | null { } } +// Reactive active-account. setActiveAccount/clearActiveAccount write sessionStorage — a silent op that +// triggers no React render — so this tab's per-account state (sort/filter/view/layout, all keyed by the +// account) would never re-read once the account is finally known. That bites exactly once, on first +// load: the provider tree above mounts with NO account, then pins it after /api/me resolves. +// The account setters call notifyActiveAccountChange() so useActiveAccountId subscribers can re-hydrate. +const accountListeners = new Set<() => void>(); +function subscribeActiveAccount(cb: () => void): () => void { + accountListeners.add(cb); + return () => accountListeners.delete(cb); +} +export function notifyActiveAccountChange(): void { + for (const cb of [...accountListeners]) cb(); +} +/** The current tab's active account id, as reactive state — re-renders when it is (re)pinned. The + * snapshot is a primitive (number | null), so useSyncExternalStore's Object.is check is stable. */ +export function useActiveAccountId(): number | null { + return useSyncExternalStore(subscribeActiveAccount, activeAccountId, activeAccountId); +} + /** Suffix a base localStorage key with an account id so per-account UI state (filters, selected * playlist, notifications, tab positions, cached prefs…) is NOT shared across the accounts signed * into one browser — a bare shared key leaks one account's state into another (and, with per-tab diff --git a/frontend/src/lib/useAccountRehydrate.dom.test.ts b/frontend/src/lib/useAccountRehydrate.dom.test.ts new file mode 100644 index 00000000..a9aac577 --- /dev/null +++ b/frontend/src/lib/useAccountRehydrate.dom.test.ts @@ -0,0 +1,45 @@ +// @vitest-environment jsdom +// +// useAccountRehydrate re-reads per-account state when the tab's account is pinned after mount (first +// load). useActiveAccountId is mocked with a plain variable so the transition can be driven by hand. +import { describe, it, expect, beforeEach, vi } from "vitest"; +import { renderHook, act } from "@testing-library/react"; + +let account: number | null = null; +vi.mock("./storage", () => ({ + useActiveAccountId: () => account, +})); + +import { useAccountRehydrate } from "./useAccountRehydrate"; + +describe("useAccountRehydrate", () => { + beforeEach(() => { + account = null; + }); + + it("runs rehydrate when the account transitions from null to a value", () => { + const rehydrate = vi.fn(); + const { rerender } = renderHook(() => useAccountRehydrate(rehydrate)); + expect(rehydrate).not.toHaveBeenCalled(); // mount with no account: no transition + account = 1; + act(() => rerender()); + expect(rehydrate).toHaveBeenCalledTimes(1); + }); + + it("does not run when the account id is already known and stable", () => { + account = 1; + const rehydrate = vi.fn(); + const { rerender } = renderHook(() => useAccountRehydrate(rehydrate)); + act(() => rerender()); + expect(rehydrate).not.toHaveBeenCalled(); + }); + + it("skips rehydrate when marked dirty before the account pins (no clobber of a user edit)", () => { + const rehydrate = vi.fn(); + const { result, rerender } = renderHook(() => useAccountRehydrate(rehydrate)); + act(() => result.current()); // markDirty — the owning setter ran pre-pin + account = 1; + act(() => rerender()); + expect(rehydrate).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/lib/useAccountRehydrate.ts b/frontend/src/lib/useAccountRehydrate.ts new file mode 100644 index 00000000..980ea6f6 --- /dev/null +++ b/frontend/src/lib/useAccountRehydrate.ts @@ -0,0 +1,30 @@ +import { useCallback, useEffect, useRef } from "react"; +import { useActiveAccountId } from "./storage"; + +// Re-read per-account persisted state when this tab's active account becomes known (or changes). The +// provider tree above mounts before the account is pinned, so its useState initializers seed from +// an unknown account (empty defaults); once pins it, `rehydrate` runs to load the real stored +// value. Guarded so it never clobbers a value the user set in the brief pre-pin window: the returned +// `markDirty` is called by the owning setter, and a dirty state skips the re-hydrate. +// +// Keyed on the account id ONLY — the user's own writes go through setAccountRaw, which does not change +// the account id, so this effect can't loop on them. +export function useAccountRehydrate(rehydrate: () => void): () => void { + const accountId = useActiveAccountId(); + const dirty = useRef(false); + const prev = useRef(accountId); + // Latest closure in a ref so the effect depends on the account id alone (a fresh `rehydrate` each + // render must not re-fire it), while still calling the current logic. + const fn = useRef(rehydrate); + fn.current = rehydrate; + + useEffect(() => { + if (accountId === prev.current) return; // mount, and renders with no real transition + prev.current = accountId; + if (!dirty.current) fn.current(); + }, [accountId]); + + return useCallback(() => { + dirty.current = true; + }, []); +} diff --git a/frontend/src/lib/useFilterState.dom.test.ts b/frontend/src/lib/useFilterState.dom.test.ts index ea692198..bac35ee8 100644 --- a/frontend/src/lib/useFilterState.dom.test.ts +++ b/frontend/src/lib/useFilterState.dom.test.ts @@ -12,6 +12,9 @@ vi.mock("./storage", () => ({ setAccountRaw: (k: string, v: string) => { store.set(k, v); }, + // A stable account id keeps useAccountRehydrate a no-op here (no null→id transition); the re-hydrate + // path itself is covered in useAccountRehydrate.dom.test.ts. + useActiveAccountId: () => 1, })); import { useFilterState } from "./useFilterState"; diff --git a/frontend/src/lib/useFilterState.ts b/frontend/src/lib/useFilterState.ts index 469a7fcf..eea933a8 100644 --- a/frontend/src/lib/useFilterState.ts +++ b/frontend/src/lib/useFilterState.ts @@ -1,5 +1,6 @@ import { useCallback, useState } from "react"; import { getAccountRaw, setAccountRaw } from "./storage"; +import { useAccountRehydrate } from "./useAccountRehydrate"; import type { FilterMap } from "../components/tableHeader"; // The per-account persisted column-filter map for a DataTable whose filters are CONTROLLED by the @@ -13,17 +14,22 @@ export function useFilterState( // flash); it stays idempotent because the seed persists as soon as the user touches the filters. migrate?: () => FilterMap | null, ): [FilterMap, (next: FilterMap) => void] { - const [filters, setFilters] = useState(() => { + // Read the account's stored map, or the migrate seed when its key is still empty. Shared by the + // initial mount AND the account-pin re-hydrate, so first-load migration runs on whichever fires with + // the account known (on a fresh tab the account is null at mount → the re-hydrate does the real read). + const read = useCallback((): FilterMap => { const raw = getAccountRaw(storageBase); - if (raw) return hydrate(raw); - return migrate?.() ?? {}; - }); + return raw ? hydrate(raw) : (migrate?.() ?? {}); + }, [storageBase, migrate]); + const [filters, setFilters] = useState(read); + const markDirty = useAccountRehydrate(() => setFilters(read())); const set = useCallback( (next: FilterMap) => { + markDirty(); setFilters(next); setAccountRaw(storageBase, JSON.stringify(next)); }, - [storageBase], + [storageBase, markDirty], ); return [filters, set]; } diff --git a/frontend/src/lib/useSortState.dom.test.ts b/frontend/src/lib/useSortState.dom.test.ts index 98989e9f..4cf751f7 100644 --- a/frontend/src/lib/useSortState.dom.test.ts +++ b/frontend/src/lib/useSortState.dom.test.ts @@ -12,6 +12,9 @@ vi.mock("./storage", () => ({ setAccountRaw: (k: string, v: string) => { store.set(k, v); }, + // A stable account id keeps useAccountRehydrate a no-op here (no null→id transition); the re-hydrate + // path itself is covered in useAccountRehydrate.dom.test.ts. + useActiveAccountId: () => 1, })); import { useSortState } from "./useSortState"; diff --git a/frontend/src/lib/useSortState.ts b/frontend/src/lib/useSortState.ts index 332b26f3..b9ad1c66 100644 --- a/frontend/src/lib/useSortState.ts +++ b/frontend/src/lib/useSortState.ts @@ -1,5 +1,6 @@ import { useCallback, useState } from "react"; import { getAccountRaw, setAccountRaw } from "./storage"; +import { useAccountRehydrate } from "./useAccountRehydrate"; import { hydrateSort, type SortOption, type SortState } from "./columnSort"; // The per-account persisted sort state, hydrated + healed against the current options — the plumbing @@ -13,12 +14,17 @@ export function useSortState( const [sort, setSort] = useState(() => hydrateSort(getAccountRaw(storageBase), options, fallback), ); + // When the account is pinned after mount (first load), re-read the stored sort for that account. + const markDirty = useAccountRehydrate(() => + setSort(hydrateSort(getAccountRaw(storageBase), options, fallback)), + ); const set = useCallback( (next: SortState) => { + markDirty(); setSort(next); setAccountRaw(storageBase, JSON.stringify(next)); }, - [storageBase], + [storageBase, markDirty], ); return [sort, set]; }