Skip to content
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

Fix middleware not executed when pages directory is empty #43205

Merged
merged 3 commits into from Nov 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion packages/next/build/entries.ts
Expand Up @@ -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 {
Expand Down
26 changes: 13 additions & 13 deletions packages/next/build/index.ts
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1387,7 +1387,7 @@ export default async function build(

const staticInfo = pagePath
? await getPageStaticInfo({
pageFilePath: join(
pageFilePath: path.join(
(pageType === 'pages' ? pagesDir : appDir) || '',
pagePath
),
Expand Down Expand Up @@ -1982,7 +1982,7 @@ export default async function build(
const cssFilePaths = await new Promise<string[]>((resolve, reject) => {
globOrig(
'**/*.css',
{ cwd: join(distDir, 'static') },
{ cwd: path.join(distDir, 'static') },
(err, files) => {
if (err) {
return reject(err)
Expand Down Expand Up @@ -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)
)
})
}
Expand Down
8 changes: 6 additions & 2 deletions packages/next/build/utils.ts
Expand Up @@ -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`
Expand Down
6 changes: 3 additions & 3 deletions packages/next/cli/next-dev.ts
Expand Up @@ -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,
Expand Down
24 changes: 12 additions & 12 deletions packages/next/server/dev/next-dev-server.ts
Expand Up @@ -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 = [
Expand All @@ -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'),
Expand Down Expand Up @@ -577,7 +576,7 @@ export default class DevServer extends Server {
new NestedMiddlewareError(
nestedMiddleware,
this.dir,
this.pagesDir!
(this.pagesDir || this.appDir)!
).message
)
nestedMiddleware = []
Expand Down Expand Up @@ -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,
Expand Down
44 changes: 39 additions & 5 deletions 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'
Expand All @@ -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',
},
})
})

Expand Down Expand Up @@ -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')
})
})
3 changes: 1 addition & 2 deletions test/e2e/app-dir/app-middleware/next.config.js
@@ -1,7 +1,6 @@
module.exports = {
experimental: {
appDir: true,
legacyBrowsers: false,
browsersListForSwc: true,
allowMiddlewareResponseBody: true,
},
}