Skip to content

Commit 4a42875

Browse files
authored
feat(nextjs): Throw when component is mounted on non-catch-all route (clerk#3204)
1 parent 3daa937 commit 4a42875

9 files changed

Lines changed: 111 additions & 22 deletions

File tree

.changeset/five-bees-boil.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@clerk/nextjs': patch
3+
---
4+
5+
Throw a descriptive error when a Clerk component that is using path-based routing is mounted in a non-catch-all route

packages/nextjs/src/app-router/client/ClerkProvider.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ import { ClerkProvider as ReactClerkProvider } from '@clerk/clerk-react';
33
import { useRouter } from 'next/navigation';
44
import React, { useEffect, useTransition } from 'react';
55

6+
import { useSafeLayoutEffect } from '../../client-boundary/hooks/useSafeLayoutEffect';
67
import { ClerkNextOptionsProvider } from '../../client-boundary/NextOptionsContext';
7-
import { useSafeLayoutEffect } from '../../client-boundary/useSafeLayoutEffect';
88
import type { NextClerkProviderProps } from '../../types';
99
import { ClerkJSScript } from '../../utils/clerk-js-script';
1010
import { mergeNextClerkPropsWithEnv } from '../../utils/mergeNextClerkPropsWithEnv';
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
import { isProductionEnvironment } from '@clerk/shared';
2+
import type { RoutingStrategy } from '@clerk/types';
3+
import React from 'react';
4+
5+
import { usePagesRouter } from './usePagesRouter';
6+
7+
/**
8+
* This ugly hook enforces that the Clerk components are mounted in a catch-all route
9+
* For pages router, we can parse the pathname we get from the useRouter hook
10+
* For app router, there is no reliable way to do the same check right now, so we
11+
* fire a request to a path under window.location.href and we check whether the path
12+
* exists or not
13+
*/
14+
export const useEnforceCatchAllRoute = (component: string, path: string, routing?: RoutingStrategy) => {
15+
const ref = React.useRef(0);
16+
const { pagesRouter } = usePagesRouter();
17+
18+
// This check does not break the rules of hooks
19+
// as the condition will remain the same for the whole app lifecycle
20+
if (isProductionEnvironment()) {
21+
return;
22+
}
23+
24+
React.useEffect(() => {
25+
if (routing && routing !== 'path') {
26+
return;
27+
}
28+
29+
const ac = new AbortController();
30+
const error = () => {
31+
const correctPath = pagesRouter ? `${path}/[[...index]].tsx` : `${path}/[[...rest]]/page.tsx`;
32+
throw new Error(
33+
`Clerk: The "${path}" route is not a catch-all route. It is recommended to convert this route to a catch-all route, eg: "${correctPath}". Alternatively, update the ${component} component to use hash-based routing by setting the "routing" prop to "hash".`,
34+
);
35+
};
36+
37+
if (pagesRouter) {
38+
if (!pagesRouter.pathname.match(/\[\[\.\.\..+]]/)) {
39+
error();
40+
}
41+
} else {
42+
const check = async () => {
43+
// make sure to run this as soon as possible
44+
// but don't run again when strict mode is enabled
45+
ref.current++;
46+
if (ref.current > 1) {
47+
return;
48+
}
49+
let res;
50+
try {
51+
const url = `${window.location.origin}${window.location.pathname}/clerk_catchall_check_${Date.now()}`;
52+
res = await fetch(url, { signal: ac.signal });
53+
} catch (e) {
54+
// no op
55+
}
56+
if (res?.status === 404) {
57+
error();
58+
}
59+
};
60+
void check();
61+
}
62+
63+
return () => {
64+
// make sure to run this as soon as possible
65+
// but don't run again when strict mode is enabled
66+
if (ref.current > 1) {
67+
ac.abort();
68+
}
69+
};
70+
}, []);
71+
};
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import { useRoutingProps } from '@clerk/clerk-react/internal';
2+
import type { RoutingOptions } from '@clerk/types';
3+
4+
import { useEnforceCatchAllRoute } from './useEnforceCatchAllRoute';
5+
import { usePathnameWithoutCatchAll } from './usePathnameWithoutCatchAll';
6+
7+
export function useEnforceCorrectRoutingProps<T extends RoutingOptions>(componentName: string, props: T): T {
8+
const path = usePathnameWithoutCatchAll();
9+
const routingProps = useRoutingProps(componentName, props, { path });
10+
useEnforceCatchAllRoute(componentName, path, routingProps.routing);
11+
return routingProps;
12+
}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import { useRouter } from 'next/compat/router';
2+
3+
export const usePagesRouter = () => {
4+
// The compat version of useRouter returns null instead of throwing an error
5+
// when used inside app router instead of pages router
6+
// we use it to detect if the component is used inside pages or app router
7+
// so we can use the correct algorithm to get the path
8+
return { pagesRouter: useRouter() };
9+
};

packages/nextjs/src/client-boundary/usePathnameWithoutCatchAll.tsx renamed to packages/nextjs/src/client-boundary/hooks/usePathnameWithoutCatchAll.tsx

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,11 @@
1-
import { useRouter } from 'next/compat/router';
21
import React from 'react';
32

4-
export const usePathnameWithoutWithCatchAll = () => {
3+
import { usePagesRouter } from './usePagesRouter';
4+
5+
export const usePathnameWithoutCatchAll = () => {
56
const pathRef = React.useRef<string>();
6-
// The compat version of useRouter returns null instead of throwing an error
7-
// when used inside app router instead of pages router
8-
// we use it to detect if the component is used inside pages or app router
9-
// so we can use the correct algorithm to get the path
10-
const pagesRouter = useRouter();
7+
8+
const { pagesRouter } = usePagesRouter();
119

1210
if (pagesRouter) {
1311
if (pathRef.current) {

packages/nextjs/src/client-boundary/useSafeLayoutEffect.tsx renamed to packages/nextjs/src/client-boundary/hooks/useSafeLayoutEffect.tsx

File renamed without changes.

packages/nextjs/src/client-boundary/uiComponents.tsx

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import {
77
SignUp as BaseSignUp,
88
UserProfile as BaseUserProfile,
99
} from '@clerk/clerk-react';
10-
import { useRoutingProps } from '@clerk/clerk-react/internal';
1110
import type {
1211
CreateOrganizationProps,
1312
OrganizationProfileProps,
@@ -17,7 +16,7 @@ import type {
1716
} from '@clerk/types';
1817
import React from 'react';
1918

20-
import { usePathnameWithoutWithCatchAll } from './usePathnameWithoutCatchAll';
19+
import { useEnforceCorrectRoutingProps } from './hooks/useEnforceRoutingProps';
2120

2221
export {
2322
OrganizationList,
@@ -32,19 +31,17 @@ export {
3231

3332
// The assignment of UserProfile with BaseUserProfile props is used
3433
// to support the CustomPage functionality (eg UserProfile.Page)
35-
// Also the `typeof BaseUserProfile` is used to resolved the following error:
34+
// Also the `typeof BaseUserProfile` is used to resolve the following error:
3635
// "The inferred type of 'UserProfile' cannot be named without a reference to ..."
3736
export const UserProfile: typeof BaseUserProfile = Object.assign(
3837
(props: UserProfileProps) => {
39-
const path = usePathnameWithoutWithCatchAll();
40-
return <BaseUserProfile {...useRoutingProps('UserProfile', props, { path })} />;
38+
return <BaseUserProfile {...useEnforceCorrectRoutingProps('UserProfile', props)} />;
4139
},
4240
{ ...BaseUserProfile },
4341
);
4442

4543
export const CreateOrganization = (props: CreateOrganizationProps) => {
46-
const path = usePathnameWithoutWithCatchAll();
47-
return <BaseCreateOrganization {...useRoutingProps('CreateOrganization', props, { path })} />;
44+
return <BaseCreateOrganization {...useEnforceCorrectRoutingProps('CreateOrganization', props)} />;
4845
};
4946

5047
// The assignment of OrganizationProfile with BaseOrganizationProfile props is used
@@ -53,18 +50,15 @@ export const CreateOrganization = (props: CreateOrganizationProps) => {
5350
// "The inferred type of 'OrganizationProfile' cannot be named without a reference to ..."
5451
export const OrganizationProfile: typeof BaseOrganizationProfile = Object.assign(
5552
(props: OrganizationProfileProps) => {
56-
const path = usePathnameWithoutWithCatchAll();
57-
return <BaseOrganizationProfile {...useRoutingProps('OrganizationProfile', props, { path })} />;
53+
return <BaseOrganizationProfile {...useEnforceCorrectRoutingProps('OrganizationProfile', props)} />;
5854
},
5955
{ ...BaseOrganizationProfile },
6056
);
6157

6258
export const SignIn = (props: SignInProps) => {
63-
const path = usePathnameWithoutWithCatchAll();
64-
return <BaseSignIn {...useRoutingProps('SignIn', props, { path })} />;
59+
return <BaseSignIn {...useEnforceCorrectRoutingProps('SignIn', props)} />;
6560
};
6661

6762
export const SignUp = (props: SignUpProps) => {
68-
const path = usePathnameWithoutWithCatchAll();
69-
return <BaseSignUp {...useRoutingProps('SignUp', props, { path })} />;
63+
return <BaseSignUp {...useEnforceCorrectRoutingProps('SignUp', props)} />;
7064
};

packages/nextjs/src/pages/ClerkProvider.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ import { setErrorThrowerOptions } from '@clerk/clerk-react/internal';
44
import { useRouter } from 'next/router';
55
import React from 'react';
66

7+
import { useSafeLayoutEffect } from '../client-boundary/hooks/useSafeLayoutEffect';
78
import { ClerkNextOptionsProvider } from '../client-boundary/NextOptionsContext';
8-
import { useSafeLayoutEffect } from '../client-boundary/useSafeLayoutEffect';
99
import type { NextClerkProviderProps } from '../types';
1010
import { ClerkJSScript } from '../utils/clerk-js-script';
1111
import { invalidateNextRouterCache } from '../utils/invalidateNextRouterCache';

0 commit comments

Comments
 (0)