From 9f143b45fb51fa5552f6f6b0807c173328b2aec4 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Tue, 22 Nov 2022 00:37:58 +0100 Subject: [PATCH] Fix middleware not executed when pages directory is empty (#43205) Fixes #41995 Closes #43144 ## Bug - [x] Related issues linked using `fixes #number` - [x] Integration tests added - [ ] Errors have a helpful link attached, see `contributing.md` Co-authored-by: Bruno Nascimento --- packages/next/build/entries.ts | 6 ++- packages/next/build/index.ts | 26 +++++------ packages/next/build/utils.ts | 8 +++- packages/next/cli/next-dev.ts | 6 +-- packages/next/server/dev/next-dev-server.ts | 24 +++++----- test/e2e/app-dir/app-middleware.test.ts | 44 ++++++++++++++++--- .../e2e/app-dir/app-middleware/next.config.js | 3 +- 7 files changed, 79 insertions(+), 38 deletions(-) diff --git a/packages/next/build/entries.ts b/packages/next/build/entries.ts index e6f5fc1d11d8e..57305cc155669 100644 --- a/packages/next/build/entries.ts +++ b/packages/next/build/entries.ts @@ -457,7 +457,11 @@ export async function createEntrypoints(params: CreateEntrypointsParams) { await Promise.all(Object.keys(pages).map(getEntryHandler(pages, 'pages'))) if (nestedMiddleware.length > 0) { - throw new NestedMiddlewareError(nestedMiddleware, rootDir, pagesDir!) + throw new NestedMiddlewareError( + nestedMiddleware, + rootDir, + (appDir || pagesDir)! + ) } return { diff --git a/packages/next/build/index.ts b/packages/next/build/index.ts index fbdf356c28a47..5ea05ac8cec21 100644 --- a/packages/next/build/index.ts +++ b/packages/next/build/index.ts @@ -11,7 +11,7 @@ import { escapeStringRegexp } from '../shared/lib/escape-regexp' import findUp from 'next/dist/compiled/find-up' import { nanoid } from 'next/dist/compiled/nanoid/index.cjs' import { pathToRegexp } from 'next/dist/compiled/path-to-regexp' -import path, { join } from 'path' +import path from 'path' import formatWebpackMessages from '../client/dev/error-overlay/format-webpack-messages' import { STATIC_STATUS_PAGE_GET_INITIAL_PROPS_ERROR, @@ -331,15 +331,16 @@ export default async function build( }) const { pagesDir, appDir } = findPagesDir(dir, isAppDirEnabled) + const isSrcDir = path + .relative(dir, pagesDir || appDir || '') + .startsWith('src') const hasPublicDir = await fileExists(publicDir) telemetry.record( eventCliSession(dir, config, { webpackVersion: 5, cliCommand: 'build', - isSrcDir: - (!!pagesDir && path.relative(dir, pagesDir).startsWith('src')) || - (!!appDir && path.relative(dir, appDir).startsWith('src')), + isSrcDir, hasNowJson: !!(await findUp('now.json', { cwd: dir })), isCustomServer: null, turboFlag: false, @@ -502,11 +503,10 @@ export default async function build( `^${MIDDLEWARE_FILENAME}\\.(?:${config.pageExtensions.join('|')})$` ) - const rootPaths = pagesDir - ? ( - await flatReaddir(join(pagesDir, '..'), middlewareDetectionRegExp) - ).map((absoluteFile) => absoluteFile.replace(dir, '')) - : [] + const rootDir = path.join((pagesDir || appDir)!, '..') + const rootPaths = ( + await flatReaddir(rootDir, middlewareDetectionRegExp) + ).map((absoluteFile) => absoluteFile.replace(dir, '')) // needed for static exporting since we want to replace with HTML // files @@ -1306,7 +1306,7 @@ export default async function build( config.experimental.gzipSize ) - const middlewareManifest: MiddlewareManifest = require(join( + const middlewareManifest: MiddlewareManifest = require(path.join( distDir, SERVER_DIRECTORY, MIDDLEWARE_MANIFEST @@ -1387,7 +1387,7 @@ export default async function build( const staticInfo = pagePath ? await getPageStaticInfo({ - pageFilePath: join( + pageFilePath: path.join( (pageType === 'pages' ? pagesDir : appDir) || '', pagePath ), @@ -1982,7 +1982,7 @@ export default async function build( const cssFilePaths = await new Promise((resolve, reject) => { globOrig( '**/*.css', - { cwd: join(distDir, 'static') }, + { cwd: path.join(distDir, 'static') }, (err, files) => { if (err) { return reject(err) @@ -2803,7 +2803,7 @@ export default async function build( .traceAsyncFn(async () => { await verifyPartytownSetup( dir, - join(distDir, CLIENT_STATIC_FILES_PATH) + path.join(distDir, CLIENT_STATIC_FILES_PATH) ) }) } diff --git a/packages/next/build/utils.ts b/packages/next/build/utils.ts index 6b20468b23398..aea1ace28871c 100644 --- a/packages/next/build/utils.ts +++ b/packages/next/build/utils.ts @@ -1774,13 +1774,17 @@ export function getPossibleMiddlewareFilenames( } export class NestedMiddlewareError extends Error { - constructor(nestedFileNames: string[], mainDir: string, pagesDir: string) { + constructor( + nestedFileNames: string[], + mainDir: string, + pagesOrAppDir: string + ) { super( `Nested Middleware is not allowed, found:\n` + `${nestedFileNames.map((file) => `pages${file}`).join('\n')}\n` + `Please move your code to a single file at ${path.join( path.posix.sep, - path.relative(mainDir, path.resolve(pagesDir, '..')), + path.relative(mainDir, path.resolve(pagesOrAppDir, '..')), 'middleware' )} instead.\n` + `Read More - https://nextjs.org/docs/messages/nested-middleware` diff --git a/packages/next/cli/next-dev.ts b/packages/next/cli/next-dev.ts index 1f0fc1ab875db..b0a689c84f660 100755 --- a/packages/next/cli/next-dev.ts +++ b/packages/next/cli/next-dev.ts @@ -380,9 +380,9 @@ If you cannot make the changes above, but still want to try out\nNext.js v13 wit eventCliSession(distDir, rawNextConfig as NextConfigComplete, { webpackVersion: 5, cliCommand: 'dev', - isSrcDir: - (!!pagesDir && path.relative(dir, pagesDir).startsWith('src')) || - (!!appDir && path.relative(dir, appDir).startsWith('src')), + isSrcDir: path + .relative(dir, pagesDir || appDir || '') + .startsWith('src'), hasNowJson: !!(await findUp('now.json', { cwd: dir })), isCustomServer: false, turboFlag: true, diff --git a/packages/next/server/dev/next-dev-server.ts b/packages/next/server/dev/next-dev-server.ts index 389f283c367fa..967d7103a58d6 100644 --- a/packages/next/server/dev/next-dev-server.ts +++ b/packages/next/server/dev/next-dev-server.ts @@ -277,12 +277,11 @@ export default class DevServer extends Server { const app = this.appDir ? [this.appDir] : [] const directories = [...pages, ...app] - const files = this.pagesDir - ? getPossibleMiddlewareFilenames( - pathJoin(this.pagesDir, '..'), - this.nextConfig.pageExtensions - ) - : [] + const rootDir = this.pagesDir || this.appDir + const files = getPossibleMiddlewareFilenames( + pathJoin(rootDir!, '..'), + this.nextConfig.pageExtensions + ) let nestedMiddleware: string[] = [] const envFiles = [ @@ -294,7 +293,7 @@ export default class DevServer extends Server { files.push(...envFiles) - // tsconfig/jsonfig paths hot-reloading + // tsconfig/jsconfig paths hot-reloading const tsconfigPaths = [ pathJoin(this.dir, 'tsconfig.json'), pathJoin(this.dir, 'jsconfig.json'), @@ -577,7 +576,7 @@ export default class DevServer extends Server { new NestedMiddlewareError( nestedMiddleware, this.dir, - this.pagesDir! + (this.pagesDir || this.appDir)! ).message ) nestedMiddleware = [] @@ -726,14 +725,15 @@ export default class DevServer extends Server { } const telemetry = new Telemetry({ distDir: this.distDir }) + const isSrcDir = relative( + this.dir, + this.pagesDir || this.appDir || '' + ).startsWith('src') telemetry.record( eventCliSession(this.distDir, this.nextConfig, { webpackVersion: 5, cliCommand: 'dev', - isSrcDir: - (!!this.pagesDir && - relative(this.dir, this.pagesDir).startsWith('src')) || - (!!this.appDir && relative(this.dir, this.appDir).startsWith('src')), + isSrcDir, hasNowJson: !!(await findUp('now.json', { cwd: this.dir })), isCustomServer: this.isCustomServer, turboFlag: false, diff --git a/test/e2e/app-dir/app-middleware.test.ts b/test/e2e/app-dir/app-middleware.test.ts index 91b79a72f52af..4c1caccd53d8a 100644 --- a/test/e2e/app-dir/app-middleware.test.ts +++ b/test/e2e/app-dir/app-middleware.test.ts @@ -1,7 +1,7 @@ /* eslint-env jest */ import { NextInstance } from 'test/lib/next-modes/base' -import { fetchViaHTTP } from 'next-test-utils' +import { fetchViaHTTP, renderViaHTTP } from 'next-test-utils' import { createNext, FileRef } from 'e2e-utils' import cheerio from 'cheerio' import path from 'path' @@ -18,10 +18,6 @@ describe('app-dir with middleware', () => { beforeAll(async () => { next = await createNext({ files: new FileRef(path.join(__dirname, 'app-middleware')), - dependencies: { - react: 'experimental', - 'react-dom': 'experimental', - }, }) }) @@ -122,3 +118,41 @@ describe('app-dir with middleware', () => { }) }) }) + +describe('app dir middleware without pages dir', () => { + if ((global as any).isNextDeploy) { + it('should skip next deploy for now', () => {}) + return + } + + let next: NextInstance + + afterAll(() => next.destroy()) + beforeAll(async () => { + next = await createNext({ + files: { + app: new FileRef(path.join(__dirname, 'app-middleware/app')), + 'next.config.js': new FileRef( + path.join(__dirname, 'app-middleware/next.config.js') + ), + 'middleware.js': ` + import { NextResponse } from 'next/server' + + export async function middleware(request) { + return new NextResponse('redirected') + } + + export const config = { + matcher: '/headers' + } + `, + }, + }) + }) + + it(`Updates headers`, async () => { + const html = await renderViaHTTP(next.url, '/headers') + + expect(html).toContain('redirected') + }) +}) diff --git a/test/e2e/app-dir/app-middleware/next.config.js b/test/e2e/app-dir/app-middleware/next.config.js index a928ea943ce24..4ab660b86b151 100644 --- a/test/e2e/app-dir/app-middleware/next.config.js +++ b/test/e2e/app-dir/app-middleware/next.config.js @@ -1,7 +1,6 @@ module.exports = { experimental: { appDir: true, - legacyBrowsers: false, - browsersListForSwc: true, + allowMiddlewareResponseBody: true, }, }