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
clerk · Oct 17, 2023 · 21f61ce · 21f61ce
1 parent 9ca2157
commit 21f61ce
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 52 deletions.
diff --git a/.changeset/wise-spiders-hammer.md 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
@@ -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<keyof typeof urls>).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<MountComponentRenderer> {

diff --git 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
@@ -210,40 +210,28 @@ export const trimTrailingSlash = (path: string): string => {
   return (path || '').replace(/\/+$/, '');
 };
 
-export const appendUrlsAsQueryParams = (
-  baseUrl: string | URL,
-  urls: Record<string, string | URL | null | undefined> = {},
-): 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 = (
   baseUrl: string | URL,
   values: Record<string, string | null | undefined> = {},
 ): 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 => {