fix: use separate watcher script for middleware in dev #1831
Conversation
✅ Deploy Preview for netlify-plugin-nextjs-nx-monorepo-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for netlify-plugin-nextjs-static-root-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for netlify-plugin-nextjs-export-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for next-i18next-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for netlify-plugin-nextjs-next-auth-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for next-plugin-canary ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for nextjs-plugin-custom-routes-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for next-plugin-edge-middleware ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for netlify-plugin-nextjs-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
| } | ||
|
|
||
| console.log('...done') | ||
| } |
There was a problem hiding this comment.
Reading the contents of the update method, I think this could be broken down into several helper functions and then have them be invoked within update:
removeOldMiddlewareFile:
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 logging in the branches of watchedFiles.length === 0 and watchedFiles.length > 1 could be it's own distinct method, with the return statement happening after that logging is done if necessary.
buildMiddlewareFile:
try {
await build({
entryPoints: watchedFiles,
outfile: join(base, '.netlify', 'middleware.js'),
bundle: true,
format: 'esm',
target: 'esnext',
})
} catch (error) {
console.error(error)
return
}
ascorbic
force-pushed
the
mk/dev-watch
branch
from
cb23846 to
6562611
Compare
|
@ericapisani I've added a stack of tests |
ascorbic
force-pushed
the
mk/dev-watch
branch
from
15a877e to
c61caf0
Compare
ascorbic
force-pushed
the
mk/dev-watch
branch
from
46f0b5f to
0f9e28b
Compare
✅ Deploy Preview for netlify-plugin-nextjs-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
I've tested all the scenarios in regards to renaming and moving the middleware file around and it all works as expected. cc: @MarcL. Just looking into the code/tests. |
Co-authored-by: Erica Pisani <[email protected]>
nickytonline
dismissed
ericapisani’s stale review
Erica is no longer on the team.
nickytonline
force-pushed
the
mk/dev-watch
branch
from
fc6c038 to
29dad6f
Compare
| expect(middlewareExists()).toBeTruthy() | ||
| }) | ||
|
|
||
| it.skip('should compile a file if it is written after the watcher starts', async () => { |
There was a problem hiding this comment.
I've created an issue to revisit reneabling the skipped tests.
| expect(middlewareExists()).toBeTruthy() | ||
| }) | ||
|
|
||
| it.skip('should compile a file if it is written after the watcher starts', async () => { |
There was a problem hiding this comment.
I created an issue to reneable the skipped tests, #2005.
Summary
Currently we build middleware in dev by spawning two different esbuild instances, each watching for either middleware.ts or middleware.js. This was always a bit of a hack, and was done because
esbuild --watchdoesn't compile unless all passed files are available. Now that middleware can also be insrc, this would require four instances, which is getting silly. The plan was always to move to a dedicated watcher script and use the esbuild API instead. This PR does that.It uses chokidar to watch for the four valid middleware filenames, and uses esbuild to build them if they change or are created. It deletes the output file if it's removed, and refuses to build if there are more than one.
The big missing part is the same as the current version: there is no way of seeing the build output. There is no way around this that I can see, because of the way netlify/build spawns the plugin commands.
Test plan
ntl devin the middleware demo. Try renaming the middleware.ts file and see.netlify/middleware.jsdisappear. Try creatingmiddleware.jsinstead. Try moving it intosrc.Relevant links (GitHub issues, Notion docs, etc.) or a picture of cute animal
Fixes #1824
Standard checks:
🧪 Once merged, make sure to update the version if needed and that it was published correctly.