fix(nextjs): prevent infinite handshake redirect loop on Netlify#7857
fix(nextjs): prevent infinite handshake redirect loop on Netlify#7857mauricioabreu wants to merge 2 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: be3ca3a The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
wobsoriano
left a comment
There was a problem hiding this comment.
This is looking good. Just a minor change! Thankss
| publishableKey: requestState.publishableKey, | ||
| }); | ||
|
|
||
| const res = NextResponse.redirect(requestState.headers.get(constants.Headers.Location) || locationHeader); |
There was a problem hiding this comment.
We can just use locationHeader here 👀
There was a problem hiding this comment.
I don't think so? this code looks a bit different from the other frameworks
I read the other framework's implementation, and the response is built with Response(null, { status: 307, headers: requestState.headers })
In Next.js, we use NextResponse.redirect(url) and explicitly skip copying the Location header.
I mean, if we reused the locationHeader value captured before the mutation, the param added by handleNetlifyCacheInDevInstance is lost
There was a problem hiding this comment.
Copy. Looks like I have to revisit the helper. Thanks!
There was a problem hiding this comment.
thanks for correcting me here! I just checked and saw we're updating the Location header so your implementation is perfect!
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughThis pull request introduces a fix for an infinite handshake redirect loop that occurs when deploying Next.js applications with Clerk development instances to Netlify. The changes add Netlify cache handling in development mode by importing and invoking 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
Description
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
Release Notes