fix(app): hold the switched view until its route commits (#3293)

fluxer_app/src/features/auth/state/Accounts.test.ts

@@ -36,7 +36,12 @@ const mocks = vi.hoisted(() => ({
 	retireForAccountTransition: vi.fn<(reason: string) => Promise<void>>(() => Promise.resolve()),
 	gatewayLogout: vi.fn<() => void>(),
 	sendInvisiblePresence: vi.fn<() => void>(),
-	replaceWith: vi.fn<(path: string) => void>(),
+	replaceWith: vi.fn<(path: string) => void>((path) => {
+		mocks.currentPath = path;
+		mocks.commitRouterPath(path);
+	}),
+	readRouterPath: (): string => '',
+	commitRouterPath: (_path: string): void => undefined,
 	abandonUnreachableLastLocation: vi.fn<() => void>(),
 	currentPath: '',
 	lastLocation: null as string | null,
@@ -80,6 +85,20 @@ vi.mock('@app/features/navigation/utils/RouterUtils', () => ({
 	history: null,
 }));
 
+vi.mock('@app/features/navigation/state/Navigation', async () => {
+	const {observable, runInAction} = await import('mobx');
+	const routerPath = observable.box('');
+	mocks.readRouterPath = () => routerPath.get();
+	mocks.commitRouterPath = (path) => runInAction(() => routerPath.set(path));
+	return {
+		default: {
+			get pathname() {
+				return mocks.readRouterPath();
+			},
+		},
+	};
+});
+
 vi.mock('@app/features/ui/state/Location', () => ({
 	default: {getLastLocation: () => mocks.lastLocation, saveLocation: mocks.saveLocation},
 }));
@@ -203,9 +222,11 @@ function gatewayOutage(accountKey: string): InstanceType<typeof ForegroundGatewa
 
 beforeEach(async () => {
 	mocks.currentPath = '';
+	mocks.commitRouterPath('');
 	mocks.lastLocation = null;
 	mocks.replaceWith.mockImplementation((path) => {
 		mocks.currentPath = path;
+		mocks.commitRouterPath(path);
 	});
 	mocks.foregroundAccountKey = null;
 	mocks.hydrateForegroundStores(null);
@@ -490,9 +511,11 @@ describe('Accounts view handover between accounts that share a guild', () => {
 		mocks.currentPath = GENERAL_ROUTE;
 		mocks.lastLocation = GENERAL_ROUTE;
 		runInAction(() => routedPath.set(GENERAL_ROUTE));
+		mocks.commitRouterPath(GENERAL_ROUTE);
 		mocks.replaceWith.mockImplementation((path) => {
 			mocks.currentPath = path;
 			runInAction(() => routedPath.set(path));
+			mocks.commitRouterPath(path);
 			redirectUnknownSharedGuildChannel(path);
 		});
 		mocks.saveLocation.mockImplementation((path) => {
@@ -581,6 +604,36 @@ describe('Accounts view handover between accounts that share a guild', () => {
 		expect(rendered.filter((entry) => !entry.live).every((entry) => entry.path === GENERAL_ROUTE)).toBe(true);
 	});
 
+	test('the target view stays held until the router commits the target route', async () => {
+		mocks.replaceWith.mockImplementation((path) => {
+			mocks.currentPath = path;
+			setTimeout(() => {
+				runInAction(() => routedPath.set(path));
+				mocks.commitRouterPath(path);
+			}, 0);
+		});
+		const rendered: Array<{accountKey: string | null; path: string; live: boolean}> = [];
+		const ViewRecorder = observer(() => {
+			rendered.push({accountKey: SessionManager.currentAccountKey, path: routedPath.get(), live: Accounts.isViewLive});
+			return null;
+		});
+		const container = document.createElement('div');
+		const root = createRoot(container);
+		(globalThis as {IS_REACT_ACT_ENVIRONMENT?: boolean}).IS_REACT_ACT_ENVIRONMENT = true;
+		act(() => root.render(createElement(ViewRecorder)));
+		disposers.push(() => act(() => root.unmount()));
+
+		await act(async () => {
+			await Accounts.switchToAccount(accountB);
+			await new Promise((resolve) => setTimeout(resolve, 10));
+		});
+
+		expect(
+			rendered.filter((entry) => entry.live && entry.accountKey === accountB && entry.path !== PRIVATE_ROUTE),
+		).toEqual([]);
+		expect(rendered.at(-1)).toEqual({accountKey: accountB, path: PRIVATE_ROUTE, live: true});
+	});
+
 	test('the target route is in place in the same tick as its state, before the gateway handshake settles', async () => {
 		await Accounts.switchToAccount(accountB);
 

fluxer_app/src/features/auth/state/Accounts.ts

@@ -17,6 +17,7 @@ import {getAccountKey, parseAccountStorageKey} from '@app/features/auth/state/Ac
 import ExperimentAssignments from '@app/features/experiment/state/ExperimentAssignments';
 import {ForegroundGatewayConnectionRecoverableError} from '@app/features/gateway/transport/ForegroundGatewayConnectionFailure';
 import GatewayConnection from '@app/features/gateway/transport/GatewayConnection';
+import Navigation from '@app/features/navigation/state/Navigation';
 import {abandonUnreachableLastLocation} from '@app/features/navigation/utils/ChannelRouteReachability';
 import * as RouterUtils from '@app/features/navigation/utils/RouterUtils';
 import * as NotificationUtils from '@app/features/notification/utils/NotificationUtils';
@@ -103,7 +104,10 @@ interface AccountReplacementCommitRequest {
 
 type AccountViewReplacement =
 	| {readonly kind: 'activation'; readonly redirectPath: string | null}
-	| {readonly kind: 'rollback'; readonly previousRoute: string};
+	| {readonly kind: 'rollback'; readonly previousRoute: string}
+	| {readonly kind: 'navigating'; readonly stalePath: string};
+
+const VIEW_NAVIGATION_COMMIT_TIMEOUT_MS = 2000;
 
 type AccountActivationOutcome =
 	| {readonly kind: 'ready'}
@@ -478,9 +482,11 @@ class Accounts {
 			throw activationError;
 		} finally {
 			stopAwaitingLiveView();
+			if (this.viewReplacement?.kind !== 'navigating') {
 				this.viewReplacement = null;
 			}
 		}
+	}
 
 	private beginViewReplacement(request: AccountReplacementCommitRequest, previous: PreviousAccountState): () => void {
 		this.viewReplacement = {kind: 'activation', redirectPath: request.redirectPath};
@@ -612,17 +618,44 @@ class Accounts {
 
 	private revealReplacementView(): void {
 		const replacement = this.viewReplacement;
-		if (replacement === null) {
+		if (replacement === null || replacement.kind === 'navigating') {
 			return;
 		}
 		const path = this.resolveReplacementViewPath(replacement);
-		if (path !== null && path !== RouterUtils.getCurrentPath()) {
-			RouterUtils.replaceWith(path);
-		}
+		const stalePath = Navigation.pathname;
+		if (path === null || path === RouterUtils.getCurrentPath()) {
 			this.viewReplacement = null;
+			return;
+		}
+		RouterUtils.replaceWith(path);
+		this.holdViewUntilNavigationCommits(stalePath);
 	}
 
-	private resolveReplacementViewPath(replacement: AccountViewReplacement): string | null {
+	private holdViewUntilNavigationCommits(stalePath: string): void {
+		if (Navigation.pathname !== stalePath) {
+			this.viewReplacement = null;
+			return;
+		}
+		const marker: AccountViewReplacement = {kind: 'navigating', stalePath};
+		this.viewReplacement = marker;
+		const release = () => {
+			if (this.viewReplacement === marker) {
+				this.viewReplacement = null;
+			}
+		};
+		const timeout = setTimeout(release, VIEW_NAVIGATION_COMMIT_TIMEOUT_MS);
+		when(
+			() => this.viewReplacement !== marker || Navigation.pathname !== stalePath,
+			() => {
+				clearTimeout(timeout);
+				release();
+			},
+		);
+	}
+
+	private resolveReplacementViewPath(
+		replacement: Exclude<AccountViewReplacement, {readonly kind: 'navigating'}>,
+	): string | null {
 		if (replacement.kind === 'rollback') {
 			return replacement.previousRoute === '' ? null : replacement.previousRoute;
 		}