From 849499de189ac2d326d770f2bef7c9c118a7b03f Mon Sep 17 00:00:00 2001 From: Emmanouela Date: Thu, 23 May 2024 13:33:45 +0300 Subject: [PATCH 01/12] chore(clerk-js): Remove qs library --- packages/clerk-js/package.json | 1 - packages/clerk-js/src/core/fapiClient.ts | 36 ++++++++++++++++--- .../clerk-js/src/ui/router/BaseRouter.tsx | 5 +-- packages/clerk-js/src/ui/router/index.tsx | 3 ++ .../src/utils/__tests__/querystring.test.ts | 14 ++++---- packages/clerk-js/src/utils/querystring.ts | 19 +++++----- 6 files changed, 55 insertions(+), 23 deletions(-) diff --git a/packages/clerk-js/package.json b/packages/clerk-js/package.json index d9904315279..49ec357be01 100644 --- a/packages/clerk-js/package.json +++ b/packages/clerk-js/package.json @@ -64,7 +64,6 @@ "core-js": "3.26.1", "dequal": "2.0.3", "qrcode.react": "3.1.0", - "qs": "6.11.0", "regenerator-runtime": "0.13.11" }, "devDependencies": { diff --git a/packages/clerk-js/src/core/fapiClient.ts b/packages/clerk-js/src/core/fapiClient.ts index aaa1705ae66..54cc3922930 100644 --- a/packages/clerk-js/src/core/fapiClient.ts +++ b/packages/clerk-js/src/core/fapiClient.ts @@ -1,6 +1,5 @@ import { camelToSnake, isBrowserOnline, runWithExponentialBackOff } from '@clerk/shared'; import type { Clerk, ClerkAPIErrorJSON, ClientJSON } from '@clerk/types'; -import qs from 'qs'; import { buildEmailAddress as buildEmailAddressUtil, buildURL as buildUrlUtil } from '../utils'; import { clerkNetworkError } from './errors'; @@ -32,8 +31,21 @@ export type FapiRequestCallback = ( response?: FapiResponse, ) => Promise | unknown | false; -const camelToSnakeEncoder: qs.IStringifyOptions['encoder'] = (str, defaultEncoder, _, type) => { - return type === 'key' ? camelToSnake(str) : defaultEncoder(str); +const customDefaultEncoder = (value: string) => { + return encodeURIComponent(value).replace(/[!'()*]/g, c => '%' + c.charCodeAt(0).toString(16).toUpperCase()); +}; + +const camelToSnakeEncoder = (str: string, type: 'key' | 'value') => { + return type === 'key' ? camelToSnake(str) : customDefaultEncoder(str); +}; +const stringifyBody = obj => { + return Object.keys(obj) + .map(key => { + const encodedKey = camelToSnakeEncoder(key, 'key'); + const encodedValue = obj[key] && camelToSnakeEncoder(obj[key], 'value'); + return encodedValue && `${encodedKey}=${encodedValue}`; + }) + .join('&'); }; // TODO: Move to @clerk/types @@ -125,7 +137,18 @@ export function createFapiClient(clerkInstance: Clerk): FapiClient { return acc; }, {} as FapiQueryStringParameters & Record); - return qs.stringify(objParams, { addQueryPrefix: true, arrayFormat: 'repeat' }); + const queryParams = new URLSearchParams(); + + objParams && + Object.keys(objParams).forEach(key => { + const value = objParams[key]; + if (Array.isArray(value)) { + value.forEach(item => queryParams.append(key, item)); + } else { + queryParams.append(key, value); + } + }); + return queryParams.toString(); } function buildUrl(requestInit: FapiRequestInit): URL { @@ -193,8 +216,11 @@ export function createFapiClient(clerkInstance: Clerk): FapiClient { // Currently, this is needed only for form-urlencoded, so that the values reach the server in the form // foo=bar&baz=bar&whatever=1 // @ts-ignore + if (requestInit.headers.get('content-type') === 'application/x-www-form-urlencoded') { - requestInit.body = qs.stringify(body, { encoder: camelToSnakeEncoder, indices: false }); + if (body && typeof body === 'object') { + requestInit.body = stringifyBody(body); + } } const beforeRequestCallbacksResult = await runBeforeRequestCallbacks(requestInit); diff --git a/packages/clerk-js/src/ui/router/BaseRouter.tsx b/packages/clerk-js/src/ui/router/BaseRouter.tsx index aad77cf2964..f79ddf0737f 100644 --- a/packages/clerk-js/src/ui/router/BaseRouter.tsx +++ b/packages/clerk-js/src/ui/router/BaseRouter.tsx @@ -1,6 +1,5 @@ import { useClerk } from '@clerk/shared/react'; import type { NavigateOptions } from '@clerk/types'; -import qs from 'qs'; import React from 'react'; import { getQueryParams, trimTrailingSlash } from '../../utils'; @@ -113,7 +112,9 @@ export const BaseRouter = ({ toQueryParams[param] = currentQueryParams[param]; } }); - toURL.search = qs.stringify(toQueryParams); + + const queryParams = new URLSearchParams(toQueryParams); + toURL.search = `?${queryParams.toString()}`; } const internalNavRes = await internalNavigate(toURL, { metadata: { navigationType: 'internal' } }); setRouteParts({ path: toURL.pathname, queryString: toURL.search }); diff --git a/packages/clerk-js/src/ui/router/index.tsx b/packages/clerk-js/src/ui/router/index.tsx index aab3a4c0b55..ef8d990d9f1 100644 --- a/packages/clerk-js/src/ui/router/index.tsx +++ b/packages/clerk-js/src/ui/router/index.tsx @@ -6,3 +6,6 @@ export * from './Route'; export * from './Switch'; export type { ParsedQs } from 'qs'; +export type ParsedQueryString = { + [key: string]: undefined | string | string[] | ParsedQueryString | ParsedQueryString[]; +}; diff --git a/packages/clerk-js/src/utils/__tests__/querystring.test.ts b/packages/clerk-js/src/utils/__tests__/querystring.test.ts index 1b29316dd11..5d3dbd093ea 100644 --- a/packages/clerk-js/src/utils/__tests__/querystring.test.ts +++ b/packages/clerk-js/src/utils/__tests__/querystring.test.ts @@ -1,4 +1,4 @@ -import { getQueryParams, stringifyQueryParams } from '../querystring'; +import { getQueryParams } from '../querystring'; describe('getQueryParams(string)', () => { it('parses a querystring', () => { @@ -8,9 +8,9 @@ describe('getQueryParams(string)', () => { }); }); -describe('stringifyQueryParams(object)', () => { - it('converts an object to querystring', () => { - expect(stringifyQueryParams({})).toEqual(''); - expect(stringifyQueryParams({ foo: '42', bar: '43' })).toBe('foo=42&bar=43'); - }); -}); +// describe('stringifyQueryParams(object)', () => { +// it('converts an object to querystring', () => { +// expect(stringifyQueryParams({})).toEqual(''); +// expect(stringifyQueryParams({ foo: '42', bar: '43' })).toBe('foo=42&bar=43'); +// }); +// }); diff --git a/packages/clerk-js/src/utils/querystring.ts b/packages/clerk-js/src/utils/querystring.ts index b5b6138a589..189465a96ce 100644 --- a/packages/clerk-js/src/utils/querystring.ts +++ b/packages/clerk-js/src/utils/querystring.ts @@ -1,11 +1,14 @@ -import qs from 'qs'; - export const getQueryParams = (queryString: string) => { - return qs.parse(queryString || '', { - ignoreQueryPrefix: true, - }) as Record; -}; + const queryParamsObject: { [key: string]: string } = {}; + const queryParams = new URLSearchParams(queryString); + queryParams?.forEach((value, key) => { + queryParamsObject[key] = value; + }); -export const stringifyQueryParams = (params: Record | Array) => { - return qs.stringify(params || {}); + return queryParamsObject as Record; }; + +// export const stringifyQueryParams = (params: Record | Array) => { +// // const queryParams = new URLSearchParams(toQueryParams); +// return qs.stringify(params || {}); +// }; From 02e100e657da32f52f28189fcf4f74bb31eada14 Mon Sep 17 00:00:00 2001 From: Emmanouela Date: Mon, 27 May 2024 18:18:09 +0300 Subject: [PATCH 02/12] chore(clerk-js): Create stringifyQueryParams function --- packages/clerk-js/src/core/fapiClient.ts | 36 ++----------------- .../clerk-js/src/ui/router/BaseRouter.tsx | 5 ++- packages/clerk-js/src/utils/querystring.ts | 20 ++++++++--- 3 files changed, 21 insertions(+), 40 deletions(-) diff --git a/packages/clerk-js/src/core/fapiClient.ts b/packages/clerk-js/src/core/fapiClient.ts index 54cc3922930..f6ec53a86e4 100644 --- a/packages/clerk-js/src/core/fapiClient.ts +++ b/packages/clerk-js/src/core/fapiClient.ts @@ -1,7 +1,7 @@ import { camelToSnake, isBrowserOnline, runWithExponentialBackOff } from '@clerk/shared'; import type { Clerk, ClerkAPIErrorJSON, ClientJSON } from '@clerk/types'; -import { buildEmailAddress as buildEmailAddressUtil, buildURL as buildUrlUtil } from '../utils'; +import { buildEmailAddress as buildEmailAddressUtil, buildURL as buildUrlUtil, stringifyQueryParams } from '../utils'; import { clerkNetworkError } from './errors'; export type HTTPMethod = 'CONNECT' | 'DELETE' | 'GET' | 'HEAD' | 'OPTIONS' | 'PATCH' | 'POST' | 'PUT' | 'TRACE'; @@ -31,23 +31,6 @@ export type FapiRequestCallback = ( response?: FapiResponse, ) => Promise | unknown | false; -const customDefaultEncoder = (value: string) => { - return encodeURIComponent(value).replace(/[!'()*]/g, c => '%' + c.charCodeAt(0).toString(16).toUpperCase()); -}; - -const camelToSnakeEncoder = (str: string, type: 'key' | 'value') => { - return type === 'key' ? camelToSnake(str) : customDefaultEncoder(str); -}; -const stringifyBody = obj => { - return Object.keys(obj) - .map(key => { - const encodedKey = camelToSnakeEncoder(key, 'key'); - const encodedValue = obj[key] && camelToSnakeEncoder(obj[key], 'value'); - return encodedValue && `${encodedKey}=${encodedValue}`; - }) - .join('&'); -}; - // TODO: Move to @clerk/types export interface FapiResponseJSON { response: T; @@ -137,18 +120,7 @@ export function createFapiClient(clerkInstance: Clerk): FapiClient { return acc; }, {} as FapiQueryStringParameters & Record); - const queryParams = new URLSearchParams(); - - objParams && - Object.keys(objParams).forEach(key => { - const value = objParams[key]; - if (Array.isArray(value)) { - value.forEach(item => queryParams.append(key, item)); - } else { - queryParams.append(key, value); - } - }); - return queryParams.toString(); + return stringifyQueryParams(objParams); } function buildUrl(requestInit: FapiRequestInit): URL { @@ -218,9 +190,7 @@ export function createFapiClient(clerkInstance: Clerk): FapiClient { // @ts-ignore if (requestInit.headers.get('content-type') === 'application/x-www-form-urlencoded') { - if (body && typeof body === 'object') { - requestInit.body = stringifyBody(body); - } + requestInit.body = stringifyQueryParams(body, camelToSnake); } const beforeRequestCallbacksResult = await runBeforeRequestCallbacks(requestInit); diff --git a/packages/clerk-js/src/ui/router/BaseRouter.tsx b/packages/clerk-js/src/ui/router/BaseRouter.tsx index f79ddf0737f..9cdafc3e581 100644 --- a/packages/clerk-js/src/ui/router/BaseRouter.tsx +++ b/packages/clerk-js/src/ui/router/BaseRouter.tsx @@ -2,7 +2,7 @@ import { useClerk } from '@clerk/shared/react'; import type { NavigateOptions } from '@clerk/types'; import React from 'react'; -import { getQueryParams, trimTrailingSlash } from '../../utils'; +import { getQueryParams, stringifyQueryParams, trimTrailingSlash } from '../../utils'; import { useWindowEventListener } from '../hooks'; import { newPaths } from './newPaths'; import { match } from './pathToRegexp'; @@ -113,8 +113,7 @@ export const BaseRouter = ({ } }); - const queryParams = new URLSearchParams(toQueryParams); - toURL.search = `?${queryParams.toString()}`; + toURL.search = stringifyQueryParams(toQueryParams); } const internalNavRes = await internalNavigate(toURL, { metadata: { navigationType: 'internal' } }); setRouteParts({ path: toURL.pathname, queryString: toURL.search }); diff --git a/packages/clerk-js/src/utils/querystring.ts b/packages/clerk-js/src/utils/querystring.ts index 189465a96ce..01cc28c7b1b 100644 --- a/packages/clerk-js/src/utils/querystring.ts +++ b/packages/clerk-js/src/utils/querystring.ts @@ -8,7 +8,19 @@ export const getQueryParams = (queryString: string) => { return queryParamsObject as Record; }; -// export const stringifyQueryParams = (params: Record | Array) => { -// // const queryParams = new URLSearchParams(toQueryParams); -// return qs.stringify(params || {}); -// }; +export const stringifyQueryParams = (params: Record | Array, encoder?) => { + const queryParams = new URLSearchParams(); + Object.keys(params).forEach(key => { + const encodedKey = encoder ? encoder(key) : key; + const value = params[key]; + if (Array.isArray(value)) { + value.forEach(item => queryParams.append(encodedKey, item)); + } else if (value === undefined) { + return; + } else { + queryParams.append(encodedKey, value); + } + }); + + return queryParams.toString(); +}; From ac0477cbc44301624973ebee9060424f40fc33b7 Mon Sep 17 00:00:00 2001 From: Emmanouela Pothitou <68468183+EmmanouelaPothitou@users.noreply.github.com> Date: Thu, 23 May 2024 16:16:09 +0300 Subject: [PATCH 03/12] Update packages/clerk-js/src/utils/querystring.ts Co-authored-by: Nikos Douvlis --- packages/clerk-js/src/utils/querystring.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/clerk-js/src/utils/querystring.ts b/packages/clerk-js/src/utils/querystring.ts index 01cc28c7b1b..f6f8915a284 100644 --- a/packages/clerk-js/src/utils/querystring.ts +++ b/packages/clerk-js/src/utils/querystring.ts @@ -1,7 +1,7 @@ export const getQueryParams = (queryString: string) => { const queryParamsObject: { [key: string]: string } = {}; const queryParams = new URLSearchParams(queryString); - queryParams?.forEach((value, key) => { + queryParams.forEach((value, key) => { queryParamsObject[key] = value; }); From 9b08b093ce54c747d1bf87b02e8b3e27f1b163e8 Mon Sep 17 00:00:00 2001 From: Emmanouela Date: Tue, 28 May 2024 20:48:34 +0300 Subject: [PATCH 04/12] chore(clerk-js): Update ParsedQueryString type and stringifyQueryParam function --- .../ui/contexts/ClerkUIComponentsContext.tsx | 8 ++--- packages/clerk-js/src/ui/router/index.tsx | 5 +-- .../src/utils/__tests__/querystring.test.ts | 16 ++++++---- packages/clerk-js/src/utils/querystring.ts | 32 ++++++++++--------- 4 files changed, 31 insertions(+), 30 deletions(-) diff --git a/packages/clerk-js/src/ui/contexts/ClerkUIComponentsContext.tsx b/packages/clerk-js/src/ui/contexts/ClerkUIComponentsContext.tsx index 2db870fb7ac..dd0fc2dde59 100644 --- a/packages/clerk-js/src/ui/contexts/ClerkUIComponentsContext.tsx +++ b/packages/clerk-js/src/ui/contexts/ClerkUIComponentsContext.tsx @@ -9,7 +9,7 @@ import { RedirectUrls } from '../../utils/redirectUrls'; import { ORGANIZATION_PROFILE_NAVBAR_ROUTE_ID } from '../constants'; import { useEnvironment, useOptions } from '../contexts'; import type { NavbarRoute } from '../elements'; -import type { ParsedQs } from '../router'; +import type { ParsedQueryString } from '../router'; import { useRouter } from '../router'; import type { AvailableComponentCtx, @@ -44,7 +44,7 @@ const getInitialValuesFromQueryParams = (queryString: string, params: string[]) export type SignUpContextType = SignUpCtx & { navigateAfterSignUp: () => any; - queryParams: ParsedQs; + queryParams: ParsedQueryString; signInUrl: string; signUpUrl: string; secondFactorUrl: string; @@ -115,7 +115,7 @@ export const useSignUpContext = (): SignUpContextType => { export type SignInContextType = SignInCtx & { navigateAfterSignIn: () => any; - queryParams: ParsedQs; + queryParams: ParsedQueryString; signUpUrl: string; signInUrl: string; signUpContinueUrl: string; @@ -203,7 +203,7 @@ type PagesType = { }; export type UserProfileContextType = UserProfileCtx & { - queryParams: ParsedQs; + queryParams: ParsedQueryString; authQueryString: string | null; pages: PagesType; }; diff --git a/packages/clerk-js/src/ui/router/index.tsx b/packages/clerk-js/src/ui/router/index.tsx index ef8d990d9f1..b6c89520467 100644 --- a/packages/clerk-js/src/ui/router/index.tsx +++ b/packages/clerk-js/src/ui/router/index.tsx @@ -5,7 +5,4 @@ export * from './VirtualRouter'; export * from './Route'; export * from './Switch'; -export type { ParsedQs } from 'qs'; -export type ParsedQueryString = { - [key: string]: undefined | string | string[] | ParsedQueryString | ParsedQueryString[]; -}; +export type ParsedQueryString = Record; diff --git a/packages/clerk-js/src/utils/__tests__/querystring.test.ts b/packages/clerk-js/src/utils/__tests__/querystring.test.ts index 5d3dbd093ea..c49805808c8 100644 --- a/packages/clerk-js/src/utils/__tests__/querystring.test.ts +++ b/packages/clerk-js/src/utils/__tests__/querystring.test.ts @@ -1,4 +1,4 @@ -import { getQueryParams } from '../querystring'; +import { getQueryParams, stringifyQueryParams } from '../querystring'; describe('getQueryParams(string)', () => { it('parses a querystring', () => { @@ -8,9 +8,11 @@ describe('getQueryParams(string)', () => { }); }); -// describe('stringifyQueryParams(object)', () => { -// it('converts an object to querystring', () => { -// expect(stringifyQueryParams({})).toEqual(''); -// expect(stringifyQueryParams({ foo: '42', bar: '43' })).toBe('foo=42&bar=43'); -// }); -// }); +describe('stringifyQueryParams(object)', () => { + it('converts an object to querystring', () => { + expect(stringifyQueryParams({})).toEqual(''); + expect(stringifyQueryParams({ foo: '42', bar: '43' })).toBe('foo=42&bar=43'); + expect(stringifyQueryParams({ foo: ['42', '22'], bar: '43' })).toBe('foo=42&foo=22&bar=43'); + expect(stringifyQueryParams({ foo: ['42', '22'], bar: undefined })).toBe('foo=42&foo=22'); + }); +}); diff --git a/packages/clerk-js/src/utils/querystring.ts b/packages/clerk-js/src/utils/querystring.ts index f6f8915a284..7b59c625979 100644 --- a/packages/clerk-js/src/utils/querystring.ts +++ b/packages/clerk-js/src/utils/querystring.ts @@ -1,26 +1,28 @@ -export const getQueryParams = (queryString: string) => { - const queryParamsObject: { [key: string]: string } = {}; +export const getQueryParams = (queryString: string): URLSearchParams => { + const queryParamsObject = new URLSearchParams(); const queryParams = new URLSearchParams(queryString); queryParams.forEach((value, key) => { - queryParamsObject[key] = value; + queryParamsObject.set(key, value); }); - return queryParamsObject as Record; + return queryParamsObject; }; export const stringifyQueryParams = (params: Record | Array, encoder?) => { const queryParams = new URLSearchParams(); - Object.keys(params).forEach(key => { - const encodedKey = encoder ? encoder(key) : key; - const value = params[key]; - if (Array.isArray(value)) { - value.forEach(item => queryParams.append(encodedKey, item)); - } else if (value === undefined) { - return; - } else { - queryParams.append(encodedKey, value); - } - }); + if (params && typeof params === 'object') { + Object.keys(params).forEach(key => { + const encodedKey = encoder ? encoder(key) : key; + const value = params[key]; + if (Array.isArray(value)) { + value.forEach(item => queryParams.append(encodedKey, item)); + } else if (value === undefined) { + return; + } else { + queryParams.append(encodedKey, value); + } + }); + } return queryParams.toString(); }; From d812bc9678e107a468f05a59cd6ccbde17123aca Mon Sep 17 00:00:00 2001 From: Emmanouela Date: Wed, 29 May 2024 16:47:38 +0300 Subject: [PATCH 05/12] chore(clerk-js): Update querystring.ts file and add more tests --- .../src/utils/__tests__/querystring.test.ts | 47 +++++++++++++++++-- packages/clerk-js/src/utils/querystring.ts | 35 ++++++++++---- 2 files changed, 69 insertions(+), 13 deletions(-) diff --git a/packages/clerk-js/src/utils/__tests__/querystring.test.ts b/packages/clerk-js/src/utils/__tests__/querystring.test.ts index c49805808c8..3d096f66899 100644 --- a/packages/clerk-js/src/utils/__tests__/querystring.test.ts +++ b/packages/clerk-js/src/utils/__tests__/querystring.test.ts @@ -1,18 +1,55 @@ +import { camelToSnake } from '@clerk/shared'; + import { getQueryParams, stringifyQueryParams } from '../querystring'; describe('getQueryParams(string)', () => { - it('parses a querystring', () => { - expect(getQueryParams('')).toEqual({}); - expect(getQueryParams('foo=42&bar=43')).toEqual({ foo: '42', bar: '43' }); - expect(getQueryParams('?foo=42&bar=43')).toEqual({ foo: '42', bar: '43' }); + it('parses an emtpy querystring', () => { + const res = getQueryParams(''); + expect(res).toEqual({}); + }); + + it('parses a querystring into a URLSearchParams instance', () => { + const res = getQueryParams('foo=42&bar=43'); + expect(res).toEqual({ foo: '42', bar: '43' }); + }); + + it('parses a querystring into a URLSearchParams instance even when prefixed with ?', () => { + const res = getQueryParams('?foo=42&bar=43'); + expect(res).toEqual({ foo: '42', bar: '43' }); + }); + + it('parses multiple occurances of the same key as an array', () => { + const res = getQueryParams('?foo=42&foo=43&bar=1'); + expect(res).toEqual({ foo: ['42', '43'], bar: '1' }); }); }); describe('stringifyQueryParams(object)', () => { + // it('converts an object to querystring', () => { + // expect(stringifyQueryParams({})).toEqual(''); + // expect(stringifyQueryParams({ foo: '42', bar: '43' })).toBe('foo=42&bar=43'); + // expect(stringifyQueryParams({ foo: ['42', '22'], bar: '43' })).toBe('foo=42&foo=22&bar=43'); + // expect(stringifyQueryParams({ foo: ['42', '22'], bar: undefined })).toBe('foo=42&foo=22'); + // expect(stringifyQueryParams({ unsafeMetadata: { bar: '1' } })).toBe('unsafe_metadata=%7B%22bar%22%3A%221%22%7D'); + // }); + it('converts an object to querystring', () => { - expect(stringifyQueryParams({})).toEqual(''); expect(stringifyQueryParams({ foo: '42', bar: '43' })).toBe('foo=42&bar=43'); + }); + it('converts an object to querystring when value is an array', () => { expect(stringifyQueryParams({ foo: ['42', '22'], bar: '43' })).toBe('foo=42&foo=22&bar=43'); + }); + it('converts an object to querystring when value is undefined', () => { expect(stringifyQueryParams({ foo: ['42', '22'], bar: undefined })).toBe('foo=42&foo=22'); }); + it('converts an object to querystring when value is an object', () => { + expect(stringifyQueryParams({ unsafe_metadata: { bar: '1' } })).toBe('unsafe_metadata=%7B%22bar%22%3A%221%22%7D'); + }); + + it('converts an object to querystring when key is camelCase', () => { + expect(stringifyQueryParams({ barFoo: '1' }, { encoder: camelToSnake })).toBe('bar_foo=1'); + expect(stringifyQueryParams({ unsafeMetadata: { bar: '1' } }, { encoder: camelToSnake })).toBe( + 'unsafe_metadata=%7B%22bar%22%3A%221%22%7D', + ); + }); }); diff --git a/packages/clerk-js/src/utils/querystring.ts b/packages/clerk-js/src/utils/querystring.ts index 7b59c625979..470176fd4f3 100644 --- a/packages/clerk-js/src/utils/querystring.ts +++ b/packages/clerk-js/src/utils/querystring.ts @@ -1,25 +1,44 @@ -export const getQueryParams = (queryString: string): URLSearchParams => { - const queryParamsObject = new URLSearchParams(); +export const getQueryParams = (queryString: string) => { + const queryParamsObject: { [key: string]: string | string[] } = {}; const queryParams = new URLSearchParams(queryString); queryParams.forEach((value, key) => { - queryParamsObject.set(key, value); + if (key in queryParamsObject) { + // If the key already exists, we need to handle it as an array + const existingValue = queryParamsObject[key]; + if (Array.isArray(existingValue)) { + existingValue.push(value); + } else { + queryParamsObject[key] = [existingValue, value]; + } + } else { + queryParamsObject[key] = value; + } }); + return queryParamsObject as Record; +}; - return queryParamsObject; +type StringifyQueryParamsOptions = { + encoder?: (key: string) => string; }; -export const stringifyQueryParams = (params: Record | Array, encoder?) => { +export const stringifyQueryParams = ( + params: Record>, + opts: StringifyQueryParamsOptions = {}, +) => { const queryParams = new URLSearchParams(); + if (params && typeof params === 'object') { Object.keys(params).forEach(key => { - const encodedKey = encoder ? encoder(key) : key; + const encodedKey = opts.encoder ? opts.encoder(key) : key; const value = params[key]; if (Array.isArray(value)) { - value.forEach(item => queryParams.append(encodedKey, item)); + value.forEach(v => v !== undefined && queryParams.append(encodedKey, v || '')); } else if (value === undefined) { return; + } else if (typeof value === 'object' && value !== null) { + queryParams.append(encodedKey, JSON.stringify(value)); } else { - queryParams.append(encodedKey, value); + queryParams.append(encodedKey, value || ''); } }); } From 8d179482d98ad49fd53385ca911b2b1401ec1776 Mon Sep 17 00:00:00 2001 From: Emmanouela Pothitou <68468183+EmmanouelaPothitou@users.noreply.github.com> Date: Wed, 5 Jun 2024 13:00:30 +0300 Subject: [PATCH 06/12] Create silent-countries-jump.md --- .changeset/silent-countries-jump.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/silent-countries-jump.md diff --git a/.changeset/silent-countries-jump.md b/.changeset/silent-countries-jump.md new file mode 100644 index 00000000000..968bbd0998c --- /dev/null +++ b/.changeset/silent-countries-jump.md @@ -0,0 +1,5 @@ +--- +"@clerk/clerk-js": patch +--- + +Remove the qs library and use the native URLSearchParams API instead. From 8a070fff8fefaba6bcee55d6a8fb2002149f5737 Mon Sep 17 00:00:00 2001 From: Emmanouela Pothitou <68468183+EmmanouelaPothitou@users.noreply.github.com> Date: Wed, 5 Jun 2024 13:41:02 +0300 Subject: [PATCH 07/12] Update packages/clerk-js/src/core/fapiClient.ts Co-authored-by: Nikos Douvlis --- packages/clerk-js/src/core/fapiClient.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/clerk-js/src/core/fapiClient.ts b/packages/clerk-js/src/core/fapiClient.ts index f6ec53a86e4..2f980857000 100644 --- a/packages/clerk-js/src/core/fapiClient.ts +++ b/packages/clerk-js/src/core/fapiClient.ts @@ -190,7 +190,7 @@ export function createFapiClient(clerkInstance: Clerk): FapiClient { // @ts-ignore if (requestInit.headers.get('content-type') === 'application/x-www-form-urlencoded') { - requestInit.body = stringifyQueryParams(body, camelToSnake); + requestInit.body = stringifyQueryParams(body, { keyEncoder: camelToSnake }); } const beforeRequestCallbacksResult = await runBeforeRequestCallbacks(requestInit); From f47f9aef23e11b3015c0bf71e6da36ec75917e63 Mon Sep 17 00:00:00 2001 From: Emmanouela Date: Wed, 5 Jun 2024 18:10:49 +0300 Subject: [PATCH 08/12] chore(clerk-js): Rename encoder to keyEncoder --- .../clerk-js/src/utils/__tests__/querystring.test.ts | 12 ++---------- packages/clerk-js/src/utils/querystring.ts | 4 ++-- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/packages/clerk-js/src/utils/__tests__/querystring.test.ts b/packages/clerk-js/src/utils/__tests__/querystring.test.ts index 3d096f66899..822539ba6b9 100644 --- a/packages/clerk-js/src/utils/__tests__/querystring.test.ts +++ b/packages/clerk-js/src/utils/__tests__/querystring.test.ts @@ -25,14 +25,6 @@ describe('getQueryParams(string)', () => { }); describe('stringifyQueryParams(object)', () => { - // it('converts an object to querystring', () => { - // expect(stringifyQueryParams({})).toEqual(''); - // expect(stringifyQueryParams({ foo: '42', bar: '43' })).toBe('foo=42&bar=43'); - // expect(stringifyQueryParams({ foo: ['42', '22'], bar: '43' })).toBe('foo=42&foo=22&bar=43'); - // expect(stringifyQueryParams({ foo: ['42', '22'], bar: undefined })).toBe('foo=42&foo=22'); - // expect(stringifyQueryParams({ unsafeMetadata: { bar: '1' } })).toBe('unsafe_metadata=%7B%22bar%22%3A%221%22%7D'); - // }); - it('converts an object to querystring', () => { expect(stringifyQueryParams({ foo: '42', bar: '43' })).toBe('foo=42&bar=43'); }); @@ -47,8 +39,8 @@ describe('stringifyQueryParams(object)', () => { }); it('converts an object to querystring when key is camelCase', () => { - expect(stringifyQueryParams({ barFoo: '1' }, { encoder: camelToSnake })).toBe('bar_foo=1'); - expect(stringifyQueryParams({ unsafeMetadata: { bar: '1' } }, { encoder: camelToSnake })).toBe( + expect(stringifyQueryParams({ barFoo: '1' }, { keyEncoder: camelToSnake })).toBe('bar_foo=1'); + expect(stringifyQueryParams({ unsafeMetadata: { bar: '1' } }, { keyEncoder: camelToSnake })).toBe( 'unsafe_metadata=%7B%22bar%22%3A%221%22%7D', ); }); diff --git a/packages/clerk-js/src/utils/querystring.ts b/packages/clerk-js/src/utils/querystring.ts index 470176fd4f3..db816e15ed3 100644 --- a/packages/clerk-js/src/utils/querystring.ts +++ b/packages/clerk-js/src/utils/querystring.ts @@ -18,7 +18,7 @@ export const getQueryParams = (queryString: string) => { }; type StringifyQueryParamsOptions = { - encoder?: (key: string) => string; + keyEncoder?: (key: string) => string; }; export const stringifyQueryParams = ( @@ -29,7 +29,7 @@ export const stringifyQueryParams = ( if (params && typeof params === 'object') { Object.keys(params).forEach(key => { - const encodedKey = opts.encoder ? opts.encoder(key) : key; + const encodedKey = opts.keyEncoder ? opts.keyEncoder(key) : key; const value = params[key]; if (Array.isArray(value)) { value.forEach(v => v !== undefined && queryParams.append(encodedKey, v || '')); From bfc7429b6b98cb655f0b8f923b9d9c43bf665af3 Mon Sep 17 00:00:00 2001 From: Nikos Douvlis Date: Wed, 5 Jun 2024 18:47:49 +0300 Subject: [PATCH 09/12] Resolve PR comments --- packages/clerk-js/package.json | 1 - packages/clerk-js/src/core/clerk.ts | 1 + packages/clerk-js/src/core/fapiClient.ts | 7 ++++++- .../src/utils/__tests__/querystring.test.ts | 19 +++++++++++++++++++ packages/clerk-js/src/utils/querystring.ts | 11 +++++++++-- 5 files changed, 35 insertions(+), 4 deletions(-) diff --git a/packages/clerk-js/package.json b/packages/clerk-js/package.json index 49ec357be01..a3b0b76a29b 100644 --- a/packages/clerk-js/package.json +++ b/packages/clerk-js/package.json @@ -76,7 +76,6 @@ "@clerk/eslint-config-custom": "*", "@pmmmwh/react-refresh-webpack-plugin": "^0.5.10", "@svgr/webpack": "^6.2.1", - "@types/qs": "^6.9.3", "@types/react": "*", "@types/react-dom": "*", "@types/webpack-dev-server": "^4.7.2", diff --git a/packages/clerk-js/src/core/clerk.ts b/packages/clerk-js/src/core/clerk.ts index d9607721469..09c7af63fc5 100644 --- a/packages/clerk-js/src/core/clerk.ts +++ b/packages/clerk-js/src/core/clerk.ts @@ -265,6 +265,7 @@ export class Clerk implements ClerkInterface { public getFapiClient = (): FapiClient => this.#fapiClient; public load = async (options?: ClerkOptions): Promise => { + console.log('localllllllllllllllllllllllllllllllllll'); if (this.loaded) { return; } diff --git a/packages/clerk-js/src/core/fapiClient.ts b/packages/clerk-js/src/core/fapiClient.ts index 2f980857000..deca8fe5b78 100644 --- a/packages/clerk-js/src/core/fapiClient.ts +++ b/packages/clerk-js/src/core/fapiClient.ts @@ -190,7 +190,12 @@ export function createFapiClient(clerkInstance: Clerk): FapiClient { // @ts-ignore if (requestInit.headers.get('content-type') === 'application/x-www-form-urlencoded') { - requestInit.body = stringifyQueryParams(body, { keyEncoder: camelToSnake }); + // The native BodyInit type is too wide for our use case, + // so we're casting it to a more specific type here. + // This is covered by the test suite. + requestInit.body = body + ? stringifyQueryParams(body as any as Record, { keyEncoder: camelToSnake }) + : body; } const beforeRequestCallbacksResult = await runBeforeRequestCallbacks(requestInit); diff --git a/packages/clerk-js/src/utils/__tests__/querystring.test.ts b/packages/clerk-js/src/utils/__tests__/querystring.test.ts index 822539ba6b9..57032b4d748 100644 --- a/packages/clerk-js/src/utils/__tests__/querystring.test.ts +++ b/packages/clerk-js/src/utils/__tests__/querystring.test.ts @@ -25,15 +25,34 @@ describe('getQueryParams(string)', () => { }); describe('stringifyQueryParams(object)', () => { + it('handles null values', () => { + expect(stringifyQueryParams(null)).toBe(''); + }); + + it('handles undefined values', () => { + expect(stringifyQueryParams(undefined)).toBe(''); + }); + + it('handles string values', () => { + expect(stringifyQueryParams('hello')).toBe(''); + }); + + it('handles empty string values', () => { + expect(stringifyQueryParams('')).toBe(''); + }); + it('converts an object to querystring', () => { expect(stringifyQueryParams({ foo: '42', bar: '43' })).toBe('foo=42&bar=43'); }); + it('converts an object to querystring when value is an array', () => { expect(stringifyQueryParams({ foo: ['42', '22'], bar: '43' })).toBe('foo=42&foo=22&bar=43'); }); + it('converts an object to querystring when value is undefined', () => { expect(stringifyQueryParams({ foo: ['42', '22'], bar: undefined })).toBe('foo=42&foo=22'); }); + it('converts an object to querystring when value is an object', () => { expect(stringifyQueryParams({ unsafe_metadata: { bar: '1' } })).toBe('unsafe_metadata=%7B%22bar%22%3A%221%22%7D'); }); diff --git a/packages/clerk-js/src/utils/querystring.ts b/packages/clerk-js/src/utils/querystring.ts index db816e15ed3..61976876f7d 100644 --- a/packages/clerk-js/src/utils/querystring.ts +++ b/packages/clerk-js/src/utils/querystring.ts @@ -22,11 +22,18 @@ type StringifyQueryParamsOptions = { }; export const stringifyQueryParams = ( - params: Record>, + params: + | Record> + | null + | undefined + | string, opts: StringifyQueryParamsOptions = {}, ) => { - const queryParams = new URLSearchParams(); + if (params === null || params === undefined) { + return ''; + } + const queryParams = new URLSearchParams(); if (params && typeof params === 'object') { Object.keys(params).forEach(key => { const encodedKey = opts.keyEncoder ? opts.keyEncoder(key) : key; From 2fbf8db571877653c52f51009c8219d2d08411fb Mon Sep 17 00:00:00 2001 From: Nikos Douvlis Date: Wed, 5 Jun 2024 21:12:32 +0300 Subject: [PATCH 10/12] Update package-lock.json --- package-lock.json | 3 +-- packages/clerk-js/src/core/clerk.ts | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 22530356e42..46c7e270d33 100644 --- a/package-lock.json +++ b/package-lock.json @@ -32009,6 +32009,7 @@ }, "node_modules/qs": { "version": "6.11.0", + "dev": true, "license": "BSD-3-Clause", "dependencies": { "side-channel": "^1.0.4" @@ -39138,7 +39139,6 @@ "core-js": "3.26.1", "dequal": "2.0.3", "qrcode.react": "3.1.0", - "qs": "6.11.0", "regenerator-runtime": "0.13.11" }, "devDependencies": { @@ -39151,7 +39151,6 @@ "@clerk/eslint-config-custom": "*", "@pmmmwh/react-refresh-webpack-plugin": "^0.5.10", "@svgr/webpack": "^6.2.1", - "@types/qs": "^6.9.3", "@types/react": "*", "@types/react-dom": "*", "@types/webpack-dev-server": "^4.7.2", diff --git a/packages/clerk-js/src/core/clerk.ts b/packages/clerk-js/src/core/clerk.ts index 09c7af63fc5..d9607721469 100644 --- a/packages/clerk-js/src/core/clerk.ts +++ b/packages/clerk-js/src/core/clerk.ts @@ -265,7 +265,6 @@ export class Clerk implements ClerkInterface { public getFapiClient = (): FapiClient => this.#fapiClient; public load = async (options?: ClerkOptions): Promise => { - console.log('localllllllllllllllllllllllllllllllllll'); if (this.loaded) { return; } From cb77adfdd62b65e57d2d94d1ba252f1c7b3e8928 Mon Sep 17 00:00:00 2001 From: Nikos Douvlis Date: Thu, 6 Jun 2024 13:34:41 +0300 Subject: [PATCH 11/12] chore(repo): Update bundlewatch config --- packages/clerk-js/bundlewatch.config.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/clerk-js/bundlewatch.config.json b/packages/clerk-js/bundlewatch.config.json index cd07dc185fd..8aafacbbc3e 100644 --- a/packages/clerk-js/bundlewatch.config.json +++ b/packages/clerk-js/bundlewatch.config.json @@ -1,7 +1,7 @@ { "files": [ - { "path": "./dist/clerk.browser.js", "maxSize": "72kB" }, - { "path": "./dist/clerk.headless.js", "maxSize": "48kB" }, + { "path": "./dist/clerk.browser.js", "maxSize": "62kB" }, + { "path": "./dist/clerk.headless.js", "maxSize": "43kB" }, { "path": "./dist/ui-common*.js", "maxSize": "85KB" }, { "path": "./dist/vendors*.js", "maxSize": "70KB" }, { "path": "./dist/createorganization*.js", "maxSize": "5KB" }, From 11a5fccb6938e8b21b2f90f6a47e81b280243905 Mon Sep 17 00:00:00 2001 From: Emmanouela Date: Mon, 10 Jun 2024 13:23:50 +0300 Subject: [PATCH 12/12] chore(clerk-js): Add more tests on querystring and fapiClient --- .../src/core/__tests__/fapiClient.test.ts | 27 +++++++++++++++- .../src/utils/__tests__/querystring.test.ts | 9 ++++++ packages/clerk-js/src/utils/querystring.ts | 32 ++++++++++--------- 3 files changed, 52 insertions(+), 16 deletions(-) diff --git a/packages/clerk-js/src/core/__tests__/fapiClient.test.ts b/packages/clerk-js/src/core/__tests__/fapiClient.test.ts index 4a55a1f098a..50e80c89169 100644 --- a/packages/clerk-js/src/core/__tests__/fapiClient.test.ts +++ b/packages/clerk-js/src/core/__tests__/fapiClient.test.ts @@ -90,20 +90,45 @@ describe('buildUrl(options)', () => { ); }); - it('correctly parses search params', () => { + it('parses search params is an object with string values', () => { expect(fapiClient.buildUrl({ path: '/foo', search: { test: '1' } }).href).toBe( 'https://clerk.example.com/v1/foo?test=1&_clerk_js_version=42.0.0', ); + }); + it('parses string search params ', () => { expect(fapiClient.buildUrl({ path: '/foo', search: 'test=2' }).href).toBe( 'https://clerk.example.com/v1/foo?test=2&_clerk_js_version=42.0.0', ); + }); + + it('parses search params when value contains invalid url symbols', () => { + expect(fapiClient.buildUrl({ path: '/foo', search: { bar: 'test=2' } }).href).toBe( + 'https://clerk.example.com/v1/foo?bar=test%3D2&_clerk_js_version=42.0.0', + ); + }); + + it('parses search params when value is an array', () => { + expect( + fapiClient.buildUrl({ + path: '/foo', + search: { + array: ['item1', 'item2'], + }, + }).href, + ).toBe('https://clerk.example.com/v1/foo?array=item1&array=item2&_clerk_js_version=42.0.0'); + }); + // The return value isn't as expected. + // The buildUrl function converts an undefined value to the string 'undefined' + // and includes it in the search parameters. + it.skip('parses search params when value is undefined', () => { expect( fapiClient.buildUrl({ path: '/foo', search: { array: ['item1', 'item2'], + test: undefined, }, }).href, ).toBe('https://clerk.example.com/v1/foo?array=item1&array=item2&_clerk_js_version=42.0.0'); diff --git a/packages/clerk-js/src/utils/__tests__/querystring.test.ts b/packages/clerk-js/src/utils/__tests__/querystring.test.ts index 57032b4d748..52744ac772f 100644 --- a/packages/clerk-js/src/utils/__tests__/querystring.test.ts +++ b/packages/clerk-js/src/utils/__tests__/querystring.test.ts @@ -53,10 +53,18 @@ describe('stringifyQueryParams(object)', () => { expect(stringifyQueryParams({ foo: ['42', '22'], bar: undefined })).toBe('foo=42&foo=22'); }); + it('converts an object to querystring when value is null', () => { + expect(stringifyQueryParams({ foo: null })).toBe('foo='); + }); + it('converts an object to querystring when value is an object', () => { expect(stringifyQueryParams({ unsafe_metadata: { bar: '1' } })).toBe('unsafe_metadata=%7B%22bar%22%3A%221%22%7D'); }); + it('converts an object to querystring when value contains invalid url symbols', () => { + expect(stringifyQueryParams({ test: 'ena=duo' })).toBe('test=ena%3Dduo'); + }); + it('converts an object to querystring when key is camelCase', () => { expect(stringifyQueryParams({ barFoo: '1' }, { keyEncoder: camelToSnake })).toBe('bar_foo=1'); expect(stringifyQueryParams({ unsafeMetadata: { bar: '1' } }, { keyEncoder: camelToSnake })).toBe( @@ -64,3 +72,4 @@ describe('stringifyQueryParams(object)', () => { ); }); }); +//test=ena%3Dduo diff --git a/packages/clerk-js/src/utils/querystring.ts b/packages/clerk-js/src/utils/querystring.ts index 61976876f7d..c63533d36a6 100644 --- a/packages/clerk-js/src/utils/querystring.ts +++ b/packages/clerk-js/src/utils/querystring.ts @@ -32,23 +32,25 @@ export const stringifyQueryParams = ( if (params === null || params === undefined) { return ''; } + if (!params || typeof params !== 'object') { + return ''; + } const queryParams = new URLSearchParams(); - if (params && typeof params === 'object') { - Object.keys(params).forEach(key => { - const encodedKey = opts.keyEncoder ? opts.keyEncoder(key) : key; - const value = params[key]; - if (Array.isArray(value)) { - value.forEach(v => v !== undefined && queryParams.append(encodedKey, v || '')); - } else if (value === undefined) { - return; - } else if (typeof value === 'object' && value !== null) { - queryParams.append(encodedKey, JSON.stringify(value)); - } else { - queryParams.append(encodedKey, value || ''); - } - }); - } + + Object.keys(params).forEach(key => { + const encodedKey = opts.keyEncoder ? opts.keyEncoder(key) : key; + const value = params[key]; + if (Array.isArray(value)) { + value.forEach(v => v !== undefined && queryParams.append(encodedKey, v || '')); + } else if (value === undefined) { + return; + } else if (typeof value === 'object' && value !== null) { + queryParams.append(encodedKey, JSON.stringify(value)); + } else { + queryParams.append(encodedKey, value || ''); + } + }); return queryParams.toString(); };