v5: NextJS improvements#2691
Conversation
🦋 Changeset detectedLatest commit: 4cf05ac The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 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 |
nikosdouvlis
force-pushed
the
nikos/sdk-1247-next-improvements
branch
from
bfc06d1 to
8f74ed3
Compare
panteliselef
left a comment
panteliselef
left a comment
There was a problem hiding this comment.
This looks good to me, Is this still WIP ?
| // TODO @nikos: we need to make this more generic | ||
| // and move the logic in clerk/backend |
There was a problem hiding this comment.
Are you referring to a possible helper named copyHeaders ?
There was a problem hiding this comment.
Yes, but also extracting this to clerk/backend
No :) please review it! |
nikosdouvlis
force-pushed
the
nikos/sdk-1247-next-improvements
branch
2 times, most recently
from
c97c276 to
bcab882
Compare
| import { errorThrower, parsePublishableKey } from './util/shared'; | ||
|
|
||
| const buildUrl = (_baseUrl: string | URL, _targetUrl: string | URL, _returnBackUrl?: string | URL) => { | ||
| const buildUrl = (_baseUrl: string | URL, _targetUrl: string | URL, _returnBackUrl?: string | URL | null) => { |
There was a problem hiding this comment.
❓ why did we prefix all the params with _? I thought we prefix a variable with an underscore to mark it as ignored which is not the case here. Should we rename the params in another PR?
| signInUrl: SIGN_IN_URL, | ||
| signUpUrl: SIGN_UP_URL, | ||
| }).redirectToSignIn; | ||
| const clerkUrl = getAuthKeyFromRequest(request, 'ClerkUrl') as AuthStatus; |
There was a problem hiding this comment.
☁️ clerkUrl is of type AuthStatus ? Is this some kind of typo? Also getAuthKeyFromRequest returns string | null | undefined, so the clerkUrl many not have a .toString() method.
There was a problem hiding this comment.
Good catch @dimkl
TS did not complain as AuthStatus overlaps with string. Let me fix it!
nikosdouvlis
force-pushed
the
nikos/sdk-1247-next-improvements
branch
from
bcab882 to
1fbd905
Compare
…directToSignIn internally
nikosdouvlis
force-pushed
the
nikos/sdk-1247-next-improvements
branch
from
1fbd905 to
4cf05ac
Compare
5 participants
Description
The
auth().redirectToSignIn()helper no longer needs to be explicitly returned when called within the middleware. The following examples are now equivalent:Calling
auth().protect()from a page will now automatically redirect back to the same page by settingredirect_urlto the request url before the redirect to the sign-in URL takes place.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