fix: rare useEffect cycle with multiple tabs

Author: cardoso

apps/meteor/client/providers/UserProvider/UserProvider.tsx

@@ -1,9 +1,8 @@
 import type { IRoom } from '@rocket.chat/core-typings';
 import { Emitter } from '@rocket.chat/emitter';
-import { useLocalStorage } from '@rocket.chat/fuselage-hooks';
 import { createPredicateFromFilter } from '@rocket.chat/mongo-adapter';
 import type { FindOptions, SubscriptionWithRoom } from '@rocket.chat/ui-contexts';
-import { UserContext, useEndpoint, useRouteParameter, useSearchParameter } from '@rocket.chat/ui-contexts';
+import { UserContext, useRouteParameter, useSearchParameter } from '@rocket.chat/ui-contexts';
 import { useQueryClient } from '@tanstack/react-query';
 import { Accounts } from 'meteor/accounts-base';
 import { Meteor } from 'meteor/meteor';
@@ -17,6 +16,7 @@ import { useDeleteUser } from './hooks/useDeleteUser';
 import { useEmailVerificationWarning } from './hooks/useEmailVerificationWarning';
 import { useReloadAfterLogin } from './hooks/useReloadAfterLogin';
 import { useUpdateAvatar } from './hooks/useUpdateAvatar';
+import { useUserLanguageSync } from './hooks/useUserLanguageSync';
 import { getUserPreference } from '../../../app/utils/client';
 import { sdk } from '../../../app/utils/client/lib/SDKClient';
 import { afterLogoutCleanUpCallback } from '../../../lib/callbacks/afterLogoutCleanUpCallback';
@@ -73,21 +73,18 @@ const UserProvider = ({ children }: UserProviderProps): ReactElement => {
 	});
 
 	const previousUserId = useRef(userId);
-	const [userLanguage, setUserLanguage] = useLocalStorage('userLanguage', '');
-	const [preferedLanguage, setPreferedLanguage] = useLocalStorage('preferedLanguage', '');
 	const [, setSamlInviteToken] = useSamlInviteToken();
 	const samlCredentialToken = useSearchParameter('saml_idp_credentialToken');
 	const inviteTokenHash = useRouteParameter('hash');
 
-	const setUserPreferences = useEndpoint('POST', '/v1/users.setPreferences');
-
 	useEmailVerificationWarning(user ?? undefined);
 	useClearRemovedRoomsHistory(userId);
 
 	useDeleteUser();
 	useUpdateAvatar();
 	useIdleConnection(userId);
 	useReloadAfterLogin(user);
+	useUserLanguageSync(user);
 
 	const querySubscriptions = useMemo(() => {
 		const createSubscriptionFactory =
@@ -156,18 +153,6 @@ const UserProvider = ({ children }: UserProviderProps): ReactElement => {
 		[userId, user, querySubscription, querySubscriptions],
 	);
 
-	useEffect(() => {
-		if (!!userId && preferedLanguage !== userLanguage) {
-			setUserPreferences({ data: { language: preferedLanguage } });
-			setUserLanguage(preferedLanguage);
-		}
-
-		if (user?.language !== undefined && user.language !== userLanguage) {
-			setUserLanguage(user.language);
-			setPreferedLanguage(user.language);
-		}
-	}, [preferedLanguage, setPreferedLanguage, setUserLanguage, user?.language, userLanguage, userId, setUserPreferences]);
-
 	useEffect(() => {
 		if (!samlCredentialToken && !inviteTokenHash) {
 			setSamlInviteToken(null);

apps/meteor/client/providers/UserProvider/hooks/useUserLanguageSync.ts

@@ -0,0 +1,63 @@
+import { useLocalStorage } from '@rocket.chat/fuselage-hooks';
+import { useEndpoint } from '@rocket.chat/ui-contexts';
+import { useEffect, useRef } from 'react';
+
+type UseUserLanguageSyncParams = { language?: string } | null;
+
+export const useUserLanguageSync = (user: UseUserLanguageSyncParams): void => {
+	const setUserPreferences = useEndpoint('POST', '/v1/users.setPreferences');
+	const [userLanguage, setUserLanguage] = useLocalStorage('userLanguage', '');
+	const [preferedLanguage, setPreferedLanguage] = useLocalStorage('preferedLanguage', '');
+	const [isLanguageSyncPending, setLanguageSyncPending] = useLocalStorage('userLanguagePendingSync', false);
+	const pendingLanguageRef = useRef<string | null>(null);
+	const serverLanguage = user?.language;
+
+	useEffect(() => {
+		if (!serverLanguage) {
+			return;
+		}
+
+		const hasPreferedLanguage = typeof preferedLanguage === 'string' && preferedLanguage.length > 0;
+
+		if (isLanguageSyncPending) {
+			if (serverLanguage !== undefined && serverLanguage === preferedLanguage) {
+				setLanguageSyncPending(false);
+				pendingLanguageRef.current = null;
+				return;
+			}
+
+			if (hasPreferedLanguage && serverLanguage !== preferedLanguage) {
+				if (pendingLanguageRef.current !== preferedLanguage) {
+					pendingLanguageRef.current = preferedLanguage;
+					void setUserPreferences({ data: { language: preferedLanguage } }).catch(() => {
+						setLanguageSyncPending(false);
+						pendingLanguageRef.current = null;
+					});
+				}
+				if (userLanguage !== preferedLanguage) {
+					setUserLanguage(preferedLanguage);
+				}
+				return;
+			}
+		} else if (pendingLanguageRef.current) {
+			pendingLanguageRef.current = null;
+		}
+
+		if (serverLanguage !== undefined && userLanguage !== serverLanguage) {
+			setUserLanguage(serverLanguage);
+		}
+
+		if (serverLanguage !== undefined && preferedLanguage !== serverLanguage) {
+			setPreferedLanguage(serverLanguage);
+		}
+	}, [
+		isLanguageSyncPending,
+		preferedLanguage,
+		serverLanguage,
+		setLanguageSyncPending,
+		setPreferedLanguage,
+		setUserLanguage,
+		setUserPreferences,
+		userLanguage,
+	]);
+};

apps/meteor/tests/e2e/user-preferences-loop.spec.ts

@@ -0,0 +1,86 @@
+import type { BrowserContext, Page, Request } from '@playwright/test';
+import type { useEndpoint } from '@rocket.chat/ui-contexts';
+
+import { Users } from './fixtures/userStates';
+import { setUserPreferences } from './utils';
+import { test, expect } from './utils/test';
+
+class RequestTracker {
+	private _method: Parameters<typeof useEndpoint>[0];
+
+	private _pathPattern: Parameters<typeof useEndpoint>[1];
+
+	private _count: number;
+
+	constructor(method: Parameters<typeof useEndpoint>[0], pathPattern: Parameters<typeof useEndpoint>[1]) {
+		this._method = method;
+		this._pathPattern = pathPattern;
+		this._count = 0;
+	}
+
+	public track(page: Page): void {
+		page.on('request', (request) => this.onRequest(request));
+	}
+
+	public get count(): number {
+		return this._count;
+	}
+
+	private onRequest(request: Request): void {
+		const url = new URL(request.url());
+		if (request.method() === this._method && url.pathname.endsWith(this._pathPattern)) {
+			this._count += 1;
+		}
+	}
+}
+
+test.describe('User preferences language propagation', () => {
+	test.use({ storageState: Users.user1.state, locale: 'en' });
+
+	test.beforeAll(async ({ api }) => {
+		await setUserPreferences(api, { language: 'en' });
+	});
+
+	test.afterAll(async ({ api }) => {
+		await setUserPreferences(api, { language: '' });
+	});
+
+	test('should keep calling users.setPreferences once a third tab opens', async ({ context }) => {
+		const requestTracker = new RequestTracker('POST', '/v1/users.setPreferences');
+
+		await test.step('load the first tab', async () => {
+			const page = await navigateToHomePage(context, 'de');
+			requestTracker.track(page);
+			return page;
+		});
+
+		await test.step('load a second tab to ensure the baseline stays stable', async () => {
+			const page = await navigateToHomePage(context, 'pt');
+			requestTracker.track(page);
+			return page;
+		});
+
+		await test.step('opening a third tab should trigger the cascading loop', async () => {
+			const page = await navigateToHomePage(context, 'fr');
+			requestTracker.track(page);
+			return page;
+		});
+
+		await Promise.all(context.pages().map((page) => page.reload()));
+
+		await test.step('assert no further calls were made', async () => {
+			expect(requestTracker.count).toBe(0);
+		});
+	});
+});
+
+async function navigateToHomePage(context: BrowserContext, language: 'en' | 'de' | 'pt' | 'fr'): Promise<Page> {
+	const page = await context.newPage();
+	await page.goto('/home');
+	await page.evaluate((language) => {
+		localStorage.setItem('fuselage-localStorage-userLanguage', `"${language}"`);
+		localStorage.setItem('fuselage-localStorage-preferedLanguage', `"${language}"`);
+	}, language);
+	await expect(page.getByRole('heading', { name: 'Home' })).toBeVisible();
+	return page;
+}