From 0f701a976b56bb0937b657b78190420aab909b0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Tue, 18 Aug 2026 15:36:27 +0200 Subject: [PATCH 1/3] Fix signIn briefly flashing the start before successful navigation --- .changeset/quiet-clouds-flash.md | 5 ++ .../src/components/SignIn/SignInFactorOne.tsx | 20 ++--- .../src/components/SignIn/SignInFactorTwo.tsx | 33 ++++--- .../SignIn/__tests__/SignInFactorOne.test.tsx | 60 +++++++++++++ .../SignIn/__tests__/SignInFactorTwo.test.tsx | 73 +++++++++++++++- .../components/SignIn/useSignInStepGuard.ts | 86 +++++++++++++++++++ 6 files changed, 243 insertions(+), 34 deletions(-) create mode 100644 .changeset/quiet-clouds-flash.md create mode 100644 packages/ui/src/components/SignIn/useSignInStepGuard.ts diff --git a/.changeset/quiet-clouds-flash.md b/.changeset/quiet-clouds-flash.md new file mode 100644 index 00000000000..1e00daa4b3e --- /dev/null +++ b/.changeset/quiet-clouds-flash.md @@ -0,0 +1,5 @@ +--- +'@clerk/ui': patch +--- + +Fix rare condition where the start of the sign-in would briefly flash before the app rendered its signed-in state. diff --git a/packages/ui/src/components/SignIn/SignInFactorOne.tsx b/packages/ui/src/components/SignIn/SignInFactorOne.tsx index fbf417e2800..ddb7b11b904 100644 --- a/packages/ui/src/components/SignIn/SignInFactorOne.tsx +++ b/packages/ui/src/components/SignIn/SignInFactorOne.tsx @@ -1,4 +1,3 @@ -import { useClerk } from '@clerk/shared/react'; import type { SignInFactor } from '@clerk/shared/types'; import React from 'react'; @@ -24,6 +23,7 @@ import type { PasswordErrorCode } from './SignInFactorOnePasswordCard'; import { SignInFactorOnePasswordCard } from './SignInFactorOnePasswordCard'; import { SignInFactorOnePhoneCodeCard } from './SignInFactorOnePhoneCodeCard'; import { useResetPasswordFactor } from './useResetPasswordFactor'; +import { useSignInStepGuard } from './useSignInStepGuard'; import { determineStartingSignInFactor, factorHasLocalStrategy } from './utils'; const factorKey = (factor: SignInFactor | null | undefined) => { @@ -75,7 +75,6 @@ function removeSignInResetPasswordIntentParam(): void { } function SignInFactorOneInternal(): JSX.Element { - const { __internal_setActiveInProgress } = useClerk(); const signIn = useCoreSignIn(); const { preferredSignInStrategy } = useEnvironment().displayConfig; const availableFactors = signIn.supportedFirstFactors; @@ -123,18 +122,11 @@ function SignInFactorOneInternal(): JSX.Element { const [passwordErrorCode, setPasswordErrorCode] = React.useState(null); - React.useEffect(() => { - if (__internal_setActiveInProgress) { - return; - } - - // Handle the case where a user lands on alternative methods screen, - // clicks a social button but then navigates back to sign in. - // SignIn status resets to 'needs_identifier' - if (signIn.status === 'needs_identifier' || signIn.status === null) { - void router.navigate('../'); - } - }, [__internal_setActiveInProgress]); + // A social flow returning to the component resets the sign-in to `needs_identifier`. + useSignInStepGuard({ + redirectStatuses: ['needs_identifier'], + onLeave: () => void router.navigate('../'), + }); if (!currentFactor) { return signIn.status ? ( diff --git a/packages/ui/src/components/SignIn/SignInFactorTwo.tsx b/packages/ui/src/components/SignIn/SignInFactorTwo.tsx index 1af218d771c..b26e0c7f972 100644 --- a/packages/ui/src/components/SignIn/SignInFactorTwo.tsx +++ b/packages/ui/src/components/SignIn/SignInFactorTwo.tsx @@ -1,5 +1,4 @@ import { useClerk } from '@clerk/shared/react'; -import React from 'react'; import { withCardStateProvider } from '@/ui/elements/contexts'; import { LoadingCard } from '@/ui/elements/LoadingCard'; @@ -14,6 +13,7 @@ import { SignInFactorTwoEmailLinkCard } from './SignInFactorTwoEmailLinkCard'; import { SignInFactorTwoPhoneCodeCard } from './SignInFactorTwoPhoneCodeCard'; import { SignInFactorTwoTOTPCard } from './SignInFactorTwoTOTPCard'; import { useSecondFactorSelection } from './useSecondFactorSelection'; +import { useSignInStepGuard } from './useSignInStepGuard'; function SignInFactorTwoInternal(): JSX.Element { const clerk = useClerk(); @@ -31,25 +31,22 @@ function SignInFactorTwoInternal(): JSX.Element { const onShowAlternativeMethodsClicked = signIn.supportedSecondFactors && signIn.supportedSecondFactors.length > 1 ? toggleAllStrategies : undefined; - React.useEffect(() => { - if (clerk.__internal_setActiveInProgress) { - return; + const leaveFactorTwo = () => { + // If the user is already signed in (e.g. multi-session app, page reload after + // successful verification), redirect forward to afterSignInUrl instead of + // back to sign-in start. + if (clerk.isSignedIn) { + void router.navigate(afterSignInUrl); + } else { + void router.navigate('../'); } + }; - // If the sign-in doesn't need second factor verification, redirect away. - // Don't redirect for 'complete' status - setActive will handle navigation. - if (signIn.status === null || signIn.status === 'needs_identifier' || signIn.status === 'needs_first_factor') { - // If the user is already signed in (e.g. multi-session app, page reload after - // successful verification), redirect forward to afterSignInUrl instead of - // back to sign-in start. - if (clerk.isSignedIn) { - void router.navigate(afterSignInUrl); - } else { - void router.navigate('../'); - } - } - // eslint-disable-next-line react-hooks/exhaustive-deps -- Only run on mount and when setActiveInProgress changes - }, [clerk.__internal_setActiveInProgress]); + // Leave if the sign-in no longer needs second-factor verification. + useSignInStepGuard({ + redirectStatuses: ['needs_identifier', 'needs_first_factor'], + onLeave: leaveFactorTwo, + }); if (!currentFactor) { return ; diff --git a/packages/ui/src/components/SignIn/__tests__/SignInFactorOne.test.tsx b/packages/ui/src/components/SignIn/__tests__/SignInFactorOne.test.tsx index 57ba4fb7448..4e14b780d50 100644 --- a/packages/ui/src/components/SignIn/__tests__/SignInFactorOne.test.tsx +++ b/packages/ui/src/components/SignIn/__tests__/SignInFactorOne.test.tsx @@ -70,6 +70,66 @@ describe('SignInFactorOne', () => { render(, { wrapper }); expect(fixtures.router.navigate).toHaveBeenCalledWith('../'); }); + + it('does not navigate to the start card when it mounts while setActive is in progress', async () => { + const { wrapper, fixtures } = await createFixtures(); + fixtures.clerk.__internal_setActiveInProgress = true; + render(, { wrapper }); + expect(fixtures.router.navigate).not.toHaveBeenCalledWith('../'); + }); + + it('navigates to the start card when a setActive running on mount finishes without restoring a sign-in', async () => { + const { wrapper, fixtures } = await createFixtures(); + fixtures.clerk.__internal_setActiveInProgress = true; + render(, { wrapper }); + + expect(fixtures.router.navigate).not.toHaveBeenCalledWith('../'); + + fixtures.clerk.__internal_setActiveInProgress = false; + + await waitFor(() => expect(fixtures.router.navigate).toHaveBeenCalledWith('../'), { timeout: 500 }); + }); + + it('does not navigate to the start card when a setActive completes while it is mounted', async () => { + const { wrapper, fixtures } = await createFixtures(f => { + f.withEmailAddress(); + f.withPreferredSignInStrategy({ strategy: 'otp' }); + f.startSignInWithEmailAddress({ supportEmailCode: true, supportPassword: false }); + }); + fixtures.signIn.prepareFirstFactor.mockReturnValueOnce(Promise.resolve({} as SignInResource)); + + const { rerender } = render(, { wrapper }); + + fixtures.clerk.__internal_setActiveInProgress = true; + fixtures.signIn.status = null; + rerender(); + + fixtures.clerk.__internal_setActiveInProgress = false; + rerender(); + + expect(fixtures.router.navigate).not.toHaveBeenCalledWith('../'); + }); + + it('navigates to the start card when setActive completes and the sign-in was abandoned', async () => { + const { wrapper, fixtures } = await createFixtures(f => { + f.withMultiSessionMode(); + f.withEmailAddress(); + f.withPreferredSignInStrategy({ strategy: 'otp' }); + f.startSignInWithEmailAddress({ supportEmailCode: true, supportPassword: false }); + }); + fixtures.signIn.prepareFirstFactor.mockReturnValueOnce(Promise.resolve({} as SignInResource)); + + const { rerender } = render(, { wrapper }); + + fixtures.clerk.__internal_setActiveInProgress = true; + rerender(); + + fixtures.clerk.__internal_setActiveInProgress = false; + fixtures.signIn.status = 'needs_identifier'; + rerender(); + + expect(fixtures.router.navigate).toHaveBeenCalledWith('../'); + }); }); describe('Submitting', () => { diff --git a/packages/ui/src/components/SignIn/__tests__/SignInFactorTwo.test.tsx b/packages/ui/src/components/SignIn/__tests__/SignInFactorTwo.test.tsx index 287e934288c..90e4fa758ac 100644 --- a/packages/ui/src/components/SignIn/__tests__/SignInFactorTwo.test.tsx +++ b/packages/ui/src/components/SignIn/__tests__/SignInFactorTwo.test.tsx @@ -18,8 +18,77 @@ describe('SignInFactorTwo', () => { }); describe('Navigation', () => { - //This isn't yet implemented in the component - it.todo('navigates to SignInStart component if user lands on SignInFactorTwo page but they should not'); + it('navigates to SignInStart if the user lands on SignInFactorTwo without a sign-in', async () => { + const { wrapper, fixtures } = await createFixtures(); + render(, { wrapper }); + expect(fixtures.router.navigate).toHaveBeenCalledWith('../'); + }); + + it('does not navigate to the start card when it mounts while setActive is in progress', async () => { + const { wrapper, fixtures } = await createFixtures(); + fixtures.clerk.__internal_setActiveInProgress = true; + render(, { wrapper }); + expect(fixtures.router.navigate).not.toHaveBeenCalledWith('../'); + }); + + it('navigates to the start card when a setActive running on mount finishes without restoring a sign-in', async () => { + const { wrapper, fixtures } = await createFixtures(); + fixtures.clerk.__internal_setActiveInProgress = true; + render(, { wrapper }); + + expect(fixtures.router.navigate).not.toHaveBeenCalledWith('../'); + + fixtures.clerk.__internal_setActiveInProgress = false; + + await waitFor(() => expect(fixtures.router.navigate).toHaveBeenCalledWith('../'), { timeout: 500 }); + }); + + it('does not navigate to the start card when a setActive completes while it is mounted', async () => { + const { wrapper, fixtures } = await createFixtures(f => { + f.startSignInFactorTwo(); + }); + fixtures.signIn.prepareSecondFactor.mockReturnValueOnce(Promise.resolve({} as SignInResource)); + + const { rerender } = render(, { wrapper }); + + fixtures.clerk.__internal_setActiveInProgress = true; + fixtures.signIn.status = null; + rerender(); + + fixtures.clerk.__internal_setActiveInProgress = false; + rerender(); + + expect(fixtures.router.navigate).not.toHaveBeenCalledWith('../'); + }); + + it('navigates to the start card when setActive completes and the sign-in returned to factor one', async () => { + const { wrapper, fixtures } = await createFixtures(f => { + f.withMultiSessionMode(); + f.startSignInFactorTwo(); + }); + fixtures.signIn.prepareSecondFactor.mockReturnValueOnce(Promise.resolve({} as SignInResource)); + + const { rerender } = render(, { wrapper }); + + fixtures.clerk.__internal_setActiveInProgress = true; + rerender(); + + fixtures.clerk.__internal_setActiveInProgress = false; + fixtures.signIn.status = 'needs_first_factor'; + rerender(); + + expect(fixtures.router.navigate).toHaveBeenCalledWith('../'); + }); + + it('navigates to afterSignInUrl when already signed in with a reset sign-in', async () => { + const { wrapper, fixtures, props } = await createFixtures(f => { + f.withMultiSessionMode(); + f.withUser({ email_addresses: ['test@clerk.com'] }); + }); + props.setProps({ forceRedirectUrl: 'https://example.com/after-sign-in' }); + render(, { wrapper }); + expect(fixtures.router.navigate).toHaveBeenCalledWith('https://example.com/after-sign-in'); + }); }); describe('Submitting', () => { diff --git a/packages/ui/src/components/SignIn/useSignInStepGuard.ts b/packages/ui/src/components/SignIn/useSignInStepGuard.ts new file mode 100644 index 00000000000..7f7cb0d385a --- /dev/null +++ b/packages/ui/src/components/SignIn/useSignInStepGuard.ts @@ -0,0 +1,86 @@ +import { useClerk, useSafeLayoutEffect } from '@clerk/shared/react'; +import type { SignInResource } from '@clerk/shared/types'; +import React from 'react'; + +import { useCoreSignIn } from '../../contexts'; + +const SET_ACTIVE_POLL_INTERVAL_MS = 50; + +type UseSignInStepGuardParams = { + redirectStatuses: SignInResource['status'][]; + onLeave: () => void; +}; + +/** + * This hook calls onLeave when signIn.status is either: + * - null on mount + * - One of redirectStatuses + * + * Additionally, if the status is null on mount, but __internal_setActiveInProgress + * is true, indicating a setActive is running, it does not call onLeave immediately, + * instead it starts a timer to poll the status of setActive (since it's not reactive). + * + * If setActive was called from this signIn process, it's going to result in a navigate + * which unmounts this component and cancels the timer. If it was an unrelated setActive, + * or the setActive fails, which does not lead to this component unmounting, and the + * status is still null after the setActive finishes, this hook calls onLeave. + * + * We need this complexity only because the logic is brittle to begin with, a better fix + * probably lies in rethinking setActive, but that's for another day. + */ +export function useSignInStepGuard({ redirectStatuses, onLeave }: UseSignInStepGuardParams): void { + const clerk = useClerk(); + const signIn = useCoreSignIn(); + const status = signIn.status; + const onLeaveRef = React.useRef(onLeave); + const redirectStatusesRef = React.useRef(redirectStatuses); + + // We don't want the effects to be reactive to these values, think of this as + // a hacky useEffectEvent. The reason we use a layout effect over mutating in + // render is for better concurrency support + useSafeLayoutEffect(() => { + onLeaveRef.current = onLeave; + redirectStatusesRef.current = redirectStatuses; + }); + + React.useEffect(function leaveOnNoSignInStatus() { + if (status !== null) { + return; + } + + // If the component remounts when setActive is in progress, we don't want + // to leave since that might bounce to the first page briefly before setActive + // navigates us correctly to post-signin, causing a quick flash of content. + if (!clerk.__internal_setActiveInProgress) { + onLeaveRef.current(); + return; + } + + // The flag is not reactive, so wait for the in-flight setActive to settle. + const intervalId = setInterval(() => { + if (clerk.__internal_setActiveInProgress) { + return; + } + + clearInterval(intervalId); + // Read directly from clerk to avoid reading stale state + if (clerk.client.signIn.status === null) { + onLeaveRef.current(); + } + }, SET_ACTIVE_POLL_INTERVAL_MS); + + return () => clearInterval(intervalId); + // `null` is also the post-setActive consumed state, so only handle it on mount. + // This is the reason we have two separate hooks. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); + + React.useEffect( + function leaveOnRedirectStatus() { + if (redirectStatusesRef.current.includes(status)) { + onLeaveRef.current(); + } + }, + [status], + ); +} From f0f6f70a1429ae33f0c0b508ba7c7c6955fdff90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Tue, 18 Aug 2026 16:05:58 +0200 Subject: [PATCH 2/3] Clarify comment --- packages/ui/src/components/SignIn/useSignInStepGuard.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/ui/src/components/SignIn/useSignInStepGuard.ts b/packages/ui/src/components/SignIn/useSignInStepGuard.ts index 7f7cb0d385a..a47197dd652 100644 --- a/packages/ui/src/components/SignIn/useSignInStepGuard.ts +++ b/packages/ui/src/components/SignIn/useSignInStepGuard.ts @@ -56,7 +56,10 @@ export function useSignInStepGuard({ redirectStatuses, onLeave }: UseSignInStepG return; } - // The flag is not reactive, so wait for the in-flight setActive to settle. + // If the setActive that was running on mount never finishes in a way that + // closes the signIn, for example when it's unrelated to the signIn process, + // or fails, the component could get stuck in a loading state forever. + // This timer is there to prevent that. const intervalId = setInterval(() => { if (clerk.__internal_setActiveInProgress) { return; From 496d499ceb7e8c6d0a4904af413b116220a1e012 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20H=C3=B6glund?= Date: Tue, 18 Aug 2026 17:18:02 +0200 Subject: [PATCH 3/3] Fix feedback --- .../SignIn/__tests__/SignInFactorOne.test.tsx | 21 +++++++------------ .../SignIn/__tests__/SignInFactorTwo.test.tsx | 21 ++++++++----------- 2 files changed, 17 insertions(+), 25 deletions(-) diff --git a/packages/ui/src/components/SignIn/__tests__/SignInFactorOne.test.tsx b/packages/ui/src/components/SignIn/__tests__/SignInFactorOne.test.tsx index 4e14b780d50..87100baf5ec 100644 --- a/packages/ui/src/components/SignIn/__tests__/SignInFactorOne.test.tsx +++ b/packages/ui/src/components/SignIn/__tests__/SignInFactorOne.test.tsx @@ -90,22 +90,17 @@ describe('SignInFactorOne', () => { await waitFor(() => expect(fixtures.router.navigate).toHaveBeenCalledWith('../'), { timeout: 500 }); }); - it('does not navigate to the start card when a setActive completes while it is mounted', async () => { - const { wrapper, fixtures } = await createFixtures(f => { - f.withEmailAddress(); - f.withPreferredSignInStrategy({ strategy: 'otp' }); - f.startSignInWithEmailAddress({ supportEmailCode: true, supportPassword: false }); - }); - fixtures.signIn.prepareFirstFactor.mockReturnValueOnce(Promise.resolve({} as SignInResource)); - - const { rerender } = render(, { wrapper }); - + it('does not navigate to the start card when a setActive running on mount restores a sign-in', async () => { + const { wrapper, fixtures } = await createFixtures(); fixtures.clerk.__internal_setActiveInProgress = true; - fixtures.signIn.status = null; - rerender(); + render(, { wrapper }); + fixtures.signIn.status = 'needs_first_factor'; fixtures.clerk.__internal_setActiveInProgress = false; - rerender(); + + await act(async () => { + await new Promise(resolve => setTimeout(resolve, 100)); + }); expect(fixtures.router.navigate).not.toHaveBeenCalledWith('../'); }); diff --git a/packages/ui/src/components/SignIn/__tests__/SignInFactorTwo.test.tsx b/packages/ui/src/components/SignIn/__tests__/SignInFactorTwo.test.tsx index 90e4fa758ac..3c9112ba100 100644 --- a/packages/ui/src/components/SignIn/__tests__/SignInFactorTwo.test.tsx +++ b/packages/ui/src/components/SignIn/__tests__/SignInFactorTwo.test.tsx @@ -3,7 +3,7 @@ import type { SignInResource } from '@clerk/shared/types'; import { describe, expect, it, vi } from 'vitest'; import { bindCreateFixtures } from '@/test/create-fixtures'; -import { render, screen, waitFor } from '@/test/utils'; +import { act, render, screen, waitFor } from '@/test/utils'; import { SignInFactorTwo } from '../SignInFactorTwo'; @@ -43,20 +43,17 @@ describe('SignInFactorTwo', () => { await waitFor(() => expect(fixtures.router.navigate).toHaveBeenCalledWith('../'), { timeout: 500 }); }); - it('does not navigate to the start card when a setActive completes while it is mounted', async () => { - const { wrapper, fixtures } = await createFixtures(f => { - f.startSignInFactorTwo(); - }); - fixtures.signIn.prepareSecondFactor.mockReturnValueOnce(Promise.resolve({} as SignInResource)); - - const { rerender } = render(, { wrapper }); - + it('does not navigate to the start card when a setActive running on mount restores a sign-in', async () => { + const { wrapper, fixtures } = await createFixtures(); fixtures.clerk.__internal_setActiveInProgress = true; - fixtures.signIn.status = null; - rerender(); + render(, { wrapper }); + fixtures.signIn.status = 'needs_second_factor'; fixtures.clerk.__internal_setActiveInProgress = false; - rerender(); + + await act(async () => { + await new Promise(resolve => setTimeout(resolve, 100)); + }); expect(fixtures.router.navigate).not.toHaveBeenCalledWith('../'); });