-
Notifications
You must be signed in to change notification settings - Fork 91
fix: fail build/deploy when using not yet unsupported Node.js Midleware #3016
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
Changes from all commits
ff5ee51
81552f7
3609d31
88febdd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,7 @@ import { trace } from '@opentelemetry/api' | |
import { wrapTracer } from '@opentelemetry/api/experimental' | ||
import glob from 'fast-glob' | ||
import type { MiddlewareManifest } from 'next/dist/build/webpack/plugins/middleware-plugin.js' | ||
import type { FunctionsConfigManifest } from 'next-with-cache-handler-v2/dist/build/index.js' | ||
import { prerelease, satisfies, lt as semverLowerThan, lte as semverLowerThanOrEqual } from 'semver' | ||
|
||
import type { RunConfig } from '../../run/config.js' | ||
|
@@ -131,6 +132,10 @@ export const copyNextServerCode = async (ctx: PluginContext): Promise<void> => { | |
return | ||
} | ||
|
||
if (path === 'server/functions-config-manifest.json') { | ||
await verifyFunctionsConfigManifest(join(srcDir, path)) | ||
} | ||
|
||
await cp(srcPath, destPath, { recursive: true, force: true }) | ||
}), | ||
) | ||
|
@@ -376,6 +381,22 @@ const replaceMiddlewareManifest = async (sourcePath: string, destPath: string) = | |
await writeFile(destPath, newData) | ||
} | ||
|
||
const verifyFunctionsConfigManifest = async (sourcePath: string) => { | ||
const data = await readFile(sourcePath, 'utf8') | ||
const manifest = JSON.parse(data) as FunctionsConfigManifest | ||
|
||
// https://github.com/vercel/next.js/blob/8367faedd61501025299e92d43a28393c7bb50e2/packages/next/src/build/index.ts#L2465 | ||
// Node.js Middleware has hardcoded /_middleware path | ||
if (manifest.functions['/_middleware']) { | ||
Comment on lines
+388
to
+390
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oof, there's no more explicit way to check this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. :S It's literally what Next.js uses internally as well https://github.com/search?q=repo%3Avercel%2Fnext.js%20%22%2F_middleware%22&type=code There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know of any better way, so I just followed what Next.js does There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, now that I think of it - possibly "nicer" way would be to actually use adapters api to get list of "outputs" where middleware should exist and we could find middleware entry and we could check, but adapters api is definitely not stable, so I wouldn't want to actually use it in production yet, but that's the only idea that comes to mind that doesn't involve checks like above |
||
throw new Error( | ||
'Node.js middleware is not yet supported.\n\n' + | ||
'Future @netlify/plugin-nextjs release will support node middleware with following limitations:\n' + | ||
' - usage of C++ Addons (https://nodejs.org/api/addons.html) not supported (for example `bcrypt` npm module will not be supported, but `bcryptjs` will be supported),\n' + | ||
' - usage of Filesystem (https://nodejs.org/api/fs.html) not supported.', | ||
) | ||
} | ||
} | ||
|
||
export const verifyHandlerDirStructure = async (ctx: PluginContext) => { | ||
const { nextConfig } = JSON.parse( | ||
await readFile(join(ctx.serverHandlerDir, RUN_CONFIG_FILE), 'utf-8'), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
export const metadata = { | ||
title: 'Simple Next App', | ||
description: 'Description for Simple Next App', | ||
} | ||
|
||
export default function RootLayout({ children }) { | ||
return ( | ||
<html lang="en"> | ||
<body>{children}</body> | ||
</html> | ||
) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
export default function Home() { | ||
return ( | ||
<main> | ||
<h1>Home</h1> | ||
</main> | ||
) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
import type { NextRequest } from 'next/server' | ||
|
||
export async function middleware(request: NextRequest) { | ||
console.log('Node.js Middleware request:', request.method, request.nextUrl.pathname) | ||
} | ||
|
||
export const config = { | ||
runtime: 'nodejs', | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
/** @type {import('next').NextConfig} */ | ||
const nextConfig = { | ||
output: 'standalone', | ||
eslint: { | ||
ignoreDuringBuilds: true, | ||
}, | ||
experimental: { | ||
nodeMiddleware: true, | ||
}, | ||
} | ||
|
||
module.exports = nextConfig |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
{ | ||
"name": "middleware-node", | ||
"version": "0.1.0", | ||
"private": true, | ||
"scripts": { | ||
"postinstall": "next build", | ||
"dev": "next dev", | ||
"build": "next build" | ||
}, | ||
"dependencies": { | ||
"next": "latest", | ||
"react": "18.2.0", | ||
"react-dom": "18.2.0" | ||
}, | ||
"test": { | ||
"dependencies": { | ||
"next": ">=15.2.0" | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,9 +33,9 @@ export function isNextCanary() { | |
export function shouldHaveAppRouterNotFoundInPrerenderManifest() { | ||
// https://github.com/vercel/next.js/pull/82199 | ||
|
||
// The canary versions are out of band, as there stable/latest patch versions higher than base of canary versions | ||
// and change was not backported to stable versions | ||
return nextVersionSatisfies('>=15.4.2-canary.33') && isNextCanary() | ||
// The canary versions are out of band, as there stable/latest patch versions higher than base of canary versions without | ||
// the change included | ||
return nextVersionSatisfies(isNextCanary() ? '>=15.4.2-canary.33' : '>=15.5.0') | ||
Comment on lines
-36
to
+38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not related to Node.js middleware, but rather new stable release - just adding it here to avoid separate PR that would slow down merging things in |
||
} | ||
|
||
/** | ||
|