-
Notifications
You must be signed in to change notification settings - Fork 408
chore(nextjs): Update middleware check for proxy usage #7269
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
base: main
Are you sure you want to change the base?
chore(nextjs): Update middleware check for proxy usage #7269
Conversation
🦋 Changeset detectedLatest commit: 26f15d6 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.
|
WalkthroughAdds Next.js 16+ detection and uses it to treat Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as CLI / runtime
participant SDK as packages/nextjs
participant FS as filesystem search
rect rgb(230,245,255)
Note over SDK: Determine Next.js major version
SDK->>SDK: evaluate isNext16OrHigher
end
alt Next.js >= 16
SDK->>FS: search for files in ['middleware','proxy'] x ['.ts','.js']
else Next.js < 16
SDK->>FS: search for files in ['middleware'] x ['.ts','.js']
end
FS-->>SDK: return first match or none
alt found
SDK->>Caller: show suggestion message including found fileName and path
else not found
SDK->>Caller: show suggestion message referencing expected fileName(s)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
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.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/nextjs/src/app-router/server/auth.ts(2 hunks)packages/nextjs/src/server/fs/middleware-location.ts(3 hunks)packages/nextjs/src/utils/sdk-versions.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/nextjs/src/app-router/server/auth.ts (1)
packages/nextjs/src/utils/sdk-versions.ts (1)
isNext16OrHigher(16-16)
packages/nextjs/src/server/fs/middleware-location.ts (3)
packages/nextjs/src/utils/sdk-versions.ts (1)
isNext16OrHigher(16-16)packages/nextjs/src/runtime/node/safe-node-apis.js (1)
path(7-7)packages/nextjs/src/runtime/browser/safe-node-apis.js (1)
path(5-5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (7)
packages/nextjs/src/app-router/server/auth.ts (2)
14-14: LGTM!The import of
isNext16OrHigheris correctly added alongside the existingisNextWithUnstableServerActionsimport.
84-85: LGTM!The conditional filename logic correctly adapts the error message based on the Next.js version, guiding users to the appropriate middleware or proxy file location.
packages/nextjs/src/utils/sdk-versions.ts (1)
16-16: LGTM!The export statement correctly includes
isNext16OrHigheralongside the existing version detection exports.packages/nextjs/src/server/fs/middleware-location.ts (4)
1-1: LGTM!The import of
isNext16OrHigheris correctly added to enable Next.js version-aware file detection.
16-18: LGTM!The conditional logic correctly handles both middleware and proxy files for Next.js 16+ while maintaining backward compatibility. The inline comment clearly explains the dual-runtime support.
20-26: LGTM!The
suggestionMessagefunction is correctly updated to accept a dynamicfileNameparameter and usesfileNameDisplayfor user-facing generic references while using the specificfileNamein the file path. This properly supports both middleware and proxy files.
35-48: LGTM!The
checkMiddlewareLocationfunction correctly iterates over both file names and extensions, returning a suggestion message with the actual filename on the first match. The nested loop structure ensures all valid combinations are checked.
| /** | ||
| * Next.js 16+ renamed middleware.ts to proxy.ts | ||
| */ | ||
| const isNext16OrHigher = nextPkg.version.startsWith('16.') || parseInt(nextPkg.version.split('.')[0]) >= 16; |
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.
Correct the misleading JSDoc comment.
The JSDoc states that Next.js 16+ "renamed" middleware.ts to proxy.ts, but this is contradicted by the implementation in middleware-location.ts (line 16), which explicitly states that both files are supported: "Next.js 16+ supports both middleware.ts (Edge runtime) and proxy.ts (Node.js runtime)". The code also checks for both filenames in the array ['middleware', 'proxy'].
Update the JSDoc to accurately reflect that both files are supported:
/**
- * Next.js 16+ renamed middleware.ts to proxy.ts
+ * Next.js 16+ introduced proxy.ts (Node.js runtime) alongside middleware.ts (Edge runtime)
*/
const isNext16OrHigher = nextPkg.version.startsWith('16.') || parseInt(nextPkg.version.split('.')[0]) >= 16;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Next.js 16+ renamed middleware.ts to proxy.ts | |
| */ | |
| const isNext16OrHigher = nextPkg.version.startsWith('16.') || parseInt(nextPkg.version.split('.')[0]) >= 16; | |
| /** | |
| * Next.js 16+ introduced proxy.ts (Node.js runtime) alongside middleware.ts (Edge runtime) | |
| */ | |
| const isNext16OrHigher = nextPkg.version.startsWith('16.') || parseInt(nextPkg.version.split('.')[0]) >= 16; |
🤖 Prompt for AI Agents
In packages/nextjs/src/utils/sdk-versions.ts around lines 11 to 14, the JSDoc
incorrectly states that Next.js 16+ "renamed middleware.ts to proxy.ts"; update
the comment to accurately state that Next.js 16+ supports both middleware.ts
(Edge runtime) and proxy.ts (Node.js runtime) or that both filenames are
supported, so it aligns with middleware-location.ts and the code that checks
['middleware','proxy'].
|
!snapshot |
|
Hey @alexcarpenter - the snapshot version command generated the following package versions:
Tip: Use the snippet copy button below to quickly install the required packages. npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact
npm i @clerk/[email protected] --save-exact |
| /** | ||
| * Next.js 16+ renamed middleware.ts to proxy.ts | ||
| */ | ||
| const isNext16OrHigher = nextPkg.version.startsWith('16.') || parseInt(nextPkg.version.split('.')[0]) >= 16; |
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.
We should add tests for this boolean expression (it kind of feels like a function)
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.
Also, we can make it a bit more robust for the future:
function meetsNextMinimumVersion(nextPkg, minimumMajorVersion = 16) {
if (!nextPkg?.version) {
return false;
}
const majorVersion = parseInt(nextPkg.version.split('.')[0], 10);
return !isNaN(majorVersion) && majorVersion >= minimumMajorVersion;
}
const isNext16OrHigher = meetsNextMinimumVersion(nextPkg, 16);
Description
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.