From 21f61ce1da8cad98ed12a98379a6d6c99c39b5ba Mon Sep 17 00:00:00 2001 From: George Desipris <73396808+desiprisg@users.noreply.github.com> Date: Wed, 18 Oct 2023 01:35:22 +0300 Subject: [PATCH] fix(clerk-js): Append initial values query params after hash (#1855) * fix(clerk-js): Append initial values query params after hash * refactor(clerk-js): Simplify appendAsQueryParams and handle urls higher up --- .changeset/wise-spiders-hammer.md | 5 +++ packages/clerk-js/src/core/clerk.ts | 26 ++++++++------ .../clerk-js/src/utils/__tests__/url.test.ts | 34 +++++++------------ packages/clerk-js/src/utils/url.ts | 30 +++++----------- 4 files changed, 43 insertions(+), 52 deletions(-) create mode 100644 .changeset/wise-spiders-hammer.md diff --git a/.changeset/wise-spiders-hammer.md b/.changeset/wise-spiders-hammer.md new file mode 100644 index 00000000000..a56c78677f2 --- /dev/null +++ b/.changeset/wise-spiders-hammer.md @@ -0,0 +1,5 @@ +--- +'@clerk/clerk-js': patch +--- + +Append query params for sign-in and sign-up initial values after the hash in order to be readable via hash routing. diff --git a/packages/clerk-js/src/core/clerk.ts b/packages/clerk-js/src/core/clerk.ts index 63489652951..c12f4aebe5f 100644 --- a/packages/clerk-js/src/core/clerk.ts +++ b/packages/clerk-js/src/core/clerk.ts @@ -63,7 +63,6 @@ import type { MountComponentRenderer } from '../ui/Components'; import { completeSignUpFlow } from '../ui/components/SignUp/util'; import { appendAsQueryParams, - appendUrlsAsQueryParams, buildURL, createBeforeUnloadTracker, createCookieHandler, @@ -86,6 +85,8 @@ import { sessionExistsAndSingleSessionModeEnabled, setDevBrowserJWTInURL, stripOrigin, + stripSameOrigin, + toURL, validateFrontendApi, windowNavigate, } from '../utils'; @@ -1588,21 +1589,26 @@ export default class Clerk implements ClerkInterface { return ''; } - const opts: RedirectOptions = { - afterSignInUrl: pickRedirectionProp('afterSignInUrl', { ctx: options, options: this.#options }, false), - afterSignUpUrl: pickRedirectionProp('afterSignUpUrl', { ctx: options, options: this.#options }, false), - redirectUrl: options?.redirectUrl || window.location.href, - }; - const signInOrUpUrl = pickRedirectionProp( key, { options: this.#options, displayConfig: this.#environment.displayConfig }, false, ); - return this.buildUrlWithAuth( - appendUrlsAsQueryParams(appendAsQueryParams(signInOrUpUrl, options?.initialValues || {}), opts), - ); + const urls: RedirectOptions = { + afterSignInUrl: pickRedirectionProp('afterSignInUrl', { ctx: options, options: this.#options }, false), + afterSignUpUrl: pickRedirectionProp('afterSignUpUrl', { ctx: options, options: this.#options }, false), + redirectUrl: options?.redirectUrl || window.location.href, + }; + + (Object.keys(urls) as Array).forEach(function (key) { + const url = urls[key]; + if (url) { + urls[key] = stripSameOrigin(toURL(url), toURL(signInOrUpUrl)); + } + }); + + return this.buildUrlWithAuth(appendAsQueryParams(signInOrUpUrl, { ...urls, ...options?.initialValues })); }; assertComponentsReady(controls: unknown): asserts controls is ReturnType { diff --git a/packages/clerk-js/src/utils/__tests__/url.test.ts b/packages/clerk-js/src/utils/__tests__/url.test.ts index 68d3d3c63b7..cd4c075068c 100644 --- a/packages/clerk-js/src/utils/__tests__/url.test.ts +++ b/packages/clerk-js/src/utils/__tests__/url.test.ts @@ -1,7 +1,7 @@ import type { SignUpResource } from '@clerk/types'; import { - appendUrlsAsQueryParams, + appendAsQueryParams, buildURL, getAllETLDs, getETLDPlusOneFromFrontendApi, @@ -242,58 +242,50 @@ describe('trimTrailingSlash(string)', () => { describe('appendQueryParams(base,url)', () => { it('returns the same url if no params provided', () => { const base = new URL('https://dashboard.clerk.com'); - const res = appendUrlsAsQueryParams(base); + const res = appendAsQueryParams(base); expect(res).toBe('https://dashboard.clerk.com/'); }); - it('handles URL objects', () => { - const base = new URL('https://dashboard.clerk.com'); - const url = new URL('https://dashboard.clerk.com/applications/appid/instances/'); - const res = appendUrlsAsQueryParams(base, { redirect_url: url }); - expect(res).toBe('https://dashboard.clerk.com/#/?redirect_url=%2Fapplications%2Fappid%2Finstances%2F'); - }); - it('handles plain strings', () => { const base = 'https://dashboard.clerk.com'; const url = 'https://dashboard.clerk.com/applications/appid/instances/'; - const res = appendUrlsAsQueryParams(base, { redirect_url: url }); - expect(res).toBe('https://dashboard.clerk.com/#/?redirect_url=%2Fapplications%2Fappid%2Finstances%2F'); + const res = appendAsQueryParams(base, { redirect_url: url }); + expect(res).toBe( + 'https://dashboard.clerk.com/#/?redirect_url=https%3A%2F%2Fdashboard.clerk.com%2Fapplications%2Fappid%2Finstances%2F', + ); }); it('handles multiple params', () => { const base = 'https://dashboard.clerk.com'; const url = 'https://dashboard.clerk.com/applications/appid/instances/'; - const res = appendUrlsAsQueryParams(base, { - redirect_url: url, - after_sign_in_url: url, - }); + const res = appendAsQueryParams(base, { redirect_url: url, after_sign_in_url: url }); expect(res).toBe( - 'https://dashboard.clerk.com/#/?redirect_url=%2Fapplications%2Fappid%2Finstances%2F&after_sign_in_url=%2Fapplications%2Fappid%2Finstances%2F', + 'https://dashboard.clerk.com/#/?redirect_url=https%3A%2F%2Fdashboard.clerk.com%2Fapplications%2Fappid%2Finstances%2F&after_sign_in_url=https%3A%2F%2Fdashboard.clerk.com%2Fapplications%2Fappid%2Finstances%2F', ); }); it('skips falsy values', () => { const base = new URL('https://dashboard.clerk.com'); - const res = appendUrlsAsQueryParams(base, { redirect_url: undefined }); + const res = appendAsQueryParams(base, { redirect_url: undefined }); expect(res).toBe('https://dashboard.clerk.com/'); }); it('converts relative to absolute urls', () => { const base = new URL('https://dashboard.clerk.com'); - const res = appendUrlsAsQueryParams(base, { redirect_url: '/test' }); + const res = appendAsQueryParams(base, { redirect_url: 'http://localhost/test' }); expect(res).toBe('https://dashboard.clerk.com/#/?redirect_url=http%3A%2F%2Flocalhost%2Ftest'); }); it('converts keys from camel to snake case', () => { const base = new URL('https://dashboard.clerk.com'); - const res = appendUrlsAsQueryParams(base, { redirectUrl: '/test' }); + const res = appendAsQueryParams(base, { redirectUrl: 'http://localhost/test' }); expect(res).toBe('https://dashboard.clerk.com/#/?redirect_url=http%3A%2F%2Flocalhost%2Ftest'); }); it('keeps origin before appending if base and url have different origin', () => { const base = new URL('https://dashboard.clerk.com'); - const url = new URL('https://www.google.com/something'); - const res = appendUrlsAsQueryParams(base, { redirect_url: url }); + const url = new URL('https://www.google.com/something').href; + const res = appendAsQueryParams(base, { redirect_url: url }); expect(res).toBe('https://dashboard.clerk.com/#/?redirect_url=https%3A%2F%2Fwww.google.com%2Fsomething'); }); }); diff --git a/packages/clerk-js/src/utils/url.ts b/packages/clerk-js/src/utils/url.ts index a4e08b0c09b..2475b288d5e 100644 --- a/packages/clerk-js/src/utils/url.ts +++ b/packages/clerk-js/src/utils/url.ts @@ -210,25 +210,9 @@ export const trimTrailingSlash = (path: string): string => { return (path || '').replace(/\/+$/, ''); }; -export const appendUrlsAsQueryParams = ( - baseUrl: string | URL, - urls: Record = {}, -): string => { - const base = toURL(baseUrl); - const params = new URLSearchParams(); - for (const [key, val] of Object.entries(urls)) { - if (!val) { - continue; - } - const url = toURL(val); - const sameOrigin = base.origin === url.origin; - params.append(camelToSnake(key), sameOrigin ? stripOrigin(url) : `${url}`); - } - - // The following line will prepend the hash with a `/`. - // This is required for ClerkJS Components Hash router to work as expected - // as it treats the hash as sub-path with its nested querystring parameters. - return `${base}${params.toString() ? '#/?' + params.toString() : ''}`; +export const stripSameOrigin = (url: URL, baseUrl: URL): string => { + const sameOrigin = baseUrl.origin === url.origin; + return sameOrigin ? stripOrigin(url) : `${url}`; }; export const appendAsQueryParams = ( @@ -236,14 +220,18 @@ export const appendAsQueryParams = ( values: Record = {}, ): string => { const base = toURL(baseUrl); + const params = new URLSearchParams(); for (const [key, val] of Object.entries(values)) { if (!val) { continue; } - base.searchParams.append(camelToSnake(key), val); + params.append(camelToSnake(key), val); } - return baseUrl.toString() + base.search; + // The following line will prepend the hash with a `/`. + // This is required for ClerkJS Components Hash router to work as expected + // as it treats the hash as sub-path with its nested querystring parameters. + return `${base}${params.toString() ? '#/?' + params.toString() : ''}`; }; export const hasExternalAccountSignUpError = (signUp: SignUpResource): boolean => {