Skip to content

Commit

Permalink
fix: use separate watcher script for middleware in dev (#1831)
Browse files Browse the repository at this point in the history
Co-authored-by: Erica Pisani <pisani.erica@gmail.com>
Co-authored-by: Nick Taylor <nick@iamdeveloper.com>
  • Loading branch information
3 people committed Mar 21, 2023
1 parent 24eaaab commit 92e209f
Show file tree
Hide file tree
Showing 10 changed files with 469 additions and 62 deletions.
4 changes: 4 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ on:
schedule:
- cron: '0 0 * * *'

concurrency:
group: ${{ github.head_ref }}
cancel-in-progress: true

jobs:
build:
name: Unit tests
Expand Down
26 changes: 14 additions & 12 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,8 @@
"\\.[jt]sx?$": "babel-jest"
},
"verbose": true,
"testTimeout": 60000
"testTimeout": 60000,
"maxWorkers": 1
},
"jest-junit": {
"outputDirectory": "reports",
Expand Down
1 change: 1 addition & 0 deletions packages/runtime/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
"@netlify/ipx": "^1.3.3",
"@vercel/node-bridge": "^2.1.0",
"chalk": "^4.1.2",
"chokidar": "^3.5.3",
"destr": "^1.1.1",
"execa": "^5.1.1",
"follow-redirects": "^1.15.2",
Expand Down
125 changes: 125 additions & 0 deletions packages/runtime/src/helpers/compiler.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
import { promises } from 'fs'
import { join } from 'path'

import { build } from '@netlify/esbuild'
import { FSWatcher, watch } from 'chokidar'

// For more information on Next.js middleware, see https://nextjs.org/docs/advanced-features/middleware

// These are the locations that a middleware file can exist in a Next.js application
// If other possible locations are added by Next.js, they should be added here.
const MIDDLEWARE_FILE_LOCATIONS: Readonly<string[]> = [
'middleware.js',
'middleware.ts',
'src/middleware.js',
'src/middleware.ts',
]

const toFileList = (watched: Record<string, Array<string>>) =>
Object.entries(watched).flatMap(([dir, files]) => files.map((file) => join(dir, file)))

/**
* Compile the middleware file using esbuild
*/

const buildMiddlewareFile = async (entryPoints: Array<string>, base: string) => {
try {
await build({
entryPoints,
outfile: join(base, '.netlify', 'middleware.js'),
bundle: true,
format: 'esm',
target: 'esnext',
absWorkingDir: base,
})
} catch (error) {
console.error(error.toString())
}
}

/**
* We only compile middleware if there's exactly one file. If there's more than one, we log a warning and don't compile.
*/
const shouldFilesBeCompiled = (watchedFiles: Array<string>, isFirstRun: boolean) => {
if (watchedFiles.length === 0) {
if (!isFirstRun) {
// Only log on subsequent builds, because having it on first build makes it seem like a warning, when it's a normal state
console.log('No middleware found')
}
return false
}
if (watchedFiles.length > 1) {
console.log('Multiple middleware files found:')
console.log(watchedFiles.join('\n'))
console.log('This is not supported.')
return false
}
return true
}

const updateWatchedFiles = async (watcher: FSWatcher, base: string, isFirstRun = false) => {
try {
// Start by deleting the old file. If we error out, we don't want to leave the old file around
await promises.unlink(join(base, '.netlify', 'middleware.js'))
} catch {
// Ignore, because it's fine if it didn't exist
}
// The list of watched files is an object with the directory as the key and an array of files as the value.
// We need to flatten this into a list of files
const watchedFiles = toFileList(watcher.getWatched())
if (!shouldFilesBeCompiled(watchedFiles, isFirstRun)) {
watcher.emit('build')
return
}
console.log(`${isFirstRun ? 'Building' : 'Rebuilding'} middleware ${watchedFiles[0]}...`)
await buildMiddlewareFile(watchedFiles, base)
console.log('...done')
watcher.emit('build')
}

/**
* Watch for changes to the middleware file location. When a change is detected, recompile the middleware file.
*
* @param base The base directory to watch
* @returns a file watcher and a promise that resolves when the initial scan is complete.
*/
export const watchForMiddlewareChanges = (base: string) => {
const watcher = watch(MIDDLEWARE_FILE_LOCATIONS, {
// Try and ensure renames just emit one event
atomic: true,
// Don't emit for every watched file, just once after the scan is done
ignoreInitial: true,
cwd: base,
})

watcher
.on('change', (path) => {
console.log(`File ${path} has been changed`)
updateWatchedFiles(watcher, base)
})
.on('add', (path) => {
console.log(`File ${path} has been added`)
updateWatchedFiles(watcher, base)
})
.on('unlink', (path) => {
console.log(`File ${path} has been removed`)
updateWatchedFiles(watcher, base)
})

return {
watcher,
isReady: new Promise<void>((resolve) => {
watcher.on('ready', async () => {
console.log('Initial scan for middleware file complete. Ready for changes.')
// This only happens on the first scan
await updateWatchedFiles(watcher, base, true)
console.log('Ready')
resolve()
})
}),
nextBuild: () =>
new Promise<void>((resolve) => {
watcher.once('build', resolve)
}),
}
}
44 changes: 8 additions & 36 deletions packages/runtime/src/helpers/dev.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
import type { Buffer } from 'buffer'
import { resolve } from 'path'
import { Transform } from 'stream'

import { OnPreBuild } from '@netlify/build'
import type { OnPreBuild } from '@netlify/build'
import execa from 'execa'
import { unlink } from 'fs-extra'
import mergeStream from 'merge-stream'

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

// Clean up old functions
await unlink(resolve('.netlify', 'middleware.js')).catch(() => {
// Ignore if it doesn't exist
})
await writeDevEdgeFunction(constants)

// Eventually we might want to do this via esbuild's API, but for now the CLI works fine
const common = [`--bundle`, `--outdir=${resolve('.netlify')}`, `--format=esm`, `--target=esnext`, '--watch']
const opts = {
all: true,
env: { ...process.env, FORCE_COLOR: '1' },
}
// TypeScript
const tsout = execa(`esbuild`, [...common, resolve(base, 'middleware.ts')], opts).all

// JavaScript
const jsout = execa(`esbuild`, [...common, resolve(base, 'middleware.js')], opts).all

const filter = new Transform({
transform(chunk: Buffer, encoding, callback) {
const str = chunk.toString(encoding)

// Skip if message includes this, because we run even when the files are missing
if (!str.includes('[ERROR] Could not resolve')) {
this.push(chunk)
}
callback()
// Don't await this or it will never finish
execa.node(
resolve(__dirname, '..', '..', 'lib', 'helpers', 'middlewareWatcher.js'),
[base, process.env.NODE_ENV === 'test' ? '--once' : ''],
{
stdio: 'inherit',
},
})

mergeStream(tsout, jsout).pipe(filter).pipe(process.stdout)

// Don't return the promise because we don't want to wait for the child process to finish
)
}
13 changes: 13 additions & 0 deletions packages/runtime/src/helpers/middlewareWatcher.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { resolve } from 'path'

import { watchForMiddlewareChanges } from './compiler'

const run = async () => {
const { isReady, watcher } = watchForMiddlewareChanges(resolve(process.argv[2]))
await isReady
if (process.argv[3] === '--once') {
watcher.close()
}
}

run()
2 changes: 1 addition & 1 deletion packages/runtime/src/templates/getPageResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export const getResolverForDependencies = ({
}) => {
const pageFiles = dependencies.map((file) => `require.resolve('${relative(functionDir, file)}')`)
return outdent/* javascript */ `
// This file is purely to allow nft to know about these pages.
// This file is purely to allow nft to know about these pages.
exports.resolvePages = () => {
try {
${pageFiles.join('\n ')}
Expand Down
12 changes: 6 additions & 6 deletions test/__snapshots__/index.spec.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ Array [
`;

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

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

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

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

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

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

0 comments on commit 92e209f

Please sign in to comment.