Skip to content

Commit 92e209f

Browse files
ascorbicericapisaninickytonline
authored
fix: use separate watcher script for middleware in dev (#1831)
Co-authored-by: Erica Pisani <[email protected]> Co-authored-by: Nick Taylor <[email protected]>
1 parent 24eaaab commit 92e209f

File tree

10 files changed

+469
-62
lines changed

10 files changed

+469
-62
lines changed

.github/workflows/test.yml

+4
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ on:
77
schedule:
88
- cron: '0 0 * * *'
99

10+
concurrency:
11+
group: ${{ github.head_ref }}
12+
cancel-in-progress: true
13+
1014
jobs:
1115
build:
1216
name: Unit tests

package-lock.json

+14-12
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

+2-1
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,8 @@
109109
"\\.[jt]sx?$": "babel-jest"
110110
},
111111
"verbose": true,
112-
"testTimeout": 60000
112+
"testTimeout": 60000,
113+
"maxWorkers": 1
113114
},
114115
"jest-junit": {
115116
"outputDirectory": "reports",

packages/runtime/package.json

+1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
"@netlify/ipx": "^1.3.3",
1717
"@vercel/node-bridge": "^2.1.0",
1818
"chalk": "^4.1.2",
19+
"chokidar": "^3.5.3",
1920
"destr": "^1.1.1",
2021
"execa": "^5.1.1",
2122
"follow-redirects": "^1.15.2",
+125
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
import { promises } from 'fs'
2+
import { join } from 'path'
3+
4+
import { build } from '@netlify/esbuild'
5+
import { FSWatcher, watch } from 'chokidar'
6+
7+
// For more information on Next.js middleware, see https://nextjs.org/docs/advanced-features/middleware
8+
9+
// These are the locations that a middleware file can exist in a Next.js application
10+
// If other possible locations are added by Next.js, they should be added here.
11+
const MIDDLEWARE_FILE_LOCATIONS: Readonly<string[]> = [
12+
'middleware.js',
13+
'middleware.ts',
14+
'src/middleware.js',
15+
'src/middleware.ts',
16+
]
17+
18+
const toFileList = (watched: Record<string, Array<string>>) =>
19+
Object.entries(watched).flatMap(([dir, files]) => files.map((file) => join(dir, file)))
20+
21+
/**
22+
* Compile the middleware file using esbuild
23+
*/
24+
25+
const buildMiddlewareFile = async (entryPoints: Array<string>, base: string) => {
26+
try {
27+
await build({
28+
entryPoints,
29+
outfile: join(base, '.netlify', 'middleware.js'),
30+
bundle: true,
31+
format: 'esm',
32+
target: 'esnext',
33+
absWorkingDir: base,
34+
})
35+
} catch (error) {
36+
console.error(error.toString())
37+
}
38+
}
39+
40+
/**
41+
* We only compile middleware if there's exactly one file. If there's more than one, we log a warning and don't compile.
42+
*/
43+
const shouldFilesBeCompiled = (watchedFiles: Array<string>, isFirstRun: boolean) => {
44+
if (watchedFiles.length === 0) {
45+
if (!isFirstRun) {
46+
// Only log on subsequent builds, because having it on first build makes it seem like a warning, when it's a normal state
47+
console.log('No middleware found')
48+
}
49+
return false
50+
}
51+
if (watchedFiles.length > 1) {
52+
console.log('Multiple middleware files found:')
53+
console.log(watchedFiles.join('\n'))
54+
console.log('This is not supported.')
55+
return false
56+
}
57+
return true
58+
}
59+
60+
const updateWatchedFiles = async (watcher: FSWatcher, base: string, isFirstRun = false) => {
61+
try {
62+
// Start by deleting the old file. If we error out, we don't want to leave the old file around
63+
await promises.unlink(join(base, '.netlify', 'middleware.js'))
64+
} catch {
65+
// Ignore, because it's fine if it didn't exist
66+
}
67+
// The list of watched files is an object with the directory as the key and an array of files as the value.
68+
// We need to flatten this into a list of files
69+
const watchedFiles = toFileList(watcher.getWatched())
70+
if (!shouldFilesBeCompiled(watchedFiles, isFirstRun)) {
71+
watcher.emit('build')
72+
return
73+
}
74+
console.log(`${isFirstRun ? 'Building' : 'Rebuilding'} middleware ${watchedFiles[0]}...`)
75+
await buildMiddlewareFile(watchedFiles, base)
76+
console.log('...done')
77+
watcher.emit('build')
78+
}
79+
80+
/**
81+
* Watch for changes to the middleware file location. When a change is detected, recompile the middleware file.
82+
*
83+
* @param base The base directory to watch
84+
* @returns a file watcher and a promise that resolves when the initial scan is complete.
85+
*/
86+
export const watchForMiddlewareChanges = (base: string) => {
87+
const watcher = watch(MIDDLEWARE_FILE_LOCATIONS, {
88+
// Try and ensure renames just emit one event
89+
atomic: true,
90+
// Don't emit for every watched file, just once after the scan is done
91+
ignoreInitial: true,
92+
cwd: base,
93+
})
94+
95+
watcher
96+
.on('change', (path) => {
97+
console.log(`File ${path} has been changed`)
98+
updateWatchedFiles(watcher, base)
99+
})
100+
.on('add', (path) => {
101+
console.log(`File ${path} has been added`)
102+
updateWatchedFiles(watcher, base)
103+
})
104+
.on('unlink', (path) => {
105+
console.log(`File ${path} has been removed`)
106+
updateWatchedFiles(watcher, base)
107+
})
108+
109+
return {
110+
watcher,
111+
isReady: new Promise<void>((resolve) => {
112+
watcher.on('ready', async () => {
113+
console.log('Initial scan for middleware file complete. Ready for changes.')
114+
// This only happens on the first scan
115+
await updateWatchedFiles(watcher, base, true)
116+
console.log('Ready')
117+
resolve()
118+
})
119+
}),
120+
nextBuild: () =>
121+
new Promise<void>((resolve) => {
122+
watcher.once('build', resolve)
123+
}),
124+
}
125+
}

packages/runtime/src/helpers/dev.ts

+8-36
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,7 @@
1-
import type { Buffer } from 'buffer'
21
import { resolve } from 'path'
3-
import { Transform } from 'stream'
42

5-
import { OnPreBuild } from '@netlify/build'
3+
import type { OnPreBuild } from '@netlify/build'
64
import execa from 'execa'
7-
import { unlink } from 'fs-extra'
8-
import mergeStream from 'merge-stream'
95

106
import { writeDevEdgeFunction } from './edge'
117
import { patchNextFiles } from './files'
@@ -17,37 +13,13 @@ export const onPreDev: OnPreBuild = async ({ constants, netlifyConfig }) => {
1713
// Need to patch the files, because build might not have been run
1814
await patchNextFiles(base)
1915

20-
// Clean up old functions
21-
await unlink(resolve('.netlify', 'middleware.js')).catch(() => {
22-
// Ignore if it doesn't exist
23-
})
2416
await writeDevEdgeFunction(constants)
25-
26-
// Eventually we might want to do this via esbuild's API, but for now the CLI works fine
27-
const common = [`--bundle`, `--outdir=${resolve('.netlify')}`, `--format=esm`, `--target=esnext`, '--watch']
28-
const opts = {
29-
all: true,
30-
env: { ...process.env, FORCE_COLOR: '1' },
31-
}
32-
// TypeScript
33-
const tsout = execa(`esbuild`, [...common, resolve(base, 'middleware.ts')], opts).all
34-
35-
// JavaScript
36-
const jsout = execa(`esbuild`, [...common, resolve(base, 'middleware.js')], opts).all
37-
38-
const filter = new Transform({
39-
transform(chunk: Buffer, encoding, callback) {
40-
const str = chunk.toString(encoding)
41-
42-
// Skip if message includes this, because we run even when the files are missing
43-
if (!str.includes('[ERROR] Could not resolve')) {
44-
this.push(chunk)
45-
}
46-
callback()
17+
// Don't await this or it will never finish
18+
execa.node(
19+
resolve(__dirname, '..', '..', 'lib', 'helpers', 'middlewareWatcher.js'),
20+
[base, process.env.NODE_ENV === 'test' ? '--once' : ''],
21+
{
22+
stdio: 'inherit',
4723
},
48-
})
49-
50-
mergeStream(tsout, jsout).pipe(filter).pipe(process.stdout)
51-
52-
// Don't return the promise because we don't want to wait for the child process to finish
24+
)
5325
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import { resolve } from 'path'
2+
3+
import { watchForMiddlewareChanges } from './compiler'
4+
5+
const run = async () => {
6+
const { isReady, watcher } = watchForMiddlewareChanges(resolve(process.argv[2]))
7+
await isReady
8+
if (process.argv[3] === '--once') {
9+
watcher.close()
10+
}
11+
}
12+
13+
run()

packages/runtime/src/templates/getPageResolver.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ export const getResolverForDependencies = ({
3434
}) => {
3535
const pageFiles = dependencies.map((file) => `require.resolve('${relative(functionDir, file)}')`)
3636
return outdent/* javascript */ `
37-
// This file is purely to allow nft to know about these pages.
37+
// This file is purely to allow nft to know about these pages.
3838
exports.resolvePages = () => {
3939
try {
4040
${pageFiles.join('\n ')}

test/__snapshots__/index.spec.js.snap

+6-6
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ Array [
5858
`;
5959

6060
exports[`onBuild() generates a file referencing all API route sources: for _api_hello-background-background 1`] = `
61-
"// This file is purely to allow nft to know about these pages.
61+
"// This file is purely to allow nft to know about these pages.
6262
exports.resolvePages = () => {
6363
try {
6464
require.resolve('../../../.next/package.json')
@@ -77,7 +77,7 @@ exports.resolvePages = () => {
7777
`;
7878

7979
exports[`onBuild() generates a file referencing all API route sources: for _api_hello-scheduled-handler 1`] = `
80-
"// This file is purely to allow nft to know about these pages.
80+
"// This file is purely to allow nft to know about these pages.
8181
exports.resolvePages = () => {
8282
try {
8383
require.resolve('../../../.next/package.json')
@@ -96,7 +96,7 @@ exports.resolvePages = () => {
9696
`;
9797

9898
exports[`onBuild() generates a file referencing all page sources 1`] = `
99-
"// This file is purely to allow nft to know about these pages.
99+
"// This file is purely to allow nft to know about these pages.
100100
exports.resolvePages = () => {
101101
try {
102102
require.resolve('../../../.next/package.json')
@@ -156,7 +156,7 @@ exports.resolvePages = () => {
156156
`;
157157

158158
exports[`onBuild() generates a file referencing all page sources 2`] = `
159-
"// This file is purely to allow nft to know about these pages.
159+
"// This file is purely to allow nft to know about these pages.
160160
exports.resolvePages = () => {
161161
try {
162162
require.resolve('../../../.next/package.json')
@@ -216,7 +216,7 @@ exports.resolvePages = () => {
216216
`;
217217

218218
exports[`onBuild() generates a file referencing all when publish dir is a subdirectory 1`] = `
219-
"// This file is purely to allow nft to know about these pages.
219+
"// This file is purely to allow nft to know about these pages.
220220
exports.resolvePages = () => {
221221
try {
222222
require.resolve('../../../web/.next/package.json')
@@ -276,7 +276,7 @@ exports.resolvePages = () => {
276276
`;
277277

278278
exports[`onBuild() generates a file referencing all when publish dir is a subdirectory 2`] = `
279-
"// This file is purely to allow nft to know about these pages.
279+
"// This file is purely to allow nft to know about these pages.
280280
exports.resolvePages = () => {
281281
try {
282282
require.resolve('../../../web/.next/package.json')

0 commit comments

Comments
 (0)