From 715a736bbc8f2d26df4790a6eb7030f9953099d5 Mon Sep 17 00:00:00 2001 From: Daniel Moerner Date: Tue, 4 Aug 2026 14:17:15 -0400 Subject: [PATCH] fix(ui): Stop the sign-in start card flashing after a code is accepted After a verification code was accepted, the sign-in start card could reappear for a moment before the app rendered its signed-in state. `SignInFactorOne` has a guard that sends you back to the start of the flow when the sign-in has no status left, written for "user tried a social button, then came back". #6462 gave that guard an early-return while `setActive` is running, and in doing so made it re-run every time `setActive` starts or stops rather than only on mount. So it runs once more as `setActive` finishes -- and at that point a successful sign-in also has no status left, because completing it consumed the sign-in. The guard can't tell the two cases apart, so it sends a user who just signed in back to the start card. It only showed up sometimes because it needs the card to re-render both during `setActive` and again after it, racing whatever the app does once the session exists. The fix records that `setActive` took over, so a card that has handed off navigation never sends the user back. Behaviour for a genuinely abandoned sign-in is unchanged. `SignInFactorTwo` has the same guard and the same bug, so it gets the same fix. This PR was written by Claude based on a reproduction video that I was able to record. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/olive-donuts-wave.md | 5 + .../src/components/SignIn/SignInFactorOne.tsx | 9 ++ .../src/components/SignIn/SignInFactorTwo.tsx | 9 ++ .../SignInFactorOneSetActiveGuard.test.tsx | 118 ++++++++++++++++++ 4 files changed, 141 insertions(+) create mode 100644 .changeset/olive-donuts-wave.md create mode 100644 packages/ui/src/components/SignIn/__tests__/SignInFactorOneSetActiveGuard.test.tsx diff --git a/.changeset/olive-donuts-wave.md b/.changeset/olive-donuts-wave.md new file mode 100644 index 00000000000..6e7eeb92a73 --- /dev/null +++ b/.changeset/olive-donuts-wave.md @@ -0,0 +1,5 @@ +--- +'@clerk/ui': patch +--- + +Fix the sign-in start card briefly flashing over `` after a verification code is accepted, before the app renders its signed-in state. diff --git a/packages/ui/src/components/SignIn/SignInFactorOne.tsx b/packages/ui/src/components/SignIn/SignInFactorOne.tsx index fbf417e2800..165b3c5bb7a 100644 --- a/packages/ui/src/components/SignIn/SignInFactorOne.tsx +++ b/packages/ui/src/components/SignIn/SignInFactorOne.tsx @@ -123,8 +123,17 @@ function SignInFactorOneInternal(): JSX.Element { const [passwordErrorCode, setPasswordErrorCode] = React.useState(null); + const setActiveTookOverRef = React.useRef(false); + React.useEffect(() => { if (__internal_setActiveInProgress) { + // setActive owns navigation from here on. It consumes the sign-in (status -> null), so the + // check below would fire as setActive winds down and flash the start card over a success. + setActiveTookOverRef.current = true; + return; + } + + if (setActiveTookOverRef.current) { return; } diff --git a/packages/ui/src/components/SignIn/SignInFactorTwo.tsx b/packages/ui/src/components/SignIn/SignInFactorTwo.tsx index 90bfddd6df7..9001f9344f7 100644 --- a/packages/ui/src/components/SignIn/SignInFactorTwo.tsx +++ b/packages/ui/src/components/SignIn/SignInFactorTwo.tsx @@ -29,8 +29,17 @@ function SignInFactorTwoInternal(): JSX.Element { toggleAllStrategies, } = useSecondFactorSelection(signIn.supportedSecondFactors); + const setActiveTookOverRef = React.useRef(false); + React.useEffect(() => { if (clerk.__internal_setActiveInProgress) { + // setActive owns navigation from here on. It consumes the sign-in (status -> null), so the + // check below would fire as setActive winds down and redirect over a flow that succeeded. + setActiveTookOverRef.current = true; + return; + } + + if (setActiveTookOverRef.current) { return; } diff --git a/packages/ui/src/components/SignIn/__tests__/SignInFactorOneSetActiveGuard.test.tsx b/packages/ui/src/components/SignIn/__tests__/SignInFactorOneSetActiveGuard.test.tsx new file mode 100644 index 00000000000..b2559c6189c --- /dev/null +++ b/packages/ui/src/components/SignIn/__tests__/SignInFactorOneSetActiveGuard.test.tsx @@ -0,0 +1,118 @@ +import { ClerkAPIResponseError } from '@clerk/shared/error'; +import type { SignInResource } from '@clerk/shared/types'; +import { waitFor } from '@testing-library/react'; +import { describe, expect, it } from 'vitest'; + +import { bindCreateFixtures } from '@/test/create-fixtures'; +import { render, screen } from '@/test/utils'; + +import { SignInFactorOne } from '../SignInFactorOne'; + +const { createFixtures } = bindCreateFixtures('SignIn'); + +/** + * Mirrors the real `setActive` lifecycle: the flag goes up, the completed sign-in is consumed on + * the client (`status` -> `null`), the card re-renders while the flag is still up (clerk-js emits + * transitive state right before navigating), then the flag drops once navigation is done. + */ +const mockSetActiveLifecycle = (fixtures: any) => { + let release = () => {}; + const gate = new Promise(resolve => (release = resolve)); + + fixtures.clerk.setActive.mockImplementation(async (params: any) => { + fixtures.clerk.__internal_setActiveInProgress = true; + fixtures.signIn.status = null; + await gate; + await params.navigate?.({ session: { currentTask: null }, decorateUrl: (url: string) => url }); + fixtures.clerk.__internal_setActiveInProgress = false; + }); + + return { finishSetActive: () => release() }; +}; + +describe('SignIn setActive guard', () => { + it('does not bounce factor one back to the start card once setActive has completed', 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)); + fixtures.signIn.attemptFirstFactor.mockResolvedValueOnce({ + status: 'complete', + createdSessionId: 'sess_123', + } as any); + const { finishSetActive } = mockSetActiveLifecycle(fixtures); + + const { userEvent, rerender } = render(, { wrapper }); + + await userEvent.type(screen.getByLabelText(/Enter verification code/i), '123456'); + await waitFor(() => expect(fixtures.clerk.setActive).toHaveBeenCalled(), { timeout: 3000 }); + + rerender(); + finishSetActive(); + await waitFor(() => expect((fixtures.clerk as any).__internal_setActiveInProgress).toBe(false)); + + // The host app keeps mounted until its own signed-in state propagates, so the card + // re-renders at least once more after setActive resolves. + rerender(); + + await waitFor(() => expect(fixtures.clerk.setActive).toHaveBeenCalled()); + expect(fixtures.router.navigate).not.toHaveBeenCalledWith('../'); + }); + + it('does not bounce back to the start card after a signUpIfMissing transfer completes', async () => { + const { wrapper, fixtures, props } = await createFixtures(f => { + f.withEmailAddress(); + f.withPreferredSignInStrategy({ strategy: 'otp' }); + f.withEnumerationProtection(); + f.startSignInWithEmailAddress({ supportEmailCode: true, supportPassword: false }); + }); + props.setProps({ withSignUp: true }); + + fixtures.signIn.prepareFirstFactor.mockReturnValueOnce(Promise.resolve({} as SignInResource)); + fixtures.signIn.attemptFirstFactor.mockImplementationOnce(() => { + (fixtures.signIn as any).firstFactorVerification = { status: 'transferable' }; + return Promise.reject( + new ClerkAPIResponseError('Error', { + data: [{ code: 'sign_up_if_missing_transfer', long_message: '', message: '' }], + status: 404, + }), + ); + }); + // A sign-up with no additional requirements transfers straight to `complete`. + fixtures.signUp.create.mockResolvedValueOnce({ status: 'complete', createdSessionId: 'sess_123' } as any); + const { finishSetActive } = mockSetActiveLifecycle(fixtures); + + const { userEvent, rerender } = render(, { wrapper }); + + await userEvent.type(screen.getByLabelText(/Enter verification code/i), '123456'); + await waitFor(() => expect(fixtures.clerk.setActive).toHaveBeenCalled(), { timeout: 3000 }); + + rerender(); + finishSetActive(); + await waitFor(() => expect((fixtures.clerk as any).__internal_setActiveInProgress).toBe(false)); + + // The terminal redirect leaves the page, but the document stays alive while the browser + // fetches the next one, so the card can still re-render and bounce. + rerender(); + + expect(fixtures.router.navigate).not.toHaveBeenCalledWith('../'); + }); + + it('still bounces to the start card when the sign-in was abandoned without setActive', 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)); + (fixtures.signIn as any).status = 'needs_identifier'; + + render(, { wrapper }); + + await waitFor(() => expect(fixtures.router.navigate).toHaveBeenCalledWith('../')); + }); +});