fix(channels): re-hydrate per-account manager state once the account is pinned
The provider tree mounts above <App>, 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.
This commit is contained in:
@@ -13,6 +13,7 @@ import { parseChannelLayout, type ChannelLayout } from "../lib/channelLayout";
|
|||||||
import { type SortState } from "../lib/columnSort";
|
import { type SortState } from "../lib/columnSort";
|
||||||
import { useSortState } from "../lib/useSortState";
|
import { useSortState } from "../lib/useSortState";
|
||||||
import { useFilterState } from "../lib/useFilterState";
|
import { useFilterState } from "../lib/useFilterState";
|
||||||
|
import { useAccountRehydrate } from "../lib/useAccountRehydrate";
|
||||||
import type { FilterMap } from "./tableHeader";
|
import type { FilterMap } from "./tableHeader";
|
||||||
import { useNavigation, useNavigationActions } from "./NavigationProvider";
|
import { useNavigation, useNavigationActions } from "./NavigationProvider";
|
||||||
import type { ChannelsView } from "./Channels";
|
import type { ChannelsView } from "./Channels";
|
||||||
@@ -129,6 +130,15 @@ export function ChannelsProvider({ children }: { children: ReactNode }) {
|
|||||||
const [focusName, setFocusName] = useState<string | null>(null);
|
const [focusName, setFocusName] = useState<string | null>(null);
|
||||||
const [focusToken, setFocusToken] = useState(0);
|
const [focusToken, setFocusToken] = useState(0);
|
||||||
|
|
||||||
|
// view + layout (like sort/filters) initialise from an unknown account on first load, because this
|
||||||
|
// provider mounts above <App>, 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
|
// 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).
|
// 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`.
|
// 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
|
// 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
|
// live YouTube search (`_yt`) do — so Back steps back through the tabs you actually visited instead
|
||||||
// of jumping straight out to the previous module.
|
// of jumping straight out to the previous module.
|
||||||
const setView = useCallback((v: ChannelsView) => {
|
const setView = useCallback(
|
||||||
// Re-selecting the tab you are already on is not a navigation — bail before pushing, or clicking
|
(v: ChannelsView) => {
|
||||||
// the active tab (and goToFullHistory forcing "subscribed") stacks Back entries that do nothing.
|
// Re-selecting the tab you are already on is not a navigation — bail before pushing, or clicking
|
||||||
if (v === viewRef.current) return;
|
// the active tab (and goToFullHistory forcing "subscribed") stacks Back entries that do nothing.
|
||||||
setViewState(v);
|
if (v === viewRef.current) return;
|
||||||
setAccountRaw(LS.channelsView, v);
|
markManagerDirty();
|
||||||
// Only own an entry while the manager is already on screen. Arriving from elsewhere
|
setViewState(v);
|
||||||
// (focusChannel / goToFullHistory) lets setPage push the entry — the stamp effect labels it — so
|
setAccountRaw(LS.channelsView, v);
|
||||||
// one navigation never costs two Back presses.
|
// Only own an entry while the manager is already on screen. Arriving from elsewhere
|
||||||
if (window.history.state?.sfPage === "channels") {
|
// (focusChannel / goToFullHistory) lets setPage push the entry — the stamp effect labels it — so
|
||||||
// A FRESH state, like setPage's: carrying the old entry's `_chan`/`_yt` markers over would make
|
// one navigation never costs two Back presses.
|
||||||
// Back re-open the channel overlay or a spent search on top of the tab.
|
if (window.history.state?.sfPage === "channels") {
|
||||||
window.history.pushState({ sfPage: "channels", _chTab: v }, "");
|
// 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);
|
[markManagerDirty],
|
||||||
}, []);
|
);
|
||||||
|
const setLayout = useCallback(
|
||||||
|
(l: ChannelLayout) => {
|
||||||
|
markManagerDirty();
|
||||||
|
setLayoutState(l);
|
||||||
|
setAccountRaw(LS.channelLayout, l);
|
||||||
|
},
|
||||||
|
[markManagerDirty],
|
||||||
|
);
|
||||||
const focusChannel = useCallback(
|
const focusChannel = useCallback(
|
||||||
(name: string) => {
|
(name: string) => {
|
||||||
setFocusName(name);
|
setFocusName(name);
|
||||||
|
|||||||
@@ -5,7 +5,7 @@
|
|||||||
import { notify, remove } from "../notifications";
|
import { notify, remove } from "../notifications";
|
||||||
import i18n from "../../i18n";
|
import i18n from "../../i18n";
|
||||||
import { reportError } from "../errorDialog";
|
import { reportError } from "../errorDialog";
|
||||||
import { activeAccountId, ACTIVE_ACCOUNT_KEY } from "../storage";
|
import { activeAccountId, ACTIVE_ACCOUNT_KEY, notifyActiveAccountChange } from "../storage";
|
||||||
|
|
||||||
export class HttpError extends Error {
|
export class HttpError extends Error {
|
||||||
status: number;
|
status: number;
|
||||||
@@ -136,6 +136,8 @@ export function setActiveAccount(id: number): void {
|
|||||||
} catch {
|
} catch {
|
||||||
/* ignore */
|
/* ignore */
|
||||||
}
|
}
|
||||||
|
// Let per-account UI state re-hydrate now the account is known (see notifyActiveAccountChange).
|
||||||
|
notifyActiveAccountChange();
|
||||||
}
|
}
|
||||||
export function clearActiveAccount(): void {
|
export function clearActiveAccount(): void {
|
||||||
try {
|
try {
|
||||||
@@ -143,6 +145,7 @@ export function clearActiveAccount(): void {
|
|||||||
} catch {
|
} catch {
|
||||||
/* ignore */
|
/* ignore */
|
||||||
}
|
}
|
||||||
|
notifyActiveAccountChange();
|
||||||
}
|
}
|
||||||
|
|
||||||
export async function req<T = unknown>(
|
export async function req<T = unknown>(
|
||||||
|
|||||||
@@ -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
|
// 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
|
// 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 <App> mounts with NO account, then <App> 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
|
/** 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
|
* 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
|
* into one browser — a bare shared key leaks one account's state into another (and, with per-tab
|
||||||
|
|||||||
@@ -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();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -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 <App> mounts before the account is pinned, so its useState initializers seed from
|
||||||
|
// an unknown account (empty defaults); once <App> 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;
|
||||||
|
}, []);
|
||||||
|
}
|
||||||
@@ -12,6 +12,9 @@ vi.mock("./storage", () => ({
|
|||||||
setAccountRaw: (k: string, v: string) => {
|
setAccountRaw: (k: string, v: string) => {
|
||||||
store.set(k, v);
|
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";
|
import { useFilterState } from "./useFilterState";
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
import { useCallback, useState } from "react";
|
import { useCallback, useState } from "react";
|
||||||
import { getAccountRaw, setAccountRaw } from "./storage";
|
import { getAccountRaw, setAccountRaw } from "./storage";
|
||||||
|
import { useAccountRehydrate } from "./useAccountRehydrate";
|
||||||
import type { FilterMap } from "../components/tableHeader";
|
import type { FilterMap } from "../components/tableHeader";
|
||||||
|
|
||||||
// The per-account persisted column-filter map for a DataTable whose filters are CONTROLLED by the
|
// 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.
|
// flash); it stays idempotent because the seed persists as soon as the user touches the filters.
|
||||||
migrate?: () => FilterMap | null,
|
migrate?: () => FilterMap | null,
|
||||||
): [FilterMap, (next: FilterMap) => void] {
|
): [FilterMap, (next: FilterMap) => void] {
|
||||||
const [filters, setFilters] = useState<FilterMap>(() => {
|
// 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);
|
const raw = getAccountRaw(storageBase);
|
||||||
if (raw) return hydrate(raw);
|
return raw ? hydrate(raw) : (migrate?.() ?? {});
|
||||||
return migrate?.() ?? {};
|
}, [storageBase, migrate]);
|
||||||
});
|
const [filters, setFilters] = useState<FilterMap>(read);
|
||||||
|
const markDirty = useAccountRehydrate(() => setFilters(read()));
|
||||||
const set = useCallback(
|
const set = useCallback(
|
||||||
(next: FilterMap) => {
|
(next: FilterMap) => {
|
||||||
|
markDirty();
|
||||||
setFilters(next);
|
setFilters(next);
|
||||||
setAccountRaw(storageBase, JSON.stringify(next));
|
setAccountRaw(storageBase, JSON.stringify(next));
|
||||||
},
|
},
|
||||||
[storageBase],
|
[storageBase, markDirty],
|
||||||
);
|
);
|
||||||
return [filters, set];
|
return [filters, set];
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -12,6 +12,9 @@ vi.mock("./storage", () => ({
|
|||||||
setAccountRaw: (k: string, v: string) => {
|
setAccountRaw: (k: string, v: string) => {
|
||||||
store.set(k, v);
|
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";
|
import { useSortState } from "./useSortState";
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
import { useCallback, useState } from "react";
|
import { useCallback, useState } from "react";
|
||||||
import { getAccountRaw, setAccountRaw } from "./storage";
|
import { getAccountRaw, setAccountRaw } from "./storage";
|
||||||
|
import { useAccountRehydrate } from "./useAccountRehydrate";
|
||||||
import { hydrateSort, type SortOption, type SortState } from "./columnSort";
|
import { hydrateSort, type SortOption, type SortState } from "./columnSort";
|
||||||
|
|
||||||
// The per-account persisted sort state, hydrated + healed against the current options — the plumbing
|
// 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<SortState>(() =>
|
const [sort, setSort] = useState<SortState>(() =>
|
||||||
hydrateSort(getAccountRaw(storageBase), options, fallback),
|
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(
|
const set = useCallback(
|
||||||
(next: SortState) => {
|
(next: SortState) => {
|
||||||
|
markDirty();
|
||||||
setSort(next);
|
setSort(next);
|
||||||
setAccountRaw(storageBase, JSON.stringify(next));
|
setAccountRaw(storageBase, JSON.stringify(next));
|
||||||
},
|
},
|
||||||
[storageBase],
|
[storageBase, markDirty],
|
||||||
);
|
);
|
||||||
return [sort, set];
|
return [sort, set];
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user