Skip to content

Commit

Permalink
fix(clerk-js): Append initial values query params after hash
Browse files Browse the repository at this point in the history
  • Loading branch information
desiprisg committed Oct 10, 2023
1 parent c9b17f5 commit 92809b0
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 31 deletions.
7 changes: 2 additions & 5 deletions packages/clerk-js/src/core/clerk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ import type { MountComponentRenderer } from '../ui/Components';
import { completeSignUpFlow } from '../ui/components/SignUp/util';
import {
appendAsQueryParams,
appendUrlsAsQueryParams,
buildURL,
createBeforeUnloadTracker,
createCookieHandler,
Expand Down Expand Up @@ -1544,7 +1543,7 @@ export default class Clerk implements ClerkInterface {
return '';
}

const opts: RedirectOptions = {
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,
Expand All @@ -1556,9 +1555,7 @@ export default class Clerk implements ClerkInterface {
false,
);

return this.buildUrlWithAuth(
appendUrlsAsQueryParams(appendAsQueryParams(signInOrUpUrl, options?.initialValues || {}), opts),
);
return this.buildUrlWithAuth(appendAsQueryParams(signInOrUpUrl, { values: options?.initialValues, urls }));
};

assertComponentsReady(controls: unknown): asserts controls is ReturnType<MountComponentRenderer> {
Expand Down
21 changes: 10 additions & 11 deletions packages/clerk-js/src/utils/__tests__/url.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { SignUpResource } from '@clerk/types';

import {
appendUrlsAsQueryParams,
appendAsQueryParams,
buildURL,
getAllETLDs,
getETLDPlusOneFromFrontendApi,
Expand Down Expand Up @@ -242,30 +242,29 @@ 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 });
const res = appendAsQueryParams(base, { urls: { 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 });
const res = appendAsQueryParams(base, { urls: { redirect_url: url } });
expect(res).toBe('https://dashboard.clerk.com/#/?redirect_url=%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, {
urls: { 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',
Expand All @@ -274,26 +273,26 @@ describe('appendQueryParams(base,url)', () => {

it('skips falsy values', () => {
const base = new URL('https://dashboard.clerk.com');
const res = appendUrlsAsQueryParams(base, { redirect_url: undefined });
const res = appendAsQueryParams(base, { urls: { 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, { urls: { redirect_url: '/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, { urls: { redirectUrl: '/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 res = appendAsQueryParams(base, { urls: { redirect_url: url } });
expect(res).toBe('https://dashboard.clerk.com/#/?redirect_url=https%3A%2F%2Fwww.google.com%2Fsomething');
});
});
Expand Down
26 changes: 11 additions & 15 deletions packages/clerk-js/src/utils/url.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,10 +210,14 @@ export const trimTrailingSlash = (path: string): string => {
return (path || '').replace(/\/+$/, '');
};

export const appendUrlsAsQueryParams = (
export const appendAsQueryParams = (
baseUrl: string | URL,
urls: Record<string, string | URL | null | undefined> = {},
options?: {
values?: Record<string, string | null | undefined>;
urls?: Record<string, string | URL | null | undefined>;
},
): string => {
const { values = {}, urls = {} } = options || {};
const base = toURL(baseUrl);
const params = new URLSearchParams();
for (const [key, val] of Object.entries(urls)) {
Expand All @@ -225,25 +229,17 @@ export const appendUrlsAsQueryParams = (
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 appendAsQueryParams = (
baseUrl: string | URL,
values: Record<string, string | null | undefined> = {},
): string => {
const base = toURL(baseUrl);
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 => {
Expand Down

0 comments on commit 92809b0

Please sign in to comment.