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. diff --git a/package-lock.json b/package-lock.json index 3fd664a8f89..64943750a51 100644 --- a/package-lock.json +++ b/package-lock.json @@ -36242,6 +36242,7 @@ }, "node_modules/qs": { "version": "6.11.0", + "dev": true, "license": "BSD-3-Clause", "dependencies": { "side-channel": "^1.0.4" @@ -44608,7 +44609,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": { @@ -44621,7 +44621,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/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" }, diff --git a/packages/clerk-js/package.json b/packages/clerk-js/package.json index a6d3df790c6..df39ec345e5 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": { @@ -77,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/__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/core/fapiClient.ts b/packages/clerk-js/src/core/fapiClient.ts index aaa1705ae66..deca8fe5b78 100644 --- a/packages/clerk-js/src/core/fapiClient.ts +++ b/packages/clerk-js/src/core/fapiClient.ts @@ -1,8 +1,7 @@ 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 { 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'; @@ -32,10 +31,6 @@ export type FapiRequestCallback = ( response?: FapiResponse, ) => Promise | unknown | false; -const camelToSnakeEncoder: qs.IStringifyOptions['encoder'] = (str, defaultEncoder, _, type) => { - return type === 'key' ? camelToSnake(str) : defaultEncoder(str); -}; - // TODO: Move to @clerk/types export interface FapiResponseJSON { response: T; @@ -125,7 +120,7 @@ export function createFapiClient(clerkInstance: Clerk): FapiClient { return acc; }, {} as FapiQueryStringParameters & Record); - return qs.stringify(objParams, { addQueryPrefix: true, arrayFormat: 'repeat' }); + return stringifyQueryParams(objParams); } function buildUrl(requestInit: FapiRequestInit): URL { @@ -193,8 +188,14 @@ 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 }); + // 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/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/BaseRouter.tsx b/packages/clerk-js/src/ui/router/BaseRouter.tsx index aad77cf2964..9cdafc3e581 100644 --- a/packages/clerk-js/src/ui/router/BaseRouter.tsx +++ b/packages/clerk-js/src/ui/router/BaseRouter.tsx @@ -1,9 +1,8 @@ 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'; +import { getQueryParams, stringifyQueryParams, trimTrailingSlash } from '../../utils'; import { useWindowEventListener } from '../hooks'; import { newPaths } from './newPaths'; import { match } from './pathToRegexp'; @@ -113,7 +112,8 @@ export const BaseRouter = ({ toQueryParams[param] = currentQueryParams[param]; } }); - toURL.search = qs.stringify(toQueryParams); + + 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/ui/router/index.tsx b/packages/clerk-js/src/ui/router/index.tsx index aab3a4c0b55..b6c89520467 100644 --- a/packages/clerk-js/src/ui/router/index.tsx +++ b/packages/clerk-js/src/ui/router/index.tsx @@ -5,4 +5,4 @@ export * from './VirtualRouter'; export * from './Route'; export * from './Switch'; -export type { ParsedQs } from 'qs'; +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 1b29316dd11..52744ac772f 100644 --- a/packages/clerk-js/src/utils/__tests__/querystring.test.ts +++ b/packages/clerk-js/src/utils/__tests__/querystring.test.ts @@ -1,16 +1,75 @@ +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('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({})).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 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( + 'unsafe_metadata=%7B%22bar%22%3A%221%22%7D', + ); + }); }); +//test=ena%3Dduo diff --git a/packages/clerk-js/src/utils/querystring.ts b/packages/clerk-js/src/utils/querystring.ts index b5b6138a589..c63533d36a6 100644 --- a/packages/clerk-js/src/utils/querystring.ts +++ b/packages/clerk-js/src/utils/querystring.ts @@ -1,11 +1,56 @@ -import qs from 'qs'; - export const getQueryParams = (queryString: string) => { - return qs.parse(queryString || '', { - ignoreQueryPrefix: true, - }) as Record; + const queryParamsObject: { [key: string]: string | string[] } = {}; + const queryParams = new URLSearchParams(queryString); + queryParams.forEach((value, key) => { + 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; +}; + +type StringifyQueryParamsOptions = { + keyEncoder?: (key: string) => string; }; -export const stringifyQueryParams = (params: Record | Array) => { - return qs.stringify(params || {}); +export const stringifyQueryParams = ( + params: + | Record> + | null + | undefined + | string, + opts: StringifyQueryParamsOptions = {}, +) => { + if (params === null || params === undefined) { + return ''; + } + if (!params || typeof params !== 'object') { + return ''; + } + + const queryParams = new URLSearchParams(); + + 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(); };