From 9efe5b8e58e3be314b46e494c95ef0378b27c32a Mon Sep 17 00:00:00 2001 From: Sokratis Vidros Date: Fri, 13 May 2022 11:54:49 +0300 Subject: [PATCH 01/22] chore(repo): Do not run ClerkJS twice when publishing --- packages/clerk-js/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/clerk-js/package.json b/packages/clerk-js/package.json index 9ae6b64668c..7195aadc868 100644 --- a/packages/clerk-js/package.json +++ b/packages/clerk-js/package.json @@ -31,7 +31,7 @@ "build:declarations": "tsc -p tsconfig.declarations.json", "check-types": "tsc", "dev": "webpack-dev-server --config webpack.dev.js", - "prepublishOnly": "npm run test && npm run build", + "prepublishOnly": "npm run build", "postpublish": "node ./scripts/purge-cache.mjs", "start": "echo \"Noop\"", "test": "jest", From 658391a29609c8e7b5e5d1e8bed00ffecff00a6b Mon Sep 17 00:00:00 2001 From: Sokratis Vidros Date: Fri, 13 May 2022 17:46:23 +0300 Subject: [PATCH 02/22] Safe url hash build (#239) * chore(clerk-js): Introduce a pathJoin utility * chore(clerk-js): Enhance buildURL utility BuildURL can no build a URL safely by using the native URL() constructor. It can also build a secondary path and search URL that lives inside the hash of the main URL. For example: https://foo.com/bar?qux=42#/hash-bar?hash-qux=42 This is very useful when we need to navigate across pages or between components in a safe way and avoid string concatenations in URLs. * fix(clerk-js): Fix navigateToFactorTwo Navigating to two factor after a sign up transfer flow was broken when the sign in URL contained querystring parameters such as redirect_url or after_sign_up_url. The culprit was the string concatenation was used to build the final factor-two url as we were appending the #/factor-two string after the query strings. This PR fixes the issue by leveraging the new, smarter buildURL utility. --- packages/clerk-js/src/core/clerk.test.ts | 69 ++++++++++--------- packages/clerk-js/src/core/clerk.ts | 6 +- .../ui/contexts/ClerkUIComponentsContext.tsx | 6 ++ packages/clerk-js/src/ui/signUp/SignUp.tsx | 2 +- packages/clerk-js/src/utils/index.ts | 5 +- packages/clerk-js/src/utils/path.test.ts | 18 +++++ packages/clerk-js/src/utils/path.ts | 11 +++ packages/clerk-js/src/utils/url.test.ts | 52 +++++++++++++- packages/clerk-js/src/utils/url.ts | 41 ++++++++++- 9 files changed, 170 insertions(+), 40 deletions(-) create mode 100644 packages/clerk-js/src/utils/path.test.ts create mode 100644 packages/clerk-js/src/utils/path.ts diff --git a/packages/clerk-js/src/core/clerk.test.ts b/packages/clerk-js/src/core/clerk.test.ts index 27332a0fe69..5ba08357da8 100644 --- a/packages/clerk-js/src/core/clerk.test.ts +++ b/packages/clerk-js/src/core/clerk.test.ts @@ -33,13 +33,39 @@ describe('Clerk singleton', () => { let mockNavigate = jest.fn(); const mockDisplayConfig = { - signInUrl: 'signInUrl', - signUpUrl: 'signUpUrl', - userProfileUrl: 'userProfileUrl', + signInUrl: 'http://test.host/sign-in', + signUpUrl: 'http://test.host/sign-up', + userProfileUrl: 'http://test.host/user-profile', } as DisplayConfig; + let mockWindowLocation; + let mockHref: jest.Mock; + + beforeEach(() => { + mockHref = jest.fn(); + mockWindowLocation = { + host: 'test.host', + hostname: 'test.host', + origin: 'http://test.host', + get href() { + return 'http://test.host'; + }, + set href(v: string) { + mockHref(v); + }, + } as any; + + Object.defineProperty(global.window, 'location', { + value: mockWindowLocation, + }); + + // sut = new Clerk(frontendApi); + }); + afterAll(() => { - global.window.location = location; + Object.defineProperty(global.window, 'location', { + value: oldWindowLocation, + }); }); beforeEach(() => { @@ -87,17 +113,17 @@ describe('Clerk singleton', () => { it('redirects to signInUrl', () => { sut.redirectToSignIn({ redirectUrl: 'https://www.example.com/' }); - expect(mockNavigate).toHaveBeenCalledWith('/signInUrl#/?redirect_url=https%3A%2F%2Fwww.example.com%2F'); + expect(mockNavigate).toHaveBeenCalledWith('/sign-in#/?redirect_url=https%3A%2F%2Fwww.example.com%2F'); }); it('redirects to signUpUrl', () => { sut.redirectToSignUp({ redirectUrl: 'https://www.example.com/' }); - expect(mockNavigate).toHaveBeenCalledWith('/signUpUrl#/?redirect_url=https%3A%2F%2Fwww.example.com%2F'); + expect(mockNavigate).toHaveBeenCalledWith('/sign-up#/?redirect_url=https%3A%2F%2Fwww.example.com%2F'); }); it('redirects to userProfileUrl', () => { sut.redirectToUserProfile(); - expect(mockNavigate).toHaveBeenCalledWith('/userProfileUrl'); + expect(mockNavigate).toHaveBeenCalledWith('/user-profile'); }); }); @@ -237,34 +263,15 @@ describe('Clerk singleton', () => { }); describe('.navigate(to)', () => { - let mockWindowLocation; - let mockHref: jest.Mock; let sut: Clerk; beforeEach(() => { - mockHref = jest.fn(); - mockWindowLocation = { - host: 'www.origin1.com', - hostname: 'www.origin1.com', - origin: 'https://www.origin1.com', - get href() { - return 'https://www.origin1.com'; - }, - set href(v: string) { - mockHref(v); - }, - } as any; - - Object.defineProperty(global.window, 'location', { - value: mockWindowLocation, - }); - sut = new Clerk(frontendApi); }); it('uses window location if a custom navigate is not defined', async () => { await sut.load(); - const toUrl = 'https://www.origin1.com/'; + const toUrl = 'http://test.host/'; await sut.navigate(toUrl); expect(mockHref).toHaveBeenCalledWith(toUrl); }); @@ -278,7 +285,7 @@ describe('Clerk singleton', () => { it('wraps custom navigate method in a promise if provided and it sync', async () => { await sut.load({ navigate: mockNavigate }); - const toUrl = 'https://www.origin1.com/path#hash'; + const toUrl = 'http://test.host/path#hash'; const res = sut.navigate(toUrl); expect(res.then).toBeDefined(); expect(mockHref).not.toHaveBeenCalled(); @@ -559,7 +566,7 @@ describe('Clerk singleton', () => { sut.handleRedirectCallback(); await waitFor(() => { - expect(mockNavigate).toHaveBeenCalledWith('/' + mockDisplayConfig.signInUrl + '#/factor-two'); + expect(mockNavigate).toHaveBeenCalledWith('/sign-in#/factor-two'); }); }); @@ -761,13 +768,13 @@ describe('Clerk singleton', () => { sut.handleRedirectCallback(); await waitFor(() => { - expect(mockNavigate).toHaveBeenCalledWith('/signUpUrl'); + expect(mockNavigate).toHaveBeenCalledWith('/sign-up'); }); }); }); describe('.handleMagicLinkVerification()', () => { - beforeEach(async () => { + beforeEach(() => { mockClientFetch.mockReset(); mockEnvironmentFetch.mockReset(); }); diff --git a/packages/clerk-js/src/core/clerk.ts b/packages/clerk-js/src/core/clerk.ts index 1557485e95c..2042b4ec4f1 100644 --- a/packages/clerk-js/src/core/clerk.ts +++ b/packages/clerk-js/src/core/clerk.ts @@ -44,6 +44,7 @@ import { validateFrontendApi, windowNavigate, } from 'utils'; +import { buildURL } from 'utils'; import { getClerkQueryParam } from 'utils/getClerkQueryParam'; import { memoizeListenerCallback } from 'utils/memoizeStateListenerCallback'; @@ -446,7 +447,10 @@ export default class Clerk implements ClerkInterface { const navigateToSignUp = makeNavigate(displayConfig.signUpUrl); - const navigateToFactorTwo = makeNavigate(params.secondFactorUrl || displayConfig.signInUrl + '#/factor-two'); + const navigateToFactorTwo = makeNavigate( + params.secondFactorUrl || + buildURL({ base: displayConfig.signInUrl, hashPath: '/factor-two' }, { stringify: true }), + ); const navigateAfterSignIn = makeNavigate( params.afterSignInUrl || params.redirectUrl || displayConfig.afterSignInUrl, diff --git a/packages/clerk-js/src/ui/contexts/ClerkUIComponentsContext.tsx b/packages/clerk-js/src/ui/contexts/ClerkUIComponentsContext.tsx index 0813c980d23..0dbeb611168 100644 --- a/packages/clerk-js/src/ui/contexts/ClerkUIComponentsContext.tsx +++ b/packages/clerk-js/src/ui/contexts/ClerkUIComponentsContext.tsx @@ -4,6 +4,7 @@ import { useEnvironment } from 'ui/contexts'; import { useNavigate } from 'ui/hooks'; import type { ParsedQs } from 'ui/router'; import { useRouter } from 'ui/router'; +import { buildURL } from 'utils/url'; import type { AvailableComponentCtx, @@ -22,6 +23,7 @@ export type SignUpContextType = SignUpProps & { navigateAfterSignUp: () => any; queryParams: ParsedQs; signInUrl: string; + secondFactorUrl: string; authQueryString: string | null; }; export const useSignUpContext = (): SignUpContextType => { @@ -66,9 +68,13 @@ export const useSignUpContext = (): SignUpContextType => { signInUrl += `#/?${authQs}`; } + // TODO: Avoid building this url again to remove duplicate code. Get it from window.Clerk instead. + const secondFactorUrl = buildURL({ base: signInUrl, hashPath: '/factor-two' }, { stringify: true }); + return { ...ctx, signInUrl, + secondFactorUrl, afterSignUpUrl, afterSignInUrl, navigateAfterSignUp, diff --git a/packages/clerk-js/src/ui/signUp/SignUp.tsx b/packages/clerk-js/src/ui/signUp/SignUp.tsx index 71718bfb421..4b89b2cd0a4 100644 --- a/packages/clerk-js/src/ui/signUp/SignUp.tsx +++ b/packages/clerk-js/src/ui/signUp/SignUp.tsx @@ -32,7 +32,7 @@ function SignUpRoutes(): JSX.Element { afterSignUpUrl={signUpContext.afterSignUpUrl} afterSignInUrl={signUpContext.afterSignInUrl} redirectUrl={signUpContext.redirectUrl} - secondFactorUrl={signUpContext.signInUrl + '#/factor-two'} + secondFactorUrl={signUpContext.secondFactorUrl} /> diff --git a/packages/clerk-js/src/utils/index.ts b/packages/clerk-js/src/utils/index.ts index 04f27f8d6bc..f7cb101b6c4 100644 --- a/packages/clerk-js/src/utils/index.ts +++ b/packages/clerk-js/src/utils/index.ts @@ -1,3 +1,4 @@ +export * from './beforeUnloadTracker'; export * from './cookies'; export * from './customCss'; export * from './customFont'; @@ -11,6 +12,8 @@ export * from './iframe'; export * from './ignoreEventValue'; export * from './instance'; export * from './jwt'; +export * from './pageLifecycle'; +export * from './path'; export * from './querystring'; export * from './runtime'; export * from './runWithExponentialBackOff'; @@ -19,5 +22,3 @@ export * from './script'; export * from './url'; export * from './web3'; export * from './windowNavigate'; -export * from './pageLifecycle'; -export * from './beforeUnloadTracker'; diff --git a/packages/clerk-js/src/utils/path.test.ts b/packages/clerk-js/src/utils/path.test.ts new file mode 100644 index 00000000000..4212168da11 --- /dev/null +++ b/packages/clerk-js/src/utils/path.test.ts @@ -0,0 +1,18 @@ +import { joinPaths } from './path'; + +describe('joinPaths(a, b)', () => { + const cases = [ + [null, null, ''], + [undefined, undefined, ''], + ['', '', ''], + ['foo', 'bar', 'foo/bar'], + ['/foo', '/bar', '/foo/bar'], + ['/foo//', '/bar', '/foo/bar'], + ['/foo///', '//bar//', '/foo/bar/'], + ]; + + // @ts-ignore + it.each(cases)('given %p and %p as arguments, returns %p', (firstPath, secondPath, expectedPath) => { + expect(joinPaths(firstPath, secondPath)).toBe(expectedPath); + }); +}); diff --git a/packages/clerk-js/src/utils/path.ts b/packages/clerk-js/src/utils/path.ts new file mode 100644 index 00000000000..73004bdb97b --- /dev/null +++ b/packages/clerk-js/src/utils/path.ts @@ -0,0 +1,11 @@ +const SEPARATOR = '/'; +const MULTIPLE_SEPARATOR_REGEX = new RegExp(SEPARATOR + '{1,}', 'g'); + +type PathString = string | null | undefined; + +export function joinPaths(a: PathString, b: PathString): string { + return [a, b] + .filter(p => p) + .join(SEPARATOR) + .replace(MULTIPLE_SEPARATOR_REGEX, SEPARATOR); +} diff --git a/packages/clerk-js/src/utils/url.test.ts b/packages/clerk-js/src/utils/url.test.ts index 81790d04a4d..b8ce499d082 100644 --- a/packages/clerk-js/src/utils/url.test.ts +++ b/packages/clerk-js/src/utils/url.test.ts @@ -1,4 +1,5 @@ import { SignUpResource } from '@clerk/types'; + import { appendAsQueryParams, buildURL, @@ -23,7 +24,8 @@ describe('isAccountsHostedPages(url)', () => { ]; test.each(goodUrls)('.isAccountsHostedPages(%s)', (a, expected) => { - expect(isAccountsHostedPages(a as any)).toBe(expected); + // @ts-ignore + expect(isAccountsHostedPages(a)).toBe(expected); }); }); @@ -47,7 +49,8 @@ describe('isDevOrStagingUrl(url)', () => { ]; test.each([...goodUrls, ...badUrls])('.isDevOrStagingUrl(%s)', (a, expected) => { - expect(isDevOrStagingUrl(a as any)).toBe(expected); + // @ts-ignore + expect(isDevOrStagingUrl(a)).toBe(expected); }); }); @@ -136,6 +139,51 @@ describe('buildURL(options: URLParams, skipOrigin)', () => { { stringify: true }, ), ).toBe('http://test.host/foo%3Fbar=42?my-search'); + expect( + buildURL( + { + base: 'http://test.host/', + pathname: '/foo?bar=42', + search: 'my-search=42', + hashPath: '/qux', + hashSearch: 'my-hash-search=42', + }, + { stringify: true }, + ), + ).toBe('http://test.host/foo%3Fbar=42?my-search=42#/qux?my-hash-search=42'); + expect( + buildURL( + { + base: 'http://test.host/', + pathname: '/foo?bar=42', + search: 'my-search=42', + hash: 'my-hash', + hashPath: '/qux', + hashSearch: 'my-hash-search=42', + }, + { stringify: true }, + ), + ).toBe('http://test.host/foo%3Fbar=42?my-search=42#my-hash/qux?my-hash-search=42'); + expect( + buildURL( + { + base: 'http://test.host/', + hash: '?my-hash-search=42', + hashPath: '/foo', + }, + { stringify: true }, + ), + ).toBe('http://test.host/#/foo?my-hash-search=42'); + expect( + buildURL( + { + base: 'http://test.host/foo?my-search=42#my-hash?my-hash-search-1=42', + hashPath: '/qux', + hashSearch: 'my-hash-search-2=42', + }, + { stringify: true }, + ), + ).toBe('http://test.host/foo?my-search=42#my-hash/qux?my-hash-search-1=42&my-hash-search-2=42'); }); }); diff --git a/packages/clerk-js/src/utils/url.ts b/packages/clerk-js/src/utils/url.ts index 15d9d1e62b6..e62722a6f8f 100644 --- a/packages/clerk-js/src/utils/url.ts +++ b/packages/clerk-js/src/utils/url.ts @@ -1,6 +1,8 @@ import { camelToSnake, isIPV4Address } from '@clerk/shared/utils/string'; import { loadScript } from 'utils'; import { SignUpResource } from '@clerk/types'; +import { getQueryParams } from './querystring'; +import { joinPaths } from './path'; declare global { export interface Window { @@ -10,6 +12,9 @@ declare global { } } +// This is used as a dummy base when we need to invoke "new URL()" but we don't care about the URL origin. +const DUMMY_URL_BASE = 'http://clerk-dummy'; + export const DEV_OR_STAGING_SUFFIXES = [ '.lcl.dev', '.stg.dev', @@ -94,6 +99,8 @@ export function getAllETLDs(hostname: string = window.location.hostname): string interface BuildURLParams extends Partial { base?: string; + hashPath?: string; + hashSearch?: string; } interface BuildURLOptions { @@ -105,7 +112,11 @@ interface BuildURLOptions { * * buildURL(params: URLParams, options: BuildURLOptions): string * - * Builds a URL safely by using the native URL() constructor. + * Builds a URL safely by using the native URL() constructor. It can + * also build a secondary path and search URL that lives inside the hash + * of the main URL. For example: + * + * https://foo.com/bar?qux=42#/hash-bar?hash-qux=42 * * References: * https://developer.mozilla.org/en-US/docs/Web/API/URL @@ -120,10 +131,34 @@ export function buildURL( ): B extends true ? string : URL; export function buildURL(params: BuildURLParams, options: BuildURLOptions = {}): URL | string { - const { base, ...rest } = params; - const url = new URL(base || window.location.href); + const { base, hashPath, hashSearch, ...rest } = params; + + const url = new URL(base || '', window.location.href); Object.assign(url, rest); + // Treat that hash part of the main URL as if it's another URL with a pathname and a search. + // Another nested hash inside the top level hash (e.g. #my-hash#my-second-hash) is currently + // not supported as there is no use case for it yet. + if (hashPath || hashSearch) { + // Parse the hash to a URL object + const dummyUrlForHash = new URL(DUMMY_URL_BASE + url.hash.substring(1)); + + // Join the current hash path and with the provided one + dummyUrlForHash.pathname = joinPaths(dummyUrlForHash.pathname, hashPath || ''); + + // Merge search params + const hashSearchParams = getQueryParams(hashSearch || ''); + for (const [key, val] of Object.entries(hashSearchParams)) { + dummyUrlForHash.searchParams.append(key, val as string); + } + + // Keep just the pathname and the search + const newHash = dummyUrlForHash.href.replace(DUMMY_URL_BASE, ''); + + // Assign them to the hash of the main url + url.hash = newHash; + } + const { stringify, skipOrigin } = options; if (stringify) { return skipOrigin ? url.href.replace(url.origin, '') : url.href; From f8943f65d220f828c6905a69be233a11e7a5e471 Mon Sep 17 00:00:00 2001 From: Sokratis Vidros Date: Fri, 13 May 2022 17:46:23 +0300 Subject: [PATCH 03/22] Safe url hash build (#239) * chore(clerk-js): Introduce a pathJoin utility * chore(clerk-js): Enhance buildURL utility BuildURL can no build a URL safely by using the native URL() constructor. It can also build a secondary path and search URL that lives inside the hash of the main URL. For example: https://foo.com/bar?qux=42#/hash-bar?hash-qux=42 This is very useful when we need to navigate across pages or between components in a safe way and avoid string concatenations in URLs. * fix(clerk-js): Fix navigateToFactorTwo Navigating to two factor after a sign up transfer flow was broken when the sign in URL contained querystring parameters such as redirect_url or after_sign_up_url. The culprit was the string concatenation was used to build the final factor-two url as we were appending the #/factor-two string after the query strings. This PR fixes the issue by leveraging the new, smarter buildURL utility. --- packages/clerk-js/src/ui/signUp/SignUp.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/clerk-js/src/ui/signUp/SignUp.tsx b/packages/clerk-js/src/ui/signUp/SignUp.tsx index 4b89b2cd0a4..22acb6b371a 100644 --- a/packages/clerk-js/src/ui/signUp/SignUp.tsx +++ b/packages/clerk-js/src/ui/signUp/SignUp.tsx @@ -4,6 +4,7 @@ import { VerifyMagicLink } from 'ui/common'; import { SSOCallback } from 'ui/common/SSOCallback'; import { ComponentContext, useCoreClerk, useSignUpContext, withCoreSessionSwitchGuard } from 'ui/contexts'; import { Route, Switch, VIRTUAL_ROUTER_BASE_PATH } from 'ui/router'; +import { buildURL } from 'utils/url'; import { SignUpStart } from './SignUpStart'; import { SignUpVerifyEmailAddress, SignUpVerifyPhoneNumber } from './SignUpVerify'; From bfe0d8cb917d9be3441b7f8a9473e905310fe6e4 Mon Sep 17 00:00:00 2001 From: Mark Pitsilos Date: Wed, 13 Apr 2022 14:57:22 +0300 Subject: [PATCH 04/22] feat(clerk-js): Make sign up flow resumable --- packages/clerk-js/src/core/clerk.test.ts | 96 +++++ packages/clerk-js/src/core/clerk.ts | 9 + .../src/ui/common/authForms/Header.tsx | 4 + .../clerk-js/src/ui/common/info/Info.test.tsx | 11 + packages/clerk-js/src/ui/common/info/Info.tsx | 21 ++ packages/clerk-js/src/ui/common/info/index.ts | 1 + .../src/ui/common/withRedirectToHome.tsx | 2 +- packages/clerk-js/src/ui/signUp/SignUp.tsx | 4 + .../src/ui/signUp/SignUpContinue.test.tsx | 306 +++++++++++++++ .../clerk-js/src/ui/signUp/SignUpContinue.tsx | 350 ++++++++++++++++++ .../src/ui/signUp/SignUpStart.test.tsx | 11 +- .../SignUpContinue.test.tsx.snap | 153 ++++++++ packages/clerk-js/src/ui/styles/_info.scss | 13 + packages/clerk-js/src/ui/styles/clerk.scss | 1 + 14 files changed, 977 insertions(+), 5 deletions(-) create mode 100644 packages/clerk-js/src/ui/common/info/Info.test.tsx create mode 100644 packages/clerk-js/src/ui/common/info/Info.tsx create mode 100644 packages/clerk-js/src/ui/common/info/index.ts create mode 100644 packages/clerk-js/src/ui/signUp/SignUpContinue.test.tsx create mode 100644 packages/clerk-js/src/ui/signUp/SignUpContinue.tsx create mode 100644 packages/clerk-js/src/ui/signUp/__snapshots__/SignUpContinue.test.tsx.snap create mode 100644 packages/clerk-js/src/ui/styles/_info.scss diff --git a/packages/clerk-js/src/core/clerk.test.ts b/packages/clerk-js/src/core/clerk.test.ts index 5ba08357da8..9ee43b797a2 100644 --- a/packages/clerk-js/src/core/clerk.test.ts +++ b/packages/clerk-js/src/core/clerk.test.ts @@ -358,6 +358,61 @@ describe('Clerk singleton', () => { }); }); + it('creates a new sign up and navigates to the continue sign-up path if the user was not found during sso signup and there are missing requirements', async () => { + mockEnvironmentFetch.mockReturnValue( + Promise.resolve({ + authConfig: {}, + displayConfig: mockDisplayConfig, + isSingleSession: () => false, + isProduction: () => false, + onWindowLocationHost: () => false, + }), + ); + + mockClientFetch.mockReturnValue( + Promise.resolve({ + activeSessions: [], + signIn: new SignIn({ + status: 'needs_identifier', + first_factor_verification: { + status: 'transferable', + strategy: 'oauth_google', + external_verification_redirect_url: '', + error: { + code: 'external_account_not_found', + long_message: 'The External Account was not found.', + message: 'Invalid external account', + }, + }, + second_factor_verification: null, + identifier: '', + user_data: null, + created_session_id: null, + created_user_id: null, + } as any as SignInJSON), + signUp: new SignUp(null), + }), + ); + + const mockSignUpCreate = jest.fn().mockReturnValue(Promise.resolve({ status: 'missing_requirements' })); + + const sut = new Clerk(frontendApi); + await sut.load({ + navigate: mockNavigate, + }); + if (!sut.client) { + fail('we should always have a client'); + } + sut.client.signUp.create = mockSignUpCreate; + + sut.handleRedirectCallback(); + + await waitFor(() => { + expect(mockSignUpCreate).toHaveBeenCalledWith({ transfer: true }); + expect(mockNavigate).toHaveBeenCalledWith('/signUpUrl#/continue'); + }); + }); + it('signs the user in if the user was found during sign up', async () => { mockEnvironmentFetch.mockReturnValue( Promise.resolve({ @@ -771,6 +826,47 @@ describe('Clerk singleton', () => { expect(mockNavigate).toHaveBeenCalledWith('/sign-up'); }); }); + + it('redirects user to the continue sign-up url if the external account was verified but there are still missing requirements', async () => { + mockEnvironmentFetch.mockReturnValue( + Promise.resolve({ + authConfig: {}, + displayConfig: mockDisplayConfig, + isSingleSession: () => false, + isProduction: () => false, + onWindowLocationHost: () => false, + }), + ); + + mockClientFetch.mockReturnValue( + Promise.resolve({ + activeSessions: [], + signIn: new SignIn(null), + signUp: new SignUp({ + status: 'missing_requirements', + verifications: { + external_account: { + status: 'verified', + strategy: 'oauth_google', + external_verification_redirect_url: '', + error: null, + }, + }, + } as any as SignUpJSON), + }), + ); + + const sut = new Clerk(frontendApi); + await sut.load({ + navigate: mockNavigate, + }); + + sut.handleRedirectCallback(); + + await waitFor(() => { + expect(mockNavigate).toHaveBeenCalledWith('/signUpUrl#/continue'); + }); + }); }); describe('.handleMagicLinkVerification()', () => { diff --git a/packages/clerk-js/src/core/clerk.ts b/packages/clerk-js/src/core/clerk.ts index 2042b4ec4f1..5fd1af46a52 100644 --- a/packages/clerk-js/src/core/clerk.ts +++ b/packages/clerk-js/src/core/clerk.ts @@ -460,6 +460,9 @@ export default class Clerk implements ClerkInterface { params.afterSignUpUrl || params.redirectUrl || displayConfig.afterSignUpUrl, ); + const navigateToContinueSignUp = makeNavigate(displayConfig.signUpUrl + '/continue'); + const navigateToContinueSignUpForTransfer = makeNavigate(displayConfig.signUpUrl + '#/continue'); + const userExistsButNeedsToSignIn = su.externalAccountStatus === 'transferable' && su.externalAccountErrorCode === 'external_account_exists'; @@ -482,6 +485,8 @@ export default class Clerk implements ClerkInterface { switch (res.status) { case 'complete': return this.setSession(res.createdSessionId, navigateAfterSignUp); + case 'missing_requirements': + return navigateToContinueSignUpForTransfer(); default: clerkOAuthCallbackDidNotCompleteSignInSIgnUp('sign in'); } @@ -509,6 +514,10 @@ export default class Clerk implements ClerkInterface { } } + if (su.externalAccountStatus === 'verified' && su.status == 'missing_requirements') { + return navigateToContinueSignUp(); + } + if (hasExternalAccountSignUpError(signUp)) { return navigateToSignUp(); } diff --git a/packages/clerk-js/src/ui/common/authForms/Header.tsx b/packages/clerk-js/src/ui/common/authForms/Header.tsx index 7ea29e9d976..a3be7294f18 100644 --- a/packages/clerk-js/src/ui/common/authForms/Header.tsx +++ b/packages/clerk-js/src/ui/common/authForms/Header.tsx @@ -4,6 +4,7 @@ import React from 'react'; import { Logo } from 'ui/common'; import { BackButton } from 'ui/common/backButton'; import { Error } from 'ui/common/error'; +import { Info } from 'ui/common/info'; export type HeaderProps = { showBack?: boolean; @@ -11,6 +12,7 @@ export type HeaderProps = { handleBack?: () => void; welcomeName?: string; error?: string; + info?: string; } & React.HTMLAttributes; export function Header({ @@ -18,6 +20,7 @@ export function Header({ showLogo = true, welcomeName, error, + info, handleBack, className, }: HeaderProps): JSX.Element { @@ -37,6 +40,7 @@ export function Header({ /> )} {error && {error}} + {info && {info}} {showLogo && (
diff --git a/packages/clerk-js/src/ui/common/info/Info.test.tsx b/packages/clerk-js/src/ui/common/info/Info.test.tsx new file mode 100644 index 00000000000..0275c6f387e --- /dev/null +++ b/packages/clerk-js/src/ui/common/info/Info.test.tsx @@ -0,0 +1,11 @@ +import { renderJSON } from '@clerk/shared/testUtils'; +import * as React from 'react'; + +import { Info } from './Info'; + +describe('', () => { + it('renders the info component', () => { + const tree = renderJSON(Foo); + expect(tree).toMatchSnapshot(); + }); +}); diff --git a/packages/clerk-js/src/ui/common/info/Info.tsx b/packages/clerk-js/src/ui/common/info/Info.tsx new file mode 100644 index 00000000000..9ff3848ad2d --- /dev/null +++ b/packages/clerk-js/src/ui/common/info/Info.tsx @@ -0,0 +1,21 @@ +import React from 'react'; + +export type InfoProps = { + children: React.ReactNode; + style?: {}; +} & React.HTMLAttributes; + +// Renders global info across components, will be replaced by notification snackbars. +export const Info: React.FC = ({ children, style }) => { + if (!children) { + return null; + } + return ( +
+ {children} +
+ ); +}; diff --git a/packages/clerk-js/src/ui/common/info/index.ts b/packages/clerk-js/src/ui/common/info/index.ts new file mode 100644 index 00000000000..3d93d87eb6c --- /dev/null +++ b/packages/clerk-js/src/ui/common/info/index.ts @@ -0,0 +1 @@ +export * from './Info'; diff --git a/packages/clerk-js/src/ui/common/withRedirectToHome.tsx b/packages/clerk-js/src/ui/common/withRedirectToHome.tsx index e6fc8a92fbd..ce01132f912 100644 --- a/packages/clerk-js/src/ui/common/withRedirectToHome.tsx +++ b/packages/clerk-js/src/ui/common/withRedirectToHome.tsx @@ -3,7 +3,7 @@ import React from 'react'; import { useCoreSession, useEnvironment } from 'ui/contexts'; import { useNavigate } from 'ui/hooks'; -export function withRedirectToHome

