Skip to content

Commit

Permalink
Fix middleware not executed when pages directory is empty (vercel#43205)
Browse files Browse the repository at this point in the history
Fixes vercel#41995

Closes vercel#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 <bruno.hn@icloud.com>
  • Loading branch information
2 people authored and jankaifer committed Nov 23, 2022
1 parent 1406516 commit 9f143b4
Show file tree
Hide file tree
Showing 7 changed files with 79 additions and 38 deletions.
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,
},
}

0 comments on commit 9f143b4

Please sign in to comment.