fix(clerk-js): Do not treat phone_number as optional if used as identifier

.changeset/little-wings-bathe.md

@@ -0,0 +1,5 @@
+---
+'@clerk/clerk-js': patch
+---
+
+Fix issue where the combined flow wouldn't trigger if a phone number was used as an identifier while set as an optional field.

packages/clerk-js/src/ui/components/SignIn/__tests__/handleCombinedFlowTransfer.test.ts

@@ -0,0 +1,177 @@
+import type { LoadedClerk, SignUpResource } from '@clerk/types';
+
+import { handleCombinedFlowTransfer, hasOptionalFields } from '../handleCombinedFlowTransfer';
+
+// eslint-disable-next-line no-var -- Jest hoists mock calls to the top of the file, so var is needed.
+var mockCompleteSignUpFlow: jest.Mock;
+jest.mock('../lazy-sign-up', () => {
+  mockCompleteSignUpFlow = jest.fn();
+  return {
+    lazyCompleteSignUpFlow: () => {
+      return Promise.resolve(mockCompleteSignUpFlow);
+    },
+  };
+});
+
+const mockNavigate = jest.fn();
+const mockHandleError = jest.fn();
+
+describe('handleCombinedFlowTransfer', () => {
+  beforeEach(() => {
+    jest.resetAllMocks();
+  });
+
+  it('should call completeSignUpFlow', async () => {
+    const mockClerk = {
+      client: {
+        signUp: {
+          create: jest.fn().mockResolvedValue({}),
+          optionalFields: [],
+        },
+      },
+    };
+
+    await handleCombinedFlowTransfer({
+      identifierAttribute: 'emailAddress',
+      identifierValue: 'test@test.com',
+      signUpMode: 'public',
+      navigate: mockNavigate,
+      handleError: mockHandleError,
+      clerk: mockClerk as unknown as LoadedClerk,
+      afterSignUpUrl: 'https://test.com',
+      passwordEnabled: false,
+    });
+
+    expect(mockCompleteSignUpFlow).toHaveBeenCalled();
+  });
+
+  it('should call completeSignUpFlow with phone number if phone number is optional field.', async () => {
+    const mockClerk = {
+      client: {
+        signUp: {
+          create: jest.fn().mockImplementation((...args) => Promise.resolve(args)),
+          optionalFields: ['phone_number'],
+        },
+      },
+    };
+
+    await handleCombinedFlowTransfer({
+      identifierAttribute: 'phoneNumber',
+      identifierValue: '+1234567890',
+      signUpMode: 'public',
+      navigate: mockNavigate,
+      handleError: mockHandleError,
+      clerk: mockClerk as unknown as LoadedClerk,
+      afterSignUpUrl: 'https://test.com',
+      passwordEnabled: false,
+    });
+
+    expect(mockNavigate).not.toHaveBeenCalled();
+    expect(mockClerk.client.signUp.create).toHaveBeenCalled();
+    expect(mockCompleteSignUpFlow).toHaveBeenCalled();
+  });
+
+  it('should call navigate if password is enabled', async () => {
+    const mockClerk = {
+      client: {
+        signUp: {
+          create: jest.fn().mockImplementation((...args) => Promise.resolve(args)),
+          optionalFields: ['password'],
+        },
+      },
+    };
+
+    await handleCombinedFlowTransfer({
+      identifierAttribute: 'phoneNumber',
+      identifierValue: '+1234567890',
+      signUpMode: 'public',
+      navigate: mockNavigate,
+      handleError: mockHandleError,
+      clerk: mockClerk as unknown as LoadedClerk,
+      afterSignUpUrl: 'https://test.com',
+      passwordEnabled: true,
+    });
+
+    expect(mockNavigate).toHaveBeenCalled();
+    expect(mockClerk.client.signUp.create).not.toHaveBeenCalled();
+    expect(mockCompleteSignUpFlow).not.toHaveBeenCalled();
+  });
+
+  it('should call navigate if identifier is username', async () => {
+    const mockClerk = {
+      client: {
+        signUp: {
+          create: jest.fn().mockImplementation((...args) => Promise.resolve(args)),
+          optionalFields: [],
+        },
+      },
+    };
+
+    await handleCombinedFlowTransfer({
+      identifierAttribute: 'username',
+      identifierValue: 'test',
+      signUpMode: 'public',
+      navigate: mockNavigate,
+      handleError: mockHandleError,
+      clerk: mockClerk as unknown as LoadedClerk,
+      afterSignUpUrl: 'https://test.com',
+      passwordEnabled: false,
+    });
+
+    expect(mockNavigate).toHaveBeenCalled();
+    expect(mockClerk.client.signUp.create).not.toHaveBeenCalled();
+    expect(mockCompleteSignUpFlow).not.toHaveBeenCalled();
+  });
+
+  it('should call navigate if first_name is optional', async () => {
+    const mockClerk = {
+      client: {
+        signUp: {
+          create: jest.fn().mockImplementation((...args) => Promise.resolve(args)),
+          optionalFields: ['first_name'],
+        },
+      },
+    };
+
+    await handleCombinedFlowTransfer({
+      identifierAttribute: 'emailAddress',
+      identifierValue: 'test@test.com',
+      signUpMode: 'public',
+      navigate: mockNavigate,
+      handleError: mockHandleError,
+      clerk: mockClerk as unknown as LoadedClerk,
+      afterSignUpUrl: 'https://test.com',
+      passwordEnabled: false,
+    });
+
+    expect(mockNavigate).toHaveBeenCalled();
+    expect(mockClerk.client.signUp.create).not.toHaveBeenCalled();
+    expect(mockCompleteSignUpFlow).not.toHaveBeenCalled();
+  });
+});
+
+describe('hasOptionalFields', () => {
+  it('should return true if there are optional fields', () => {
+    const signUp = {
+      optionalFields: ['legal_accepted'],
+    } as unknown as SignUpResource;
+
+    expect(hasOptionalFields(signUp, 'phoneNumber')).toBe(true);
+  });
+
+  it('should return false if the identifier attribute is phoneNumber and the optional field is phone_number', () => {
+    const signUp = {
+      optionalFields: ['phone_number'],
+    } as unknown as SignUpResource;
+
+    expect(hasOptionalFields(signUp, 'phoneNumber')).toBe(false);
+  });
+
+  it('should return false if there are no optional fields', () => {
+    const signUp = {
+      optionalFields: [],
+    } as unknown as SignUpResource;
+
+    expect(hasOptionalFields(signUp, 'phoneNumber')).toBe(false);
+  });
+});

packages/clerk-js/src/ui/components/SignIn/handleCombinedFlowTransfer.ts

@@ -69,7 +69,7 @@ export function handleCombinedFlowTransfer({
   // inform us if the instance is eligible for moving directly to verification.
   if (
     !passwordEnabled &&
-    !hasOptionalFields(clerk.client.signUp) &&
+    !hasOptionalFields(clerk.client.signUp, identifierAttribute) &&
     (identifierAttribute === 'emailAddress' || identifierAttribute === 'phoneNumber')
   ) {
     return clerk.client.signUp
@@ -95,14 +95,28 @@ export function handleCombinedFlowTransfer({
   return navigate(`create`, { searchParams: paramsToForward });
 }
 
-function hasOptionalFields(signUp: SignUpResource) {
-  const filteredFields = signUp.optionalFields.filter(
-    field =>
-      !field.startsWith('oauth_') &&
-      !field.startsWith('web3_') &&
-      field !== 'password' &&
-      field !== 'enterprise_sso' &&
-      field !== 'saml',
-  );
+export function hasOptionalFields(
+  signUp: SignUpResource,
+  identifierAttribute: 'emailAddress' | 'phoneNumber' | 'username',
+) {
+  const filteredFields = signUp.optionalFields.filter(field => {
+    // OAuth, Web3, and SAML fields, while optional, are not relevant once sign up has been initiated with an identifier.
+    if (field.startsWith('oauth_') || field.startsWith('web3_') || ['enterprise_sso', 'saml'].includes(field)) {
+      return false;
+    }
+
+    // We already check for whether password is enabled, so we don't consider it an optional field.
+    if (field === 'password') {
+      return false;
+    }
+
+    // If a phone number is used as the identifier, we don't need to consider the phone_number field.
+    if (identifierAttribute === 'phoneNumber' && field === 'phone_number') {
+      return false;
+    }
+
+    return true;
+  });
+
   return filteredFields.length > 0;
 }