( +export function withRedirectToHome

( Component: React.ComponentType

, displayName?: string, ): (props: P) => null | JSX.Element { diff --git a/packages/clerk-js/src/ui/signUp/SignUp.tsx b/packages/clerk-js/src/ui/signUp/SignUp.tsx index 22acb6b371a..0bba0997c99 100644 --- a/packages/clerk-js/src/ui/signUp/SignUp.tsx +++ b/packages/clerk-js/src/ui/signUp/SignUp.tsx @@ -6,6 +6,7 @@ import { ComponentContext, useCoreClerk, useSignUpContext, withCoreSessionSwitch import { Route, Switch, VIRTUAL_ROUTER_BASE_PATH } from 'ui/router'; import { buildURL } from 'utils/url'; +import { SignUpContinue } from './SignUpContinue'; import { SignUpStart } from './SignUpStart'; import { SignUpVerifyEmailAddress, SignUpVerifyPhoneNumber } from './SignUpVerify'; @@ -39,6 +40,9 @@ function SignUpRoutes(): JSX.Element { + + + diff --git a/packages/clerk-js/src/ui/signUp/SignUpContinue.test.tsx b/packages/clerk-js/src/ui/signUp/SignUpContinue.test.tsx new file mode 100644 index 00000000000..1fd88ff0a53 --- /dev/null +++ b/packages/clerk-js/src/ui/signUp/SignUpContinue.test.tsx @@ -0,0 +1,306 @@ +import { render, renderJSON, screen, userEvent, waitFor } from '@clerk/shared/testUtils'; +import { UserSettingsJSON } from '@clerk/types'; +import { Session, UserSettings } from 'core/resources/internal'; +import React from 'react'; +import { useCoreSignUp } from 'ui/contexts'; + +import { SignUpContinue } from './SignUpContinue'; + +const navigateMock = jest.fn(); +const mockUpdateRequest = jest.fn(); +const mockSetSession = jest.fn(); +let mockUserSettings: UserSettings; + +jest.mock('ui/router/RouteContext'); + +jest.mock('ui/contexts', () => { + return { + useCoreSession: () => { + return { + id: 'sess_id', + } as Partial; + }, + useSignUpContext: () => { + return { + signInUrl: 'http://test.host/sign-in', + navigateAfterSignUp: jest.fn(), + }; + }, + useCoreClerk: jest.fn(() => ({ + frontendAPI: 'clerk.clerk.dev', + setSession: mockSetSession, + })), + useCoreSignUp: jest.fn(() => ({ + verifications: { + emailAddress: {}, + phoneNumber: {}, + externalAccount: {}, + }, + })), + useEnvironment: jest.fn(() => ({ + displayConfig: { + applicationName: 'My test app', + afterSignUpUrl: 'http://test.host/welcome', + signUpUrl: 'http://test.host/sign-up', + }, + userSettings: mockUserSettings, + authConfig: { singleSessionMode: false }, + })), + }; +}); + +jest.mock('ui/hooks', () => ({ + useNavigate: () => { + return { + navigate: navigateMock, + }; + }, +})); + +describe('', () => { + const { location } = window; + + beforeEach(() => { + mockUserSettings = new UserSettings({ + attributes: { + username: { + enabled: true, + required: true, + }, + first_name: { + enabled: true, + required: true, + }, + last_name: { + enabled: true, + required: true, + }, + password: { + enabled: true, + required: true, + }, + email_address: { + enabled: true, + required: true, + used_for_first_factor: true, + }, + phone_number: { + enabled: true, + }, + }, + social: { + oauth_google: { + enabled: true, + strategy: 'oauth_google', + }, + oauth_facebook: { + enabled: true, + strategy: 'oauth_facebook', + }, + }, + } as UserSettingsJSON); + }); + + beforeEach(() => { + (useCoreSignUp as jest.Mock).mockImplementation(() => { + return { + id: 'su_perman', + update: mockUpdateRequest, + verifications: { + externalAccount: { + status: 'verified', + }, + emailAddress: { + status: 'unverified', + }, + }, + firstName: null, + lastName: null, + emailAddress: 'bryan@taken.com', + phoneNumber: '+12125551001', + username: 'bryanmills', + }; + }); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + afterAll(() => { + global.window.location = location; + }); + + it('renders the sign up continue screen', () => { + const tree = renderJSON(); + expect(tree).toMatchSnapshot(); + }); + + it('redirects to sign-up if no current sign-up exists', () => { + (useCoreSignUp as jest.Mock).mockImplementation(() => { + return {}; + }); + + render(); + expect(navigateMock).toHaveBeenCalledTimes(1); + expect(navigateMock).toHaveBeenCalledWith('http://test.host/sign-up'); + }); + + it('pre-fills form with unverified email', () => { + render(); + + expect(screen.getByLabelText('Email address')).toHaveValue('bryan@taken.com'); + }); + + it('does not show oauth providers if we already have a verified external account', () => { + render(); + + expect(screen.queryByRole('button', { name: 'Sign up with Facebook' })).not.toBeInTheDocument(); + expect(screen.queryByRole('button', { name: 'Sign up with Google' })).not.toBeInTheDocument(); + }); + + it('patches the existing signup when user submits the form', async () => { + mockUpdateRequest.mockImplementation(() => + Promise.resolve({ + firstName: 'Bryan', + lastName: 'Mills', + emailAddress: 'bryan@taken.com', + verifications: { + emailAddress: { + status: 'unverified', + }, + }, + }), + ); + + render(); + + const firstNameInput = screen.getByLabelText('First name'); + userEvent.clear(firstNameInput); + userEvent.type(screen.getByLabelText('First name'), 'Bryan'); + + const lastNameInput = screen.getByLabelText('Last name'); + userEvent.clear(lastNameInput); + userEvent.type(lastNameInput, 'Mills'); + + userEvent.click(screen.getByRole('button', { name: 'Sign up' })); + + await waitFor(() => { + expect(mockUpdateRequest).toHaveBeenCalledTimes(1); + expect(mockUpdateRequest).toHaveBeenCalledWith({ + first_name: 'Bryan', + last_name: 'Mills', + email_address: 'bryan@taken.com', + }); + expect(navigateMock).toHaveBeenCalledTimes(1); + expect(navigateMock).toHaveBeenCalledWith('http://test.host/sign-up/verify-email-address'); + }); + }); + + it('skips the email input if the email is already verified', () => { + (useCoreSignUp as jest.Mock).mockImplementation(() => { + return { + id: 'su_perman', + update: mockUpdateRequest, + verifications: { + externalAccount: { + status: 'verified', + }, + emailAddress: { + status: 'verified', + }, + }, + firstName: null, + lastName: null, + emailAddress: 'bryan@taken.com', + phoneNumber: '+12125551001', + username: 'bryanmills', + }; + }); + + render(); + + expect(screen.queryByText('Email address')).not.toBeInTheDocument(); + }); + + it('skips the password input if there is a verified external account', () => { + render(); + + expect(screen.queryByText('Password')).not.toBeInTheDocument(); + }); + + it('skips already collected fields if they need no verification', () => { + (useCoreSignUp as jest.Mock).mockImplementation(() => { + return { + id: 'su_perman', + update: mockUpdateRequest, + verifications: { + externalAccount: { + status: 'verified', + }, + emailAddress: { + status: 'unverified', + }, + }, + firstName: 'Bryan', + lastName: 'Mills', + emailAddress: 'bryan@taken.com', + phoneNumber: '+12125551001', + username: 'bryanmills', + }; + }); + + render(); + + expect(screen.queryByText('First name')).not.toBeInTheDocument(); + expect(screen.queryByText('Last name')).not.toBeInTheDocument(); + expect(screen.queryByText('Username')).not.toBeInTheDocument(); + }); + + it('skips non-required fields', () => { + mockUserSettings = new UserSettings({ + attributes: { + username: { + enabled: true, + required: false, + }, + first_name: { + enabled: true, + required: false, + }, + last_name: { + enabled: true, + required: false, + }, + password: { + enabled: true, + required: true, + }, + email_address: { + enabled: true, + required: true, + used_for_first_factor: true, + }, + phone_number: { + enabled: true, + }, + }, + social: { + oauth_google: { + enabled: true, + strategy: 'oauth_google', + }, + oauth_facebook: { + enabled: true, + strategy: 'oauth_facebook', + }, + }, + } as UserSettingsJSON); + + render(); + + expect(screen.queryByText('First name')).not.toBeInTheDocument(); + expect(screen.queryByText('Last name')).not.toBeInTheDocument(); + expect(screen.queryByText('Username')).not.toBeInTheDocument(); + }); +}); diff --git a/packages/clerk-js/src/ui/signUp/SignUpContinue.tsx b/packages/clerk-js/src/ui/signUp/SignUpContinue.tsx new file mode 100644 index 00000000000..459becaab35 --- /dev/null +++ b/packages/clerk-js/src/ui/signUp/SignUpContinue.tsx @@ -0,0 +1,350 @@ +import { Control } from '@clerk/shared/components/control'; +import { Form } from '@clerk/shared/components/form'; +import { Input } from '@clerk/shared/components/input'; +import { PhoneInput } from '@clerk/shared/components/phoneInput'; +import { SignUpResource } from '@clerk/types'; +import React from 'react'; +import type { FieldState } from 'ui/common'; +import { + buildRequest, + Footer, + handleError, + PoweredByClerk, + Separator, + useFieldState, + withRedirectToHome, +} from 'ui/common'; +import { Body, Header } from 'ui/common/authForms'; +import { useCoreClerk, useCoreSignUp, useEnvironment, useSignUpContext } from 'ui/contexts'; +import { useNavigate } from 'ui/hooks'; + +import { SignInLink } from './SignInLink'; +import { SignUpOAuth } from './SignUpOAuth'; +import { SignUpWeb3 } from './SignUpWeb3'; +import { determineFirstPartyFields } from './utils'; + +type ActiveIdentifier = 'emailAddress' | 'phoneNumber'; + +function _SignUpContinue(): JSX.Element | null { + const { navigate } = useNavigate(); + const environment = useEnvironment(); + const { displayConfig, userSettings } = environment; + const { setSession } = useCoreClerk(); + const { navigateAfterSignUp } = useSignUpContext(); + const [emailOrPhoneActive, setEmailOrPhoneActive] = React.useState('emailAddress'); + const signUp = useCoreSignUp(); + + // Redirect to sign-up if there is no persisted sign-up + if (!signUp.id) { + void navigate(displayConfig.signUpUrl); + return null; + } + + // Pre-populate fields from existing sign-up object + const formFields = { + firstName: useFieldState('first_name', signUp.firstName || ''), + lastName: useFieldState('last_name', signUp.lastName || ''), + emailAddress: useFieldState('email_address', signUp.emailAddress || ''), + username: useFieldState('username', signUp.username || ''), + phoneNumber: useFieldState('phone_number', signUp.phoneNumber || ''), + password: useFieldState('password', ''), + } as const; + + type FormFieldsKey = keyof typeof formFields; + + const [error, setError] = React.useState(); + + const hasVerifiedEmail = signUp.verifications?.emailAddress?.status == 'verified'; + const hasVerifiedPhone = signUp.verifications?.phoneNumber?.status == 'verified'; + const hasVerifiedExternalAccount = signUp.verifications?.externalAccount?.status == 'verified'; + + const fields = determineFirstPartyFields(environment, false); + + // Show only fields absolutely necessary for completing sign-up to minimize fiction + + if (hasVerifiedEmail) { + delete fields.emailAddress; + delete fields.emailOrPhone; + } + + if (hasVerifiedPhone) { + delete fields.phoneNumber; + delete fields.emailOrPhone; + } + + if (hasVerifiedExternalAccount) { + delete fields.password; + } + + if (signUp.firstName) { + delete fields.firstName; + } + + if (signUp.lastName) { + delete fields.lastName; + } + + if (signUp.username) { + delete fields.username; + } + + // Remove any non-required fields + Object.entries(fields).forEach(([k, v]) => { + if (v !== 'required') { + // @ts-ignore + delete fields[k]; + } + }); + + const oauthOptions = userSettings.socialProviderStrategies; + const web3Options = userSettings.web3FirstFactors; + + // Handle oauth errors? + + // React.useEffect(() => { + // async function handleOauthError() { + // const error = signUp.verifications.externalAccount.error; + // + // if (error) { + // switch (error.code) { + // case ERROR_CODES.NOT_ALLOWED_TO_SIGN_UP: + // case ERROR_CODES.OAUTH_ACCESS_DENIED: + // setError(error.longMessage); + // break; + // default: + // // Error from server may be too much information for the end user, so set a generic error + // setError('Unable to complete action at this time. If the problem persists please contact support.'); + // } + // + // // TODO: This is a hack to reset the sign in attempt so that the oauth error + // // does not persist on full page reloads. + // // + // // We will revise this strategy as part of the Clerk DX epic. + // void (await signUp.create({})); + // } + // } + // + // void handleOauthError(); + // }); + + const handleChangeActive = (type: ActiveIdentifier) => (e: React.MouseEvent) => { + e.preventDefault(); + if (!fields.emailOrPhone) { + return; + } + setEmailOrPhoneActive(type); + }; + + const handleSubmit = async (e: React.FormEvent) => { + e.preventDefault(); + + const reqFields = Object.entries(fields).reduce( + (acc, [k, v]) => [...acc, ...(v && formFields[k as FormFieldsKey] ? [formFields[k as FormFieldsKey]] : [])], + [] as Array>, + ); + + if (fields.emailOrPhone && emailOrPhoneActive === 'emailAddress') { + reqFields.push(formFields.emailAddress); + } + + if (fields.emailOrPhone && emailOrPhoneActive === 'phoneNumber') { + reqFields.push(formFields.phoneNumber); + } + + try { + setError(undefined); + const req = buildRequest(reqFields); + const res = await signUp.update(req); + return completeSignUpFlow(res); + } catch (err) { + handleError(err, reqFields, setError); + } + }; + + const completeSignUpFlow = (su: SignUpResource) => { + if (su.status === 'complete') { + return setSession(su.createdSessionId, navigateAfterSignUp); + } else if (su.emailAddress && su.verifications.emailAddress.status !== 'verified') { + return navigate('../verify-email-address'); + } else if (su.phoneNumber && su.verifications.phoneNumber.status !== 'verified') { + return navigate('../verify-phone-number'); + } else if (su.status === 'missing_requirements') { + // do nothing + } + }; + + const firstNameField = fields.firstName ? ( + + formFields.firstName.setValue(el.value || '')} + /> + + ) : null; + + const lastNameField = fields.lastName ? ( + + formFields.lastName.setValue(el.value || '')} + /> + + ) : null; + + const nameField = (fields.firstName || fields.lastName) && ( +

+ {firstNameField} + {lastNameField} +
+ ); + + const usernameField = fields.username ? ( + + formFields.username.setValue(el.value || '')} + /> + + ) : null; + + const passwordField = fields.password ? ( + + formFields.password.setValue(el.value || '')} + /> + + ) : null; + + const shouldShowEmailAddressField = + fields.emailAddress || (fields.emailOrPhone && emailOrPhoneActive === 'emailAddress'); + + const emailAddressField = shouldShowEmailAddressField && ( + + formFields.emailAddress.setValue(el.value || '')} + /> + + ); + + const shouldShowPhoneNumberField = + fields.phoneNumber || (fields.emailOrPhone && emailOrPhoneActive === 'phoneNumber'); + + const phoneNumberField = shouldShowPhoneNumberField ? ( + + + + ) : null; + + const atLeastOneFormField = nameField || usernameField || emailAddressField || phoneNumberField || passwordField; + + const showOauthProviders = !hasVerifiedExternalAccount && oauthOptions.length > 0; + + const showWeb3Providers = web3Options.length > 0; + + return ( + <> +
+ + {showOauthProviders && ( + + )} + {showWeb3Providers && ( + + )} + {atLeastOneFormField && ( + <> + {(showOauthProviders || showWeb3Providers) && } + + {/* @ts-ignore */} +
+ <> + {nameField} + {usernameField} + {emailAddressField} + {phoneNumberField} + {passwordField} + +
+ + )} +
+ + +
+ + + ); +} + +export const SignUpContinue = withRedirectToHome(_SignUpContinue); diff --git a/packages/clerk-js/src/ui/signUp/SignUpStart.test.tsx b/packages/clerk-js/src/ui/signUp/SignUpStart.test.tsx index 932583477a9..ae5a57d8824 100644 --- a/packages/clerk-js/src/ui/signUp/SignUpStart.test.tsx +++ b/packages/clerk-js/src/ui/signUp/SignUpStart.test.tsx @@ -1,12 +1,11 @@ -import { mocked, render, renderJSON, screen, userEvent, waitFor } from '@clerk/shared/testUtils'; +import { render, renderJSON, screen, userEvent, waitFor } from '@clerk/shared/testUtils'; import { titleize } from '@clerk/shared/utils/string'; -import { SignInResource, UserSettingsJSON } from '@clerk/types'; +import { UserSettingsJSON } from '@clerk/types'; import { Session, UserSettings } from 'core/resources/internal'; import React from 'react'; -import { useCoreSignIn, useCoreSignUp } from 'ui/contexts'; +import { useCoreSignUp } from 'ui/contexts'; import { SignUpStart } from './SignUpStart'; -import { SignInStart } from 'ui/signIn/SignInStart'; const navigateMock = jest.fn(); const mockCreateRequest = jest.fn(); @@ -323,6 +322,10 @@ describe('', () => { } as UserSettingsJSON); }); + afterEach(() => { + setWindowQueryParams([]); + }); + it('it auto-completes sign up flow if sign up is complete after create', async () => { mockCreateRequest.mockImplementation(() => Promise.resolve({ diff --git a/packages/clerk-js/src/ui/signUp/__snapshots__/SignUpContinue.test.tsx.snap b/packages/clerk-js/src/ui/signUp/__snapshots__/SignUpContinue.test.tsx.snap new file mode 100644 index 00000000000..b21caa4d459 --- /dev/null +++ b/packages/clerk-js/src/ui/signUp/__snapshots__/SignUpContinue.test.tsx.snap @@ -0,0 +1,153 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[` renders the sign up continue screen 1`] = ` +Array [ +
+
+ Please fill in the following information to complete your sign-up +
+
+
+ My test app +
+
+
, +
+
+
+
+
+ + +
+
+ + +
+
+
+ + +
+
+
+ +
+
+
+
+ + Already have an account? + + + + Sign in + +
+
+
, +] +`; diff --git a/packages/clerk-js/src/ui/styles/_info.scss b/packages/clerk-js/src/ui/styles/_info.scss new file mode 100644 index 00000000000..b0436771606 --- /dev/null +++ b/packages/clerk-js/src/ui/styles/_info.scss @@ -0,0 +1,13 @@ +.info { + padding: 1em; + margin-bottom: 1em; + border-radius: 0.5em; + + color: $primary; + background-color: #e8fcff; + + font-size: 1rem; + line-height: 1.25rem; + font-weight: $semibold-weight; + text-align: center; +} diff --git a/packages/clerk-js/src/ui/styles/clerk.scss b/packages/clerk-js/src/ui/styles/clerk.scss index f12a5ab8aa1..18c7a19e673 100644 --- a/packages/clerk-js/src/ui/styles/clerk.scss +++ b/packages/clerk-js/src/ui/styles/clerk.scss @@ -31,6 +31,7 @@ $cl-red-O10: rgba(205, 68, 74, 0.1); @import 'authForms'; @import 'backButton'; @import 'error'; + @import 'info'; @import 'userprofile'; @import 'userbutton'; @import 'backButton'; From 0692fad420cb64a0b32abe92a4e7e76ebfb4ac73 Mon Sep 17 00:00:00 2001 From: Mark Pitsilos Date: Tue, 10 May 2022 00:41:50 +0300 Subject: [PATCH 05/22] chore(clerk-js): Move sign-up fields to SignUpForm, move field determination logic to utils --- .../src/ui/signUp/SignUpContinue.test.tsx | 76 +++- .../clerk-js/src/ui/signUp/SignUpContinue.tsx | 272 ++----------- .../clerk-js/src/ui/signUp/SignUpForm.tsx | 144 +++++++ .../src/ui/signUp/SignUpStart.test.tsx | 64 ++- .../clerk-js/src/ui/signUp/SignUpStart.tsx | 229 +++-------- .../SignUpContinue.test.tsx.snap | 1 + .../ui/signUp/sign_up_form_helpers.test.ts | 385 ++++++++++++++++++ .../src/ui/signUp/sign_up_form_helpers.ts | 188 +++++++++ packages/clerk-js/src/ui/signUp/utils.test.ts | 218 ---------- packages/clerk-js/src/ui/signUp/utils.ts | 49 --- 10 files changed, 951 insertions(+), 675 deletions(-) create mode 100644 packages/clerk-js/src/ui/signUp/SignUpForm.tsx create mode 100644 packages/clerk-js/src/ui/signUp/sign_up_form_helpers.test.ts create mode 100644 packages/clerk-js/src/ui/signUp/sign_up_form_helpers.ts delete mode 100644 packages/clerk-js/src/ui/signUp/utils.test.ts delete mode 100644 packages/clerk-js/src/ui/signUp/utils.ts diff --git a/packages/clerk-js/src/ui/signUp/SignUpContinue.test.tsx b/packages/clerk-js/src/ui/signUp/SignUpContinue.test.tsx index 1fd88ff0a53..8ec0f3ebb77 100644 --- a/packages/clerk-js/src/ui/signUp/SignUpContinue.test.tsx +++ b/packages/clerk-js/src/ui/signUp/SignUpContinue.test.tsx @@ -86,6 +86,7 @@ describe('', () => { }, phone_number: { enabled: true, + required: false, }, }, social: { @@ -177,7 +178,7 @@ describe('', () => { const firstNameInput = screen.getByLabelText('First name'); userEvent.clear(firstNameInput); - userEvent.type(screen.getByLabelText('First name'), 'Bryan'); + userEvent.type(firstNameInput, 'Bryan'); const lastNameInput = screen.getByLabelText('Last name'); userEvent.clear(lastNameInput); @@ -223,6 +224,77 @@ describe('', () => { expect(screen.queryByText('Email address')).not.toBeInTheDocument(); }); + it('redirects to the phone verification step if the user completes their sign up by providing their phone', async () => { + mockUserSettings = new UserSettings({ + attributes: { + username: { + enabled: true, + required: false, + }, + first_name: { + enabled: true, + required: false, + }, + last_name: { + enabled: true, + required: false, + }, + password: { + enabled: true, + required: true, + }, + email_address: { + enabled: true, + required: false, + used_for_first_factor: false, + }, + phone_number: { + enabled: true, + required: true, + used_for_first_factor: true, + }, + }, + social: { + oauth_google: { + enabled: true, + strategy: 'oauth_google', + }, + oauth_facebook: { + enabled: true, + strategy: 'oauth_facebook', + }, + }, + } as UserSettingsJSON); + + mockUpdateRequest.mockImplementation(() => + Promise.resolve({ + phoneNumber: '+15615551001', + verifications: { + phoneNumber: { + status: 'unverified', + }, + }, + }), + ); + + render(); + + const phoneNumberInput = screen.getByRole('textbox'); + userEvent.clear(phoneNumberInput); + userEvent.type(phoneNumberInput, '5615551001'); + + userEvent.click(screen.getByRole('button', { name: 'Sign up' })); + + await waitFor(() => { + expect(mockUpdateRequest).toHaveBeenCalledTimes(1); + expect(mockUpdateRequest).toHaveBeenCalledWith({ + phone_number: '+15615551001', + }); + expect(navigateMock).toHaveBeenCalledTimes(1); + expect(navigateMock).toHaveBeenCalledWith('http://test.host/sign-up/verify-phone-number'); + }); + }); + it('skips the password input if there is a verified external account', () => { render(); @@ -283,6 +355,7 @@ describe('', () => { }, phone_number: { enabled: true, + required: false, }, }, social: { @@ -299,6 +372,7 @@ describe('', () => { render(); + expect(screen.queryByText('Phone number')).not.toBeInTheDocument(); expect(screen.queryByText('First name')).not.toBeInTheDocument(); expect(screen.queryByText('Last name')).not.toBeInTheDocument(); expect(screen.queryByText('Username')).not.toBeInTheDocument(); diff --git a/packages/clerk-js/src/ui/signUp/SignUpContinue.tsx b/packages/clerk-js/src/ui/signUp/SignUpContinue.tsx index 459becaab35..b89f0589415 100644 --- a/packages/clerk-js/src/ui/signUp/SignUpContinue.tsx +++ b/packages/clerk-js/src/ui/signUp/SignUpContinue.tsx @@ -1,7 +1,3 @@ -import { Control } from '@clerk/shared/components/control'; -import { Form } from '@clerk/shared/components/form'; -import { Input } from '@clerk/shared/components/input'; -import { PhoneInput } from '@clerk/shared/components/phoneInput'; import { SignUpResource } from '@clerk/types'; import React from 'react'; import type { FieldState } from 'ui/common'; @@ -17,147 +13,95 @@ import { import { Body, Header } from 'ui/common/authForms'; import { useCoreClerk, useCoreSignUp, useEnvironment, useSignUpContext } from 'ui/contexts'; import { useNavigate } from 'ui/hooks'; +import { SignUpForm } from 'ui/signUp/SignUpForm'; +import { + ActiveIdentifier, + determineActiveFields, + emailOrPhoneUsedForFF, + getInitialActiveIdentifier, + minimizeFieldsForExistingSignup, + showFormFields, +} from './sign_up_form_helpers'; import { SignInLink } from './SignInLink'; import { SignUpOAuth } from './SignUpOAuth'; import { SignUpWeb3 } from './SignUpWeb3'; -import { determineFirstPartyFields } from './utils'; - -type ActiveIdentifier = 'emailAddress' | 'phoneNumber'; function _SignUpContinue(): JSX.Element | null { const { navigate } = useNavigate(); const environment = useEnvironment(); const { displayConfig, userSettings } = environment; + const { attributes } = userSettings; const { setSession } = useCoreClerk(); const { navigateAfterSignUp } = useSignUpContext(); - const [emailOrPhoneActive, setEmailOrPhoneActive] = React.useState('emailAddress'); + const [activeCommIdentifierType, setActiveCommIdentifierType] = React.useState( + getInitialActiveIdentifier(attributes), + ); const signUp = useCoreSignUp(); // Redirect to sign-up if there is no persisted sign-up + if (!signUp.id) { void navigate(displayConfig.signUpUrl); return null; } // Pre-populate fields from existing sign-up object - const formFields = { + + const formState = { firstName: useFieldState('first_name', signUp.firstName || ''), lastName: useFieldState('last_name', signUp.lastName || ''), emailAddress: useFieldState('email_address', signUp.emailAddress || ''), username: useFieldState('username', signUp.username || ''), phoneNumber: useFieldState('phone_number', signUp.phoneNumber || ''), password: useFieldState('password', ''), + ticket: useFieldState('ticket', ''), } as const; - type FormFieldsKey = keyof typeof formFields; + type FormStateKey = keyof typeof formState; const [error, setError] = React.useState(); - const hasVerifiedEmail = signUp.verifications?.emailAddress?.status == 'verified'; - const hasVerifiedPhone = signUp.verifications?.phoneNumber?.status == 'verified'; + const hasEmail = !!formState.emailAddress.value; const hasVerifiedExternalAccount = signUp.verifications?.externalAccount?.status == 'verified'; - const fields = determineFirstPartyFields(environment, false); - - // Show only fields absolutely necessary for completing sign-up to minimize fiction - - if (hasVerifiedEmail) { - delete fields.emailAddress; - delete fields.emailOrPhone; - } - - if (hasVerifiedPhone) { - delete fields.phoneNumber; - delete fields.emailOrPhone; - } - - if (hasVerifiedExternalAccount) { - delete fields.password; - } - - if (signUp.firstName) { - delete fields.firstName; - } - - if (signUp.lastName) { - delete fields.lastName; - } - - if (signUp.username) { - delete fields.username; - } - - // Remove any non-required fields - Object.entries(fields).forEach(([k, v]) => { - if (v !== 'required') { - // @ts-ignore - delete fields[k]; - } + const fields = determineActiveFields({ + environment, + hasEmail, + activeCommIdentifierType, + signUp, }); + minimizeFieldsForExistingSignup(fields, signUp); + const oauthOptions = userSettings.socialProviderStrategies; const web3Options = userSettings.web3FirstFactors; - // Handle oauth errors? - - // React.useEffect(() => { - // async function handleOauthError() { - // const error = signUp.verifications.externalAccount.error; - // - // if (error) { - // switch (error.code) { - // case ERROR_CODES.NOT_ALLOWED_TO_SIGN_UP: - // case ERROR_CODES.OAUTH_ACCESS_DENIED: - // setError(error.longMessage); - // break; - // default: - // // Error from server may be too much information for the end user, so set a generic error - // setError('Unable to complete action at this time. If the problem persists please contact support.'); - // } - // - // // TODO: This is a hack to reset the sign in attempt so that the oauth error - // // does not persist on full page reloads. - // // - // // We will revise this strategy as part of the Clerk DX epic. - // void (await signUp.create({})); - // } - // } - // - // void handleOauthError(); - // }); - const handleChangeActive = (type: ActiveIdentifier) => (e: React.MouseEvent) => { e.preventDefault(); - if (!fields.emailOrPhone) { + + if (!emailOrPhoneUsedForFF(attributes)) { return; } - setEmailOrPhoneActive(type); + + setActiveCommIdentifierType(type); }; const handleSubmit = async (e: React.FormEvent) => { e.preventDefault(); - const reqFields = Object.entries(fields).reduce( - (acc, [k, v]) => [...acc, ...(v && formFields[k as FormFieldsKey] ? [formFields[k as FormFieldsKey]] : [])], + const fieldsToSubmit = Object.entries(fields).reduce( + (acc, [k, v]) => [...acc, ...(v.enabled && formState[k as FormStateKey] ? [formState[k as FormStateKey]] : [])], [] as Array>, ); - if (fields.emailOrPhone && emailOrPhoneActive === 'emailAddress') { - reqFields.push(formFields.emailAddress); - } - - if (fields.emailOrPhone && emailOrPhoneActive === 'phoneNumber') { - reqFields.push(formFields.phoneNumber); - } - try { setError(undefined); - const req = buildRequest(reqFields); + const req = buildRequest(fieldsToSubmit); const res = await signUp.update(req); return completeSignUpFlow(res); } catch (err) { - handleError(err, reqFields, setError); + handleError(err, fieldsToSubmit, setError); } }; @@ -165,135 +109,14 @@ function _SignUpContinue(): JSX.Element | null { if (su.status === 'complete') { return setSession(su.createdSessionId, navigateAfterSignUp); } else if (su.emailAddress && su.verifications.emailAddress.status !== 'verified') { - return navigate('../verify-email-address'); + return navigate(displayConfig.signUpUrl + '/verify-email-address'); } else if (su.phoneNumber && su.verifications.phoneNumber.status !== 'verified') { - return navigate('../verify-phone-number'); + return navigate(displayConfig.signUpUrl + '/verify-phone-number'); } else if (su.status === 'missing_requirements') { // do nothing } }; - const firstNameField = fields.firstName ? ( - - formFields.firstName.setValue(el.value || '')} - /> - - ) : null; - - const lastNameField = fields.lastName ? ( - - formFields.lastName.setValue(el.value || '')} - /> - - ) : null; - - const nameField = (fields.firstName || fields.lastName) && ( -
- {firstNameField} - {lastNameField} -
- ); - - const usernameField = fields.username ? ( - - formFields.username.setValue(el.value || '')} - /> - - ) : null; - - const passwordField = fields.password ? ( - - formFields.password.setValue(el.value || '')} - /> - - ) : null; - - const shouldShowEmailAddressField = - fields.emailAddress || (fields.emailOrPhone && emailOrPhoneActive === 'emailAddress'); - - const emailAddressField = shouldShowEmailAddressField && ( - - formFields.emailAddress.setValue(el.value || '')} - /> - - ); - - const shouldShowPhoneNumberField = - fields.phoneNumber || (fields.emailOrPhone && emailOrPhoneActive === 'phoneNumber'); - - const phoneNumberField = shouldShowPhoneNumberField ? ( - - - - ) : null; - - const atLeastOneFormField = nameField || usernameField || emailAddressField || phoneNumberField || passwordField; - const showOauthProviders = !hasVerifiedExternalAccount && oauthOptions.length > 0; const showWeb3Providers = web3Options.length > 0; @@ -312,30 +135,25 @@ function _SignUpContinue(): JSX.Element | null { setError={setError} /> )} + {showWeb3Providers && ( )} - {atLeastOneFormField && ( + + {showFormFields(userSettings) && ( <> {(showOauthProviders || showWeb3Providers) && } - {/* @ts-ignore */} -
- <> - {nameField} - {usernameField} - {emailAddressField} - {phoneNumberField} - {passwordField} - -
+ handleChangeActive={handleChangeActive} + /> )}