From 39b82caebd4a0483b6da3e8a09a8104ed3742027 Mon Sep 17 00:00:00 2001 From: Vaggelis Yfantis Date: Thu, 23 Nov 2023 13:43:32 +0200 Subject: [PATCH 1/3] feat(clerk-js): Throw error if routing options are used incorrectly --- packages/clerk-js/src/core/errors.ts | 4 ++++ packages/clerk-js/src/utils/authPropHelpers.ts | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/packages/clerk-js/src/core/errors.ts b/packages/clerk-js/src/core/errors.ts index cf3171782c..3eaf22c650 100644 --- a/packages/clerk-js/src/core/errors.ts +++ b/packages/clerk-js/src/core/errors.ts @@ -115,3 +115,7 @@ export function clerkRedirectUrlIsMissingScheme(): never { export function clerkFailedToLoadThirdPartyScript(name?: string): never { throw new Error(`${errorPrefix} Unable to retrieve a third party script${name ? ` ${name}` : ''}.`); } + +export function clerkInvalidRoutingStrategy(strategy?: string): never { + throw new Error(`${errorPrefix} Invalid routing strategy, path cannot be used in tandem with ${strategy}.`); +} diff --git a/packages/clerk-js/src/utils/authPropHelpers.ts b/packages/clerk-js/src/utils/authPropHelpers.ts index e2ebff8527..fd71f4d935 100644 --- a/packages/clerk-js/src/utils/authPropHelpers.ts +++ b/packages/clerk-js/src/utils/authPropHelpers.ts @@ -3,6 +3,7 @@ import type { ClerkOptions, DisplayConfigResource, RoutingOptions, RoutingStrate import type { ParsedQs } from 'qs'; import qs from 'qs'; +import { clerkInvalidRoutingStrategy } from '../core/errors'; import { hasBannedProtocol, isAllowedRedirectOrigin, isValidUrl } from './url'; type PickRedirectionUrlKey = 'afterSignUpUrl' | 'afterSignInUrl' | 'signInUrl' | 'signUpUrl'; @@ -124,5 +125,9 @@ export const normalizeRoutingOptions = ({ return { routing: 'path', path }; } + if (routing !== 'path' && !!path) { + clerkInvalidRoutingStrategy(routing); + } + return { routing, path } as RoutingOptions; }; From ba0940423985c26baf5930a8a6c9a35381c455cc Mon Sep 17 00:00:00 2001 From: Vaggelis Yfantis Date: Fri, 24 Nov 2023 12:28:19 +0200 Subject: [PATCH 2/3] test(clerk-js): Update normalizeRoutingOptions tests --- .../src/ui/common/__tests__/authPropHelpers.test.ts | 11 +++++++---- packages/clerk-js/src/utils/authPropHelpers.ts | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/clerk-js/src/ui/common/__tests__/authPropHelpers.test.ts b/packages/clerk-js/src/ui/common/__tests__/authPropHelpers.test.ts index 7cc2241ca4..e083d50549 100644 --- a/packages/clerk-js/src/ui/common/__tests__/authPropHelpers.test.ts +++ b/packages/clerk-js/src/ui/common/__tests__/authPropHelpers.test.ts @@ -6,10 +6,13 @@ describe('auth prop helpers', () => { expect(normalizeRoutingOptions({ path: 'test' })).toEqual({ routing: 'path', path: 'test' }); }); - it('returns the same options if routing was provided (any routing) and path was provided (avoid breaking integrations)', () => { - expect(normalizeRoutingOptions({ routing: 'path', path: 'test' })).toEqual({ routing: 'path', path: 'test' }); - expect(normalizeRoutingOptions({ routing: 'hash', path: 'test' })).toEqual({ routing: 'hash', path: 'test' }); - expect(normalizeRoutingOptions({ routing: 'virtual' })).toEqual({ routing: 'virtual' }); + it('it throws an error when path is provided and routing strategy is not path', () => { + expect(() => { + normalizeRoutingOptions({ path: 'test', routing: 'hash' }); + }).toThrow('ClerkJS: Invalid routing strategy, path cannot be used in tandem with hash.'); + expect(() => { + normalizeRoutingOptions({ path: 'test', routing: 'virtual' }); + }).toThrow('ClerkJS: Invalid routing strategy, path cannot be used in tandem with virtual.'); }); }); }); diff --git a/packages/clerk-js/src/utils/authPropHelpers.ts b/packages/clerk-js/src/utils/authPropHelpers.ts index fd71f4d935..45442f1e71 100644 --- a/packages/clerk-js/src/utils/authPropHelpers.ts +++ b/packages/clerk-js/src/utils/authPropHelpers.ts @@ -126,7 +126,7 @@ export const normalizeRoutingOptions = ({ } if (routing !== 'path' && !!path) { - clerkInvalidRoutingStrategy(routing); + return clerkInvalidRoutingStrategy(routing); } return { routing, path } as RoutingOptions; From 9d95ebf89d23e01a8ee4d36e3e10d4ec40b3d63c Mon Sep 17 00:00:00 2001 From: Vaggelis Yfantis Date: Fri, 24 Nov 2023 13:39:38 +0200 Subject: [PATCH 3/3] chore(repo): Added Changeset --- .changeset/brave-suits-drive.md | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 .changeset/brave-suits-drive.md diff --git a/.changeset/brave-suits-drive.md b/.changeset/brave-suits-drive.md new file mode 100644 index 0000000000..466a47e522 --- /dev/null +++ b/.changeset/brave-suits-drive.md @@ -0,0 +1,10 @@ +--- +'@clerk/clerk-js': major +--- + +All the components that using routing will throw a runtime error if the a path property is provided with a routing strategy other than path. + +Example that will throw an error: +```tsx + +```