-
Couldn't load subscription status.
- Fork 402
feat(clerk-js): Change the default behaviour of afterSignOutUrl, afterSignIn and afterSignUp #2020
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(clerk-js): Change the default behaviour of afterSignOutUrl, afterSignIn and afterSignUp #2020
Conversation
🦋 Changeset detectedLatest commit: 59ab8ae The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
2e81ce1 to
91fe24d
Compare
packages/clerk-js/src/core/clerk.ts
Outdated
| afterSignInUrl: '/', | ||
| afterSignUpUrl: '/', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are more places in our codebase where we use displayConfig.afterSignInUrl and displayConfig.afterSignUpUrl. Are those also considered?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After the changes I made the default values will be directly taken from the defaultOptions so the displayConfig.afterSignInUrl or displayConfig.afterSignUpUrl are never used.
7aecc46 to
eae95f2
Compare
35db2db to
0ffd8bf
Compare
42e46a1 to
320b770
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@octoper You will need to apply the changes to the ui-retheme dir. I will share more details within the day
74cc3f8 to
9997c00
Compare
5c30f34 to
15b3a32
Compare
af58073 to
021108b
Compare
packages/clerk-js/src/core/clerk.ts
Outdated
| afterSignInUrl: '/', | ||
| afterSignUpUrl: '/', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to take another look, but my instinct tells me that this is not the direction we want to go as this will cause these to have actual values so, by default, the URLs will include the query params.
What we want to do instead, is to change the behavior of Clerk IF these are undefined
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made the changes to only fallback to / and the values will not be included to the query params.
bf04a09 to
2c10bf3
Compare
…ding Clerk client
2c10bf3 to
59ab8ae
Compare
Description
This PR changes the default behaviour of the following
afterSignOutUrlprop in<UserButton />to be redirected to/instead of the Account Portal defined url.afterSignInandafterSignUpprops to be redirected to/instead of the Account Portal defined url.Clerksingleton as right now they get overwritten from undefined values.@clerk/sharedpackage, that receives an object and returns it back without the properties undefined value.Checklist
npm testruns as expected.npm run buildruns as expected.Type of change
Packages affected
@clerk/backend@clerk/chrome-extension@clerk/clerk-js@clerk/clerk-expo@clerk/fastifygatsby-plugin-clerk@clerk/localizations@clerk/nextjs@clerk/clerk-react@clerk/remix@clerk/clerk-sdk-node@clerk/shared@clerk/themes@clerk/typesbuild/tooling/chore