diff --git a/frontend/src/pages/Login.tsx b/frontend/src/pages/Login.tsx index 9bfae649f2..701eb4dc0b 100644 --- a/frontend/src/pages/Login.tsx +++ b/frontend/src/pages/Login.tsx @@ -16,7 +16,12 @@ export const LoginError = new Error( export default function Login() { const query = useQuery(); const { data: config, user, setUserFromAPI } = useAuth(); - const [error, setError] = useState(''); + // Read the error param synchronously on mount instead of via effect, so the + // first render already carries it. Otherwise `error` starts empty, flows + // into `LoginForm`'s own `useState(error)` initial value, and only catches + // up once both components' effects have flushed -- a two-hop async chain + // that a slow test runner can lose a race against (chainlit#3023). + const [error, setError] = useState(() => query.get('error') || ''); const apiClient = useContext(ChainlitContext); const navigate = useNavigate(); const { variant } = useTheme(); diff --git a/frontend/tests/Login.spec.tsx b/frontend/tests/Login.spec.tsx new file mode 100644 index 0000000000..b21b891530 --- /dev/null +++ b/frontend/tests/Login.spec.tsx @@ -0,0 +1,103 @@ +import { i18nSetupLocalization } from '@/i18n'; +import { flushSync } from 'react-dom'; +import { createRoot } from 'react-dom/client'; +import { MemoryRouter } from 'react-router-dom'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import Login from '@/pages/Login'; + +// Real i18next init (no backend plugin, so it resolves synchronously) -- +// LoginForm's calls i18n.exists() at render time and throws +// if no instance was ever initialized. +i18nSetupLocalization(); + +vi.mock('client-types/*', async () => { + const { createContext } = await import('react'); + const mockApiClient = { + buildEndpoint: (path: string) => `http://localhost:8000${path}`, + getOAuthEndpoint: (provider: string) => + `http://localhost:8000/auth/oauth/${provider}` + }; + return { + ChainlitContext: createContext(mockApiClient), + useAuth: () => ({ + data: { requireLogin: true, oauthProviders: [] }, + user: null, + setUserFromAPI: vi.fn() + }) + }; +}); + +// Logo pulls useConfig() from @chainlit/react-client, which needs a +// RecoilRoot ancestor. It is unrelated to the error-render race under test, +// so stub it out rather than wiring up Recoil for this test. +vi.mock('@/components/Logo', () => ({ + Logo: () => null +})); + +// jsdom does not implement matchMedia, and Login -> useTheme() calls it +// synchronously during render (not from an effect), so stub it the same way +// FavoriteButton.spec.tsx stubs ResizeObserver. +window.matchMedia = + window.matchMedia || + vi.fn().mockImplementation((query: string) => ({ + matches: false, + media: query, + addEventListener: vi.fn(), + removeEventListener: vi.fn(), + dispatchEvent: vi.fn() + })); + +describe('Login', () => { + let container: HTMLDivElement | null = null; + let root: ReturnType | null = null; + + afterEach(() => { + if (root) { + root.unmount(); + root = null; + } + if (container) { + document.body.removeChild(container); + container = null; + } + }); + + // Regression test for the render race fixed by reading the ?error= query + // param via a lazy useState initializer instead of an effect. Uses + // createRoot + flushSync directly instead of Testing Library's render(), + // because render() wraps in act(), which flushes passive effects + // synchronously and hides the gap this test is pinning: what is in the DOM + // on the very first commit, before any useEffect has run. + it('renders the [role="alert"] error message on the first synchronous commit', () => { + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + + flushSync(() => { + root.render( + + + + ); + }); + + expect(container.querySelector('[role="alert"]')).not.toBeNull(); + }); + + it('renders no [role="alert"] on the first synchronous commit when there is no error param', () => { + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + + flushSync(() => { + root.render( + + + + ); + }); + + expect(container.querySelector('[role="alert"]')).toBeNull(); + }); +});