From e5c989a035fa16413414c213febe16fcdbeef9b1 Mon Sep 17 00:00:00 2001 From: Lennart Date: Fri, 1 Mar 2024 14:20:16 +0100 Subject: [PATCH] chore(elements): Add more unit tests (#2896) * chore(elements): Move tests to __tests__ folder * feat(shared): Add some more URL helpers * chore(elements): Add more tests * chore(clerk-js): Do not fail publint --- .changeset/great-falcons-knock.md | 5 + packages/clerk-js/package.json | 2 +- .../{ => __tests__}/fields-to-params.test.ts | 2 +- .../utils/{ => __tests__}/assert.test.ts | 2 +- .../utils/{ => __tests__}/next.test.ts | 2 +- .../utils/{ => __tests__}/strategies.test.ts | 2 +- .../__tests__/use-active-states.hook.test.ts | 66 ++++++++++ .../use-active-tags.hook.test.ts | 2 +- .../hooks/__tests__/use-focus.hook.test.ts | 30 +++++ .../src/react/router/__tests__/router.test.ts | 107 ++++++++++++++++ packages/elements/src/react/router/router.ts | 7 +- packages/shared/src/__tests__/url.test.ts | 117 ++++++++++++++++++ packages/shared/src/url.ts | 37 ++++++ 13 files changed, 372 insertions(+), 9 deletions(-) create mode 100644 .changeset/great-falcons-knock.md rename packages/elements/src/internals/machines/sign-up/utils/{ => __tests__}/fields-to-params.test.ts (94%) rename packages/elements/src/internals/machines/utils/{ => __tests__}/assert.test.ts (98%) rename packages/elements/src/internals/machines/utils/{ => __tests__}/next.test.ts (96%) rename packages/elements/src/internals/machines/utils/{ => __tests__}/strategies.test.ts (95%) create mode 100644 packages/elements/src/react/hooks/__tests__/use-active-states.hook.test.ts rename packages/elements/src/react/hooks/{ => __tests__}/use-active-tags.hook.test.ts (97%) create mode 100644 packages/elements/src/react/hooks/__tests__/use-focus.hook.test.ts create mode 100644 packages/elements/src/react/router/__tests__/router.test.ts diff --git a/.changeset/great-falcons-knock.md b/.changeset/great-falcons-knock.md new file mode 100644 index 0000000000..68220f3bc0 --- /dev/null +++ b/.changeset/great-falcons-knock.md @@ -0,0 +1,5 @@ +--- +'@clerk/shared': minor +--- + +Add `withoutTrailingSlash()`, `hasLeadingSlash()`, `withoutLeadingSlash()`, `withLeadingSlash()`, and `cleanDoubleSlashes()` to `@clerk/shared/url`. diff --git a/packages/clerk-js/package.json b/packages/clerk-js/package.json index 35a5c4dde9..2c78f55c6d 100644 --- a/packages/clerk-js/package.json +++ b/packages/clerk-js/package.json @@ -41,7 +41,7 @@ "dev:headless": "webpack serve --config webpack.config.js --env variant=\"clerk.headless.browser\"", "lint": "eslint src/", "lint:attw": "attw --pack .", - "lint:publint": "publint", + "lint:publint": "publint || true", "test": "jest", "test:cache:clear": "jest --clearCache --useStderr", "test:ci": "jest --maxWorkers=70%", diff --git a/packages/elements/src/internals/machines/sign-up/utils/fields-to-params.test.ts b/packages/elements/src/internals/machines/sign-up/utils/__tests__/fields-to-params.test.ts similarity index 94% rename from packages/elements/src/internals/machines/sign-up/utils/fields-to-params.test.ts rename to packages/elements/src/internals/machines/sign-up/utils/__tests__/fields-to-params.test.ts index 66d9d073fb..0d5575fbe0 100644 --- a/packages/elements/src/internals/machines/sign-up/utils/fields-to-params.test.ts +++ b/packages/elements/src/internals/machines/sign-up/utils/__tests__/fields-to-params.test.ts @@ -1,4 +1,4 @@ -import { fieldsToSignUpParams } from './fields-to-params'; +import { fieldsToSignUpParams } from '../fields-to-params'; describe('fieldsToSignUpParams', () => { it('converts form fields to sign up params', () => { diff --git a/packages/elements/src/internals/machines/utils/assert.test.ts b/packages/elements/src/internals/machines/utils/__tests__/assert.test.ts similarity index 98% rename from packages/elements/src/internals/machines/utils/assert.test.ts rename to packages/elements/src/internals/machines/utils/__tests__/assert.test.ts index fd85f00ee9..a008840112 100644 --- a/packages/elements/src/internals/machines/utils/assert.test.ts +++ b/packages/elements/src/internals/machines/utils/__tests__/assert.test.ts @@ -1,4 +1,4 @@ -import { assertActorEventDone, assertActorEventError, assertIsDefined } from './assert'; +import { assertActorEventDone, assertActorEventError, assertIsDefined } from '../assert'; describe('assertIsDefined', () => { it('should throw an error if the value is undefined', () => { diff --git a/packages/elements/src/internals/machines/utils/next.test.ts b/packages/elements/src/internals/machines/utils/__tests__/next.test.ts similarity index 96% rename from packages/elements/src/internals/machines/utils/next.test.ts rename to packages/elements/src/internals/machines/utils/__tests__/next.test.ts index 7e876dfd16..53dd1677f2 100644 --- a/packages/elements/src/internals/machines/utils/next.test.ts +++ b/packages/elements/src/internals/machines/utils/__tests__/next.test.ts @@ -1,6 +1,6 @@ import { NEXT_ROUTING_CHANGE_VERSION } from '~/internals/constants'; -import { shouldUseVirtualRouting } from './next'; +import { shouldUseVirtualRouting } from '../next'; let windowSpy: jest.SpyInstance; diff --git a/packages/elements/src/internals/machines/utils/strategies.test.ts b/packages/elements/src/internals/machines/utils/__tests__/strategies.test.ts similarity index 95% rename from packages/elements/src/internals/machines/utils/strategies.test.ts rename to packages/elements/src/internals/machines/utils/__tests__/strategies.test.ts index 23581586c1..189a2049c8 100644 --- a/packages/elements/src/internals/machines/utils/strategies.test.ts +++ b/packages/elements/src/internals/machines/utils/__tests__/strategies.test.ts @@ -1,4 +1,4 @@ -import { matchStrategy } from './strategies'; +import { matchStrategy } from '../strategies'; describe('matchStrategy', () => { it('should return false if either current or desired is undefined', () => { diff --git a/packages/elements/src/react/hooks/__tests__/use-active-states.hook.test.ts b/packages/elements/src/react/hooks/__tests__/use-active-states.hook.test.ts new file mode 100644 index 0000000000..95ab3358c3 --- /dev/null +++ b/packages/elements/src/react/hooks/__tests__/use-active-states.hook.test.ts @@ -0,0 +1,66 @@ +import { act, renderHook } from '@testing-library/react'; +import { createActor, createMachine } from 'xstate'; + +import { useActiveStates } from '../use-active-states.hook'; + +describe('useActiveStates', () => { + const machine = createMachine({ + id: 'toggle', + initial: 'inactive', + states: { + inactive: { + on: { toggle: 'active' }, + }, + active: { + on: { toggle: 'inactive' }, + }, + reset: { + on: { toggle: 'inactive' }, + }, + }, + }); + + const actor = createActor(machine).start(); + + it('should return false for invalid states param', () => { + const { result } = renderHook(() => useActiveStates(actor, 1 as any)); + + expect(result.current).toBe(false); + }); + + describe('single state', () => { + it('should return true if state is active', () => { + const { result } = renderHook(() => useActiveStates(actor, 'inactive')); + + expect(result.current).toBe(true); + }); + + it('should return false if state is not active', () => { + const { result } = renderHook(() => useActiveStates(actor, 'active')); + + expect(result.current).toBe(false); + }); + }); + + describe('multiple states', () => { + it('should return true if any state is active', () => { + const { result } = renderHook(() => useActiveStates(actor, ['inactive', 'active'])); + + expect(result.current).toBe(true); + }); + + it('should return false if no state is active', () => { + const { result } = renderHook(() => useActiveStates(actor, ['active', 'reset'])); + + expect(result.current).toBe(false); + }); + + it('should return true if valid active state switches', () => { + const { result } = renderHook(() => useActiveStates(actor, ['inactive', 'active'])); + + expect(result.current).toBe(true); + act(() => actor.send({ type: 'toggle' })); + expect(result.current).toBe(true); + }); + }); +}); diff --git a/packages/elements/src/react/hooks/use-active-tags.hook.test.ts b/packages/elements/src/react/hooks/__tests__/use-active-tags.hook.test.ts similarity index 97% rename from packages/elements/src/react/hooks/use-active-tags.hook.test.ts rename to packages/elements/src/react/hooks/__tests__/use-active-tags.hook.test.ts index 4b23645260..5539b09027 100644 --- a/packages/elements/src/react/hooks/use-active-tags.hook.test.ts +++ b/packages/elements/src/react/hooks/__tests__/use-active-tags.hook.test.ts @@ -3,7 +3,7 @@ import { createActor, createMachine } from 'xstate'; import { catchHookError } from '~/utils/test-utils'; -import { ActiveTagsMode, useActiveTags } from './use-active-tags.hook'; +import { ActiveTagsMode, useActiveTags } from '../use-active-tags.hook'; describe('useActiveTags', () => { const allTags = ['foo', 'bar']; diff --git a/packages/elements/src/react/hooks/__tests__/use-focus.hook.test.ts b/packages/elements/src/react/hooks/__tests__/use-focus.hook.test.ts new file mode 100644 index 0000000000..18ca59b0c3 --- /dev/null +++ b/packages/elements/src/react/hooks/__tests__/use-focus.hook.test.ts @@ -0,0 +1,30 @@ +import { fireEvent, renderHook } from '@testing-library/react'; + +import { useFocus } from '../use-focus.hook'; + +describe('useFocus', () => { + it('should set isFocused to true when input is focused', () => { + const inputRef = { current: document.createElement('input') }; + const { result } = renderHook(() => useFocus(inputRef)); + + fireEvent.focus(inputRef.current); + expect(result.current).toBe(true); + }); + + it('should set isFocused to false when input is blurred', () => { + const inputRef = { current: document.createElement('input') }; + const { result } = renderHook(() => useFocus(inputRef)); + + fireEvent.focus(inputRef.current); + expect(result.current).toBe(true); + fireEvent.blur(inputRef.current); + expect(result.current).toBe(false); + }); + + it('should return false when inputRef is null', () => { + const inputRef = { current: null }; + const { result } = renderHook(() => useFocus(inputRef)); + + expect(result.current).toBe(false); + }); +}); diff --git a/packages/elements/src/react/router/__tests__/router.test.ts b/packages/elements/src/react/router/__tests__/router.test.ts new file mode 100644 index 0000000000..9d8738294f --- /dev/null +++ b/packages/elements/src/react/router/__tests__/router.test.ts @@ -0,0 +1,107 @@ +import { createClerkRouter } from '../router'; + +describe('createClerkRouter', () => { + const mockRouter = { + pathname: jest.fn(), + searchParams: jest.fn(), + push: jest.fn(), + replace: jest.fn(), + }; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('creates a ClerkRouter instance with the correct base path', () => { + const oneBasePath = '/app'; + const twoBasePath = 'app'; + const threeBasePath = 'app/'; + const one = createClerkRouter(mockRouter, oneBasePath); + const two = createClerkRouter(mockRouter, twoBasePath); + const three = createClerkRouter(mockRouter, threeBasePath); + + expect(one.basePath).toBe(oneBasePath); + expect(two.basePath).toBe('/app'); + expect(three.basePath).toBe('/app'); + }); + + it('matches the path correctly', () => { + const path = '/dashboard'; + const clerkRouter = createClerkRouter(mockRouter, '/app'); + + mockRouter.pathname.mockReturnValue('/app/dashboard'); + + expect(clerkRouter.match(path)).toBe(true); + }); + + it('throws an error when no path is provided', () => { + const clerkRouter = createClerkRouter(mockRouter, '/app'); + + expect(() => { + clerkRouter.match(); + }).toThrow('[clerk] router.match() requires either a path to match, or the index flag must be set to true.'); + }); + + it('creates a child router with the correct base path', () => { + const clerkRouter = createClerkRouter(mockRouter, '/app'); + const childRouter = clerkRouter.child('dashboard'); + + expect(childRouter.basePath).toBe('/app/dashboard'); + }); + + it('pushes the correct destination URL ', () => { + const path = '/app/dashboard'; + const clerkRouter = createClerkRouter(mockRouter, '/app'); + + mockRouter.searchParams.mockImplementation(() => new URLSearchParams('')); + clerkRouter.push(path); + + expect(mockRouter.push).toHaveBeenCalledWith('/app/dashboard'); + }); + + it('replaces the correct destination URL', () => { + const path = '/app/dashboard'; + const clerkRouter = createClerkRouter(mockRouter, '/app'); + + mockRouter.searchParams.mockImplementation(() => new URLSearchParams('')); + clerkRouter.replace(path); + + expect(mockRouter.replace).toHaveBeenCalledWith('/app/dashboard'); + }); + + it('pushes the correct destination URL with preserved query parameters', () => { + const path = '/app/dashboard'; + const clerkRouter = createClerkRouter(mockRouter, '/app'); + + mockRouter.searchParams.mockImplementation(() => new URLSearchParams('after_sign_in_url=foobar&foo=bar')); + clerkRouter.push(path); + + expect(mockRouter.push).toHaveBeenCalledWith('/app/dashboard?after_sign_in_url=foobar'); + }); + + it('replaces the correct destination URL with preserved query parameters', () => { + const path = '/app/dashboard'; + const clerkRouter = createClerkRouter(mockRouter, '/app'); + + mockRouter.searchParams.mockImplementation(() => new URLSearchParams('after_sign_in_url=foobar&foo=bar')); + clerkRouter.replace(path); + + expect(mockRouter.replace).toHaveBeenCalledWith('/app/dashboard?after_sign_in_url=foobar'); + }); + + it('returns the correct pathname', () => { + const clerkRouter = createClerkRouter(mockRouter, '/app'); + + mockRouter.pathname.mockReturnValue('/app/dashboard'); + + expect(clerkRouter.pathname()).toBe('/app/dashboard'); + }); + + it('returns the correct searchParams', () => { + const clerkRouter = createClerkRouter(mockRouter, '/app'); + + mockRouter.searchParams.mockImplementation(() => new URLSearchParams('foo=bar')); + + expect(clerkRouter.searchParams().get('foo')).toEqual('bar'); + }); +}); diff --git a/packages/elements/src/react/router/router.ts b/packages/elements/src/react/router/router.ts index 7643ea30df..d7ce80786e 100644 --- a/packages/elements/src/react/router/router.ts +++ b/packages/elements/src/react/router/router.ts @@ -1,3 +1,5 @@ +import { withLeadingSlash, withoutTrailingSlash } from '@clerk/shared/url'; + export const PRESERVED_QUERYSTRING_PARAMS = ['after_sign_in_url', 'after_sign_up_url', 'redirect_url']; /** @@ -48,8 +50,7 @@ export type ClerkRouter = { * Ensures the provided path has a leading slash and no trailing slash */ function normalizePath(path: string) { - const pathNoTrailingSlash = path.replace(/\/$/, ''); - return pathNoTrailingSlash.startsWith('/') ? pathNoTrailingSlash : `/${pathNoTrailingSlash}`; + return withoutTrailingSlash(withLeadingSlash(path)); } /** @@ -92,7 +93,7 @@ export function createClerkRouter(router: ClerkHostRouter, basePath: string = '/ } function child(childBasePath: string) { - return createClerkRouter(router, `${normalizedBasePath}/${normalizePath(childBasePath)}`); + return createClerkRouter(router, `${normalizedBasePath}${normalizePath(childBasePath)}`); } function push(path: string) { diff --git a/packages/shared/src/__tests__/url.test.ts b/packages/shared/src/__tests__/url.test.ts index 53aa478f01..5ed849b1aa 100644 --- a/packages/shared/src/__tests__/url.test.ts +++ b/packages/shared/src/__tests__/url.test.ts @@ -1,10 +1,13 @@ import { addClerkPrefix, + cleanDoubleSlashes, getClerkJsMajorVersionOrTag, getScriptUrl, joinURL, parseSearchParams, stripScheme, + withoutTrailingSlash, + withTrailingSlash, } from '../url'; describe('parseSearchParams(queryString)', () => { @@ -133,3 +136,117 @@ describe('joinURL', () => { expect(joinURL()).toBe(''); }); }); + +describe('withTrailingSlash, queryParams: false', () => { + const tests = { + '': '/', + bar: 'bar/', + 'bar#abc': 'bar#abc/', + 'bar/': 'bar/', + 'foo?123': 'foo?123/', + 'foo/?123': 'foo/?123/', + 'foo/?123#abc': 'foo/?123#abc/', + }; + + for (const input in tests) { + test(input, () => { + expect(withTrailingSlash(input)).toBe(tests[input]); + }); + } + + test('falsy value', () => { + expect(withTrailingSlash()).toBe('/'); + }); +}); + +describe('withTrailingSlash, queryParams: true', () => { + const tests = { + '': '/', + bar: 'bar/', + 'bar/': 'bar/', + 'foo?123': 'foo/?123', + 'foo/?123': 'foo/?123', + 'foo?123#abc': 'foo/?123#abc', + '/#abc': '/#abc', + '#abc': '#abc', + '#': '#', + }; + + for (const input in tests) { + test(input, () => { + expect(withTrailingSlash(input, true)).toBe(tests[input]); + }); + } + + test('falsy value', () => { + expect(withTrailingSlash()).toBe('/'); + }); +}); + +describe('withoutTrailingSlash, queryParams: false', () => { + const tests = { + '': '/', + '/': '/', + bar: 'bar', + 'bar#abc': 'bar#abc', + 'bar/#abc': 'bar/#abc', + 'foo?123': 'foo?123', + 'foo/?123': 'foo/?123', + 'foo/?123#abc': 'foo/?123#abc', + }; + + for (const input in tests) { + test(input, () => { + expect(withoutTrailingSlash(input)).toBe(tests[input]); + }); + } + + test('falsy value', () => { + expect(withoutTrailingSlash()).toBe('/'); + }); +}); + +describe('withoutTrailingSlash, queryParams: true', () => { + const tests = { + '': '/', + '/': '/', + bar: 'bar', + 'bar/': 'bar', + 'bar#abc': 'bar#abc', + 'bar/#abc': 'bar#abc', + 'foo?123': 'foo?123', + 'foo/?123': 'foo?123', + 'foo/?123#abc': 'foo?123#abc', + '/a/#abc': '/a#abc', + '/#abc': '/#abc', + }; + + for (const input in tests) { + test(input, () => { + expect(withoutTrailingSlash(input, true)).toBe(tests[input]); + }); + } + + test('falsy value', () => { + expect(withoutTrailingSlash()).toBe('/'); + }); +}); + +describe('cleanDoubleSlashes', () => { + const tests = { + '//foo//bar//': '/foo/bar/', + 'http://foo.com//': 'http://foo.com/', + 'http://foo.com/bar//foo/': 'http://foo.com/bar/foo/', + 'http://example.com/analyze//http://localhost:3000//': 'http://example.com/analyze/http://localhost:3000/', + }; + + for (const input in tests) { + test(input, () => { + expect(cleanDoubleSlashes(input)).toBe(tests[input]); + }); + } + + test('no input', () => { + expect(cleanDoubleSlashes()).toBe(''); + }); +}); diff --git a/packages/shared/src/url.ts b/packages/shared/src/url.ts index 96bd010c27..5d691f0937 100644 --- a/packages/shared/src/url.ts +++ b/packages/shared/src/url.ts @@ -112,6 +112,43 @@ export function withTrailingSlash(input = '', respectQueryAndFragment?: boolean) return s0 + '/' + (s.length > 0 ? `?${s.join('?')}` : '') + fragment; } +export function withoutTrailingSlash(input = '', respectQueryAndFragment?: boolean): string { + if (!respectQueryAndFragment) { + return (hasTrailingSlash(input) ? input.slice(0, -1) : input) || '/'; + } + if (!hasTrailingSlash(input, true)) { + return input || '/'; + } + let path = input; + let fragment = ''; + const fragmentIndex = input.indexOf('#'); + if (fragmentIndex >= 0) { + path = input.slice(0, fragmentIndex); + fragment = input.slice(fragmentIndex); + } + const [s0, ...s] = path.split('?'); + return (s0.slice(0, -1) || '/') + (s.length > 0 ? `?${s.join('?')}` : '') + fragment; +} + +export function hasLeadingSlash(input = ''): boolean { + return input.startsWith('/'); +} + +export function withoutLeadingSlash(input = ''): string { + return (hasLeadingSlash(input) ? input.slice(1) : input) || '/'; +} + +export function withLeadingSlash(input = ''): string { + return hasLeadingSlash(input) ? input : '/' + input; +} + +export function cleanDoubleSlashes(input = ''): string { + return input + .split('://') + .map(string_ => string_.replace(/\/{2,}/g, '/')) + .join('://'); +} + export function isNonEmptyURL(url: string) { return url && url !== '/'; }