Skip to content

Commit 17a6158

Browse files
nikosdouvlisnikosdouvlis
authored
fix(clerk-js,types,nextjs): Allow legacy redirect props as a fallback (clerk#3224)
* fix(clerk-js,types,nextjs): Allow legacy redirect props as a fallback * chore(repo): Temporarily skip elements tasks
1 parent 68b227e commit 17a6158

13 files changed

Lines changed: 97 additions & 54 deletions

File tree

‎.changeset/cold-files-refuse.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@clerk/clerk-js': patch
3+
---
4+
5+
Warn about legacy redirect search params

‎.changeset/friendly-masks-obey.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,4 @@
44
'@clerk/types': patch
55
---
66

7-
Introduce forceRedirectUrl and fallbackRedirectUrl
7+
Support but warn when `afterSignInUrl` and `afterSignUpUrl` are used

‎.github/workflows/ci.yml‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,10 @@ jobs:
5353
run: npm run format:check
5454

5555
- name: Build Packages
56-
run: npx turbo build $TURBO_ARGS --only
56+
run: npx turbo build $TURBO_ARGS --filter=!elements --only
5757

5858
- name: Lint packages using publint
59-
run: npx turbo lint:publint $TURBO_ARGS --only
59+
run: npx turbo lint:publint $TURBO_ARGS --filter=!elements --only
6060

6161
- name: Lint types using attw
6262
run: npx turbo lint:attw $TURBO_ARGS --filter=!nextjs --filter=!elements --filter=!backend --only
@@ -66,7 +66,7 @@ jobs:
6666
continue-on-error: true # TODO: Remove this when all related errors are fixed
6767

6868
- name: Run lint
69-
run: npx turbo lint $TURBO_ARGS --only -- --quiet
69+
run: npx turbo lint $TURBO_ARGS --filter=!elements --only -- --quiet
7070

7171
- name: Upload Turbo Summary
7272
uses: actions/upload-artifact@v3
@@ -108,7 +108,7 @@ jobs:
108108
turbo-token: ${{ secrets.TURBO_TOKEN }}
109109

110110
- name: Run tests
111-
run: npx turbo test $TURBO_ARGS
111+
run: npx turbo test $TURBO_ARGS --filter=!elements
112112
env:
113113
NODE_VERSION: ${{ matrix.node-version }}
114114

@@ -152,7 +152,7 @@ jobs:
152152
uses: ./.github/actions/verdaccio
153153
with:
154154
publish-cmd: |
155-
if [ "$(npm config get registry)" = "https://registry.npmjs.org/" ]; then echo 'Error: Using default registry' && exit 1; else npx turbo build $TURBO_ARGS --only && npx changeset publish --no-git-tag; fi
155+
if [ "$(npm config get registry)" = "https://registry.npmjs.org/" ]; then echo 'Error: Using default registry' && exit 1; else npx turbo build $TURBO_ARGS --filter=!elements --only && npx changeset publish --no-git-tag; fi
156156
157157
- name: Install @clerk/backend in /integration
158158
working-directory: ./integration

‎package.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
]
1010
},
1111
"scripts": {
12-
"build": "FORCE_COLOR=1 turbo build --concurrency=${TURBO_CONCURRENCY:-80%}",
12+
"build": "FORCE_COLOR=1 turbo build --concurrency=${TURBO_CONCURRENCY:-80%} --filter=!elements",
1313
"bundlewatch": "turbo bundlewatch",
1414
"changeset": "changeset",
1515
"changeset:empty": "npm run changeset -- --empty",
@@ -28,7 +28,7 @@
2828
"release:canary": "changeset publish --tag canary --no-git-tag",
2929
"release:snapshot": "changeset publish --tag snapshot --no-git-tag",
3030
"release:verdaccio": "if [ \"$(npm config get registry)\" = \"https://registry.npmjs.org/\" ]; then echo 'Error: Using default registry' && exit 1; else TURBO_CONCURRENCY=1 npm run build && changeset publish --no-git-tag; fi",
31-
"test": "FORCE_COLOR=1 turbo test --concurrency=${TURBO_CONCURRENCY:-80%}",
31+
"test": "FORCE_COLOR=1 turbo test --concurrency=${TURBO_CONCURRENCY:-80%} --filter=!elements",
3232
"test:cache:clear": "FORCE_COLOR=1 turbo test:cache:clear --continue --concurrency=${TURBO_CONCURRENCY:-80%}",
3333
"test:integration:ap-flows": "npm run test:integration:base -- --grep @ap-flows",
3434
"test:integration:base": "DEBUG=1 npx playwright test --config integration/playwright.config.ts",

‎packages/clerk-js/src/core/constants.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// TODO: Do we still have a use for this or can we simply preserve all params?
22
export const PRESERVED_QUERYSTRING_PARAMS = [
33
'redirect_url',
4+
'after_sign_in_url',
5+
'after_sign_up_url',
46
'sign_in_force_redirect_url',
57
'sign_in_fallback_redirect_url',
68
'sign_up_force_redirect_url',

‎packages/clerk-js/src/ui/contexts/ClerkUIComponentsContext.tsx‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import React, { useMemo } from 'react';
55

66
import { SIGN_IN_INITIAL_VALUE_KEYS, SIGN_UP_INITIAL_VALUE_KEYS } from '../../core/constants';
77
import { buildURL, createDynamicParamParser } from '../../utils';
8-
import { assertNoLegacyProp } from '../../utils/assertNoLegacyProp';
98
import { RedirectUrls } from '../../utils/redirectUrls';
109
import { ORGANIZATION_PROFILE_NAVBAR_ROUTE_ID } from '../constants';
1110
import { useEnvironment, useOptions } from '../contexts';
@@ -62,9 +61,6 @@ export const useSignUpContext = (): SignUpContextType => {
6261
const options = useOptions();
6362
const clerk = useClerk();
6463

65-
assertNoLegacyProp(options);
66-
assertNoLegacyProp(ctx);
67-
6864
const initialValuesFromQueryParams = useMemo(
6965
() => getInitialValuesFromQueryParams(queryString, SIGN_UP_INITIAL_VALUE_KEYS),
7066
[],
@@ -135,9 +131,6 @@ export const useSignInContext = (): SignInContextType => {
135131
const options = useOptions();
136132
const clerk = useClerk();
137133

138-
assertNoLegacyProp(options);
139-
assertNoLegacyProp(ctx);
140-
141134
const initialValuesFromQueryParams = useMemo(
142135
() => getInitialValuesFromQueryParams(queryString, SIGN_IN_INITIAL_VALUE_KEYS),
143136
[],

‎packages/clerk-js/src/utils/__tests__/redirectUrls.test.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,31 @@ describe('redirectUrls', () => {
7676
});
7777

7878
describe('get redirect urls', () => {
79+
// TODO: v6 - remove this test
80+
it('prioritizes new props over legacy props ', () => {
81+
const redirectUrls = new RedirectUrls({
82+
signInFallbackRedirectUrl: 'sign-in-fallback-redirect-url',
83+
signUpFallbackRedirectUrl: 'sign-up-fallback-redirect-url',
84+
afterSignInUrl: 'after-sign-in-url',
85+
afterSignUpUrl: 'after-sign-up-url',
86+
});
87+
88+
expect(redirectUrls.getAfterSignInUrl()).toBe(`${mockWindowLocation.href}sign-in-fallback-redirect-url`);
89+
expect(redirectUrls.getAfterSignUpUrl()).toBe(`${mockWindowLocation.href}sign-up-fallback-redirect-url`);
90+
});
91+
92+
// TODO: v6 - remove this test
93+
it('falls back to legacy props if no new props are found', () => {
94+
const redirectUrls = new RedirectUrls({
95+
signUpFallbackRedirectUrl: 'sign-up-fallback-redirect-url',
96+
afterSignInUrl: 'after-sign-in-url',
97+
afterSignUpUrl: 'after-sign-up-url',
98+
});
99+
100+
expect(redirectUrls.getAfterSignInUrl()).toBe(`${mockWindowLocation.href}after-sign-in-url`);
101+
expect(redirectUrls.getAfterSignUpUrl()).toBe(`${mockWindowLocation.href}sign-up-fallback-redirect-url`);
102+
});
103+
79104
it('prioritizes force urls among other urls in the same group', () => {
80105
const redirectUrls = new RedirectUrls({
81106
signInForceRedirectUrl: 'sign-in-force-redirect-url',

‎packages/clerk-js/src/utils/assertNoLegacyProp.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
export function assertNoLegacyProp(props: Record<string, any>) {
2-
const legacyProps = ['redirectUrl', 'afterSignInUrl', 'afterSignUpUrl'];
2+
const legacyProps = ['redirectUrl', 'afterSignInUrl', 'afterSignUpUrl', 'after_sign_in_url', 'after_sign_up_url'];
33
const legacyProp = Object.keys(props).find(key => legacyProps.includes(key));
4+
45
if (legacyProp) {
56
// TODO: @nikos update with the docs link
67
console.warn(
7-
`Clerk: The prop "${legacyProp}" is deprecated and should be removed as it no longer works. Use the new "fallbackRedirectUrl" and "forceRedirectUrl" props instead.`,
8+
`Clerk: The prop "${legacyProp}" is deprecated and should be replaced with the new "fallbackRedirectUrl" or "forceRedirectUrl" props instead.`,
89
);
910
}
1011
}

‎packages/clerk-js/src/utils/redirectUrls.ts‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { camelToSnake } from '@clerk/shared/underscore';
33
import type { ClerkOptions, RedirectOptions } from '@clerk/types';
44
import type { ParsedQs } from 'qs';
55

6+
import { assertNoLegacyProp } from './assertNoLegacyProp';
67
import { buildURL, isAllowedRedirectOrigin, relativeToAbsoluteUrl } from './url';
78

89
export class RedirectUrls {
@@ -11,6 +12,8 @@ export class RedirectUrls {
1112
'signInFallbackRedirectUrl',
1213
'signUpForceRedirectUrl',
1314
'signUpFallbackRedirectUrl',
15+
'afterSignInUrl',
16+
'afterSignUpUrl',
1417
];
1518

1619
private static preserved = ['redirectUrl'];
@@ -68,17 +71,26 @@ export class RedirectUrls {
6871
#getRedirectUrl(prefix: 'signIn' | 'signUp') {
6972
const forceKey = `${prefix}ForceRedirectUrl` as const;
7073
const fallbackKey = `${prefix}FallbackRedirectUrl` as const;
74+
75+
const legacyPropKey = `after${prefix[0].toUpperCase()}${prefix.slice(1)}Url` as 'afterSignInUrl' | 'afterSignUpUrl';
76+
7177
let result;
7278
// Prioritize forceRedirectUrl
7379
result = this.fromSearchParams[forceKey] || this.fromProps[forceKey] || this.fromOptions[forceKey];
74-
// Try to get redirect_url that only allowed as a search param
80+
// Try to get redirect_url, only allowed as a search param
7581
result ||= this.fromSearchParams.redirectUrl;
7682
// Otherwise, fallback to fallbackRedirectUrl
7783
result ||= this.fromSearchParams[fallbackKey] || this.fromProps[fallbackKey] || this.fromOptions[fallbackKey];
84+
85+
// TODO: v6
86+
// Remove the compatibility layer for afterSignInUrl and afterSignUpUrl
87+
result ||= this.fromSearchParams[legacyPropKey] || this.fromProps[legacyPropKey] || this.fromOptions[legacyPropKey];
88+
7889
return result || '/';
7990
}
8091

8192
#parse(obj: unknown) {
93+
assertNoLegacyProp(obj as any);
8294
const res = {} as RedirectOptions;
8395
RedirectUrls.keys.forEach(key => {
8496
// @ts-expect-error
@@ -88,6 +100,7 @@ export class RedirectUrls {
88100
}
89101

90102
#parseSearchParams(obj: any) {
103+
assertNoLegacyProp(obj);
91104
const res = {} as typeof this.fromSearchParams;
92105
RedirectUrls.keys.forEach(key => {
93106
res[key] = obj[camelToSnake(key)];

‎packages/nextjs/src/global.d.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ declare global {
1919
NEXT_PUBLIC_CLERK_SIGN_UP_FORCE_REDIRECT_URL: string | undefined;
2020
NEXT_PUBLIC_CLERK_SIGN_IN_FALLBACK_REDIRECT_URL: string | undefined;
2121
NEXT_PUBLIC_CLERK_SIGN_UP_FALLBACK_REDIRECT_URL: string | undefined;
22+
NEXT_PUBLIC_CLERK_AFTER_SIGN_IN_URL: string | undefined;
23+
NEXT_PUBLIC_CLERK_AFTER_SIGN_UP_URL: string | undefined;
2224
}
2325
}
2426
}

0 commit comments

Comments
 (0)