Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/olive-donuts-wave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@clerk/ui': patch
---

Fix the sign-in start card briefly flashing over `<SignIn />` after a verification code is accepted, before the app renders its signed-in state.
9 changes: 9 additions & 0 deletions packages/ui/src/components/SignIn/SignInFactorOne.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,17 @@ function SignInFactorOneInternal(): JSX.Element {

const [passwordErrorCode, setPasswordErrorCode] = React.useState<PasswordErrorCode | null>(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;
}

Expand Down
9 changes: 9 additions & 0 deletions packages/ui/src/components/SignIn/SignInFactorTwo.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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<void>(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(<SignInFactorOne />, { wrapper });

await userEvent.type(screen.getByLabelText(/Enter verification code/i), '123456');
await waitFor(() => expect(fixtures.clerk.setActive).toHaveBeenCalled(), { timeout: 3000 });

rerender(<SignInFactorOne />);
finishSetActive();
await waitFor(() => expect((fixtures.clerk as any).__internal_setActiveInProgress).toBe(false));

// The host app keeps <SignIn> mounted until its own signed-in state propagates, so the card
// re-renders at least once more after setActive resolves.
rerender(<SignInFactorOne />);

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(<SignInFactorOne />, { wrapper });

await userEvent.type(screen.getByLabelText(/Enter verification code/i), '123456');
await waitFor(() => expect(fixtures.clerk.setActive).toHaveBeenCalled(), { timeout: 3000 });

rerender(<SignInFactorOne />);
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(<SignInFactorOne />);

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(<SignInFactorOne />, { wrapper });

await waitFor(() => expect(fixtures.router.navigate).toHaveBeenCalledWith('../'));
});
});
Loading