From 8907e1553fc35c5d6b8ed869e1f573629ab2d296 Mon Sep 17 00:00:00 2001 From: Pooya Parsa Date: Sun, 19 Jan 2020 09:34:35 +0100 Subject: [PATCH] feat: HMR support for serverMiddleware (#6881) --- packages/builder/src/builder.js | 78 ++++++++--- packages/builder/test/builder.watch.test.js | 2 - packages/server/src/server.js | 136 ++++++++++++++++---- packages/server/test/server.test.js | 12 +- packages/utils/src/cjs.js | 18 ++- test/fixtures/cli/nuxt.config.js | 2 +- 6 files changed, 193 insertions(+), 55 deletions(-) diff --git a/packages/builder/src/builder.js b/packages/builder/src/builder.js index 4cee71cfd2..1693af5cec 100644 --- a/packages/builder/src/builder.js +++ b/packages/builder/src/builder.js @@ -7,6 +7,7 @@ import hash from 'hash-sum' import pify from 'pify' import upath from 'upath' import semver from 'semver' +import chalk from 'chalk' import debounce from 'lodash/debounce' import omit from 'lodash/omit' @@ -21,10 +22,8 @@ import { waitFor, determineGlobals, stripWhitespace, - isString, isIndexFileAndFolder, - isPureObject, - clearRequireCache + scanRequireTree } from '@nuxt/utils' import Ignore from './ignore' @@ -60,6 +59,9 @@ export default class Builder { this.watchRestart() }) + // Enable HMR for serverMiddleware + this.serverMiddlewareHMR() + // Close hook this.nuxt.hook('close', () => this.close()) } @@ -649,6 +651,9 @@ export default class Builder { assignWatcher (key) { return (watcher) => { + if (this.watchers[key]) { + this.watchers[key].close() + } this.watchers[key] = watcher } } @@ -690,25 +695,61 @@ export default class Builder { this.createFileWatcher([r(this.options.srcDir, this.options.dir.app)], ['add', 'change', 'unlink'], refreshFiles, this.assignWatcher('app')) } - getServerMiddlewarePaths () { - return this.options.serverMiddleware - .map((serverMiddleware) => { - if (isString(serverMiddleware)) { - return serverMiddleware + serverMiddlewareHMR () { + // Check nuxt.server dependency + if (!this.nuxt.server) { + return + } + + // Get registered server middleware with path + const entries = this.nuxt.server.serverMiddlewarePaths() + + // Resolve dependency tree + const deps = new Set() + const dep2Entry = {} + + for (const entry of entries) { + for (const dep of scanRequireTree(entry)) { + deps.add(dep) + if (!dep2Entry[dep]) { + dep2Entry[dep] = new Set() } - if (isPureObject(serverMiddleware) && isString(serverMiddleware.handler)) { - return serverMiddleware.handler + dep2Entry[dep].add(entry) + } + } + + // Create watcher + this.createFileWatcher( + Array.from(deps), + ['all'], + debounce((event, fileName) => { + for (const entry of dep2Entry[fileName]) { + // Reload entry + let newItem + try { + newItem = this.nuxt.server.replaceMiddleware(entry, entry) + } catch (error) { + consola.error(error) + consola.error(`[HMR Error]: ${error}`) + } + + if (!newItem) { + // Full reload if HMR failed + return this.nuxt.callHook('watch:restart', { event, path: fileName }) + } + + // Log + consola.info(`[HMR] ${chalk.cyan(newItem.route)} (${chalk.grey(fileName)})`) } - }) - .filter(Boolean) - .map(p => path.extname(p) ? p : this.nuxt.resolver.resolvePath(p)) + // Tree may be changed so recreate watcher + this.serverMiddlewareHMR() + }, 200), + this.assignWatcher('serverMiddleware') + ) } watchRestart () { - const serverMiddlewarePaths = this.getServerMiddlewarePaths() const nuxtRestartWatch = [ - // Server middleware - ...serverMiddlewarePaths, // Custom watchers ...this.options.watch ].map(this.nuxt.resolver.resolveAlias) @@ -732,11 +773,6 @@ export default class Builder { if (['add', 'change', 'unlink'].includes(event) === false) { return } - /* istanbul ignore if */ - if (serverMiddlewarePaths.includes(fileName)) { - consola.debug(`Clear cache for ${fileName}`) - clearRequireCache(fileName) - } await this.nuxt.callHook('watch:fileChanged', this, fileName) // Legacy await this.nuxt.callHook('watch:restart', { event, path: fileName }) }, diff --git a/packages/builder/test/builder.watch.test.js b/packages/builder/test/builder.watch.test.js index bbe5ce3cb1..46d0245a4c 100644 --- a/packages/builder/test/builder.watch.test.js +++ b/packages/builder/test/builder.watch.test.js @@ -290,8 +290,6 @@ describe('builder: builder watch', () => { expect(chokidar.watch).toBeCalledTimes(1) expect(chokidar.watch).toBeCalledWith( [ - 'resolveAlias(resolvePath(/var/nuxt/src/serverMiddleware/test))', - 'resolveAlias(resolvePath(/var/nuxt/src/serverMiddleware/test-handler))', 'resolveAlias(/var/nuxt/src/watch/test)', '/var/nuxt/src/.nuxtignore', path.join('/var/nuxt/src/var/nuxt/src/store') // because store == false + using path.join() diff --git a/packages/server/src/server.js b/packages/server/src/server.js index 86f9cdfdae..dab8445934 100644 --- a/packages/server/src/server.js +++ b/packages/server/src/server.js @@ -171,40 +171,128 @@ export default class Server { })) } - useMiddleware (middleware) { - let handler = middleware.handler || middleware + _normalizeMiddleware (middleware) { + // Normalize plain function + if (typeof middleware === 'function') { + middleware = { handle: middleware } + } - // Resolve handler setup as string (path) - if (typeof handler === 'string') { - try { - const requiredModuleFromHandlerPath = this.nuxt.resolver.requireModule(handler) + // If a plain string provided as path to middleware + if (typeof middleware === 'string') { + middleware = this._requireMiddleware(middleware) + } - // In case the "handler" is not derived from an object but is a normal string, another object with - // path and handler could be the result + // Normalize handler to handle (backward compatiblity) + if (middleware.handler && !middleware.handle) { + middleware.handle = middleware.handler + delete middleware.handler + } - // If the required module has handler, treat the module as new "middleware" object - if (requiredModuleFromHandlerPath.handler) { - middleware = requiredModuleFromHandlerPath - } + // Normalize path to route (backward compatiblity) + if (middleware.path && !middleware.route) { + middleware.route = middleware.path + delete middleware.path + } - handler = requiredModuleFromHandlerPath.handler || requiredModuleFromHandlerPath - } catch (err) { - consola.error(err) - // Throw error in production mode - if (!this.options.dev) { - throw err - } + // If handle is a string pointing to path + if (typeof middleware.handle === 'string') { + Object.assign(middleware, this._requireMiddleware(middleware.handle)) + } + + // No handle + if (!middleware.handle) { + middleware.handle = (req, res, next) => { + next(new Error('ServerMiddleware should expose a handle: ' + middleware.entry)) } } - // Resolve path - const path = ( + return middleware + } + + _requireMiddleware (entry) { + // Resolve entry + entry = this.nuxt.resolver.resolvePath(entry) + + // Require middleware + let middleware + try { + middleware = this.nuxt.resolver.requireModule(entry) + } catch (error) { + // Show full error + consola.error('ServerMiddleware Error:', error) + + // Placeholder for error + middleware = { + route: '#error', + handle: (req, res, next) => { next(error) } + } + } + + // Normalize + middleware = this._normalizeMiddleware(middleware) + + // Set entry + middleware.entry = entry + + return middleware + } + + resolveMiddleware (middleware) { + // Ensure middleware is normalized + middleware = this._normalizeMiddleware(middleware) + + // Resolve final route + middleware.route = ( (middleware.prefix !== false ? this.options.router.base : '') + - (typeof middleware.path === 'string' ? middleware.path : '') + (typeof middleware.route === 'string' ? middleware.route : '') ).replace(/\/\//g, '/') - // Use middleware - this.app.use(path, handler) + // Assign _middleware to handle to make accessable from app.stack + middleware.handle._middleware = middleware + + return middleware + } + + useMiddleware (middleware) { + const { route, handle } = this.resolveMiddleware(middleware) + this.app.use(route.includes('#error') ? '/' : route, handle) + } + + replaceMiddleware (query, middleware) { + let serverStackItem + + if (typeof query === 'string') { + // Search by entry + serverStackItem = this.app.stack.find(({ handle }) => handle._middleware && handle._middleware.entry === query) + } else { + // Search by reference + serverStackItem = this.app.stack.find(({ handle }) => handle === query) + } + + // Stop if item not found + if (!serverStackItem) { + return + } + + // Resolve middleware + const { route, handle } = this.resolveMiddleware(middleware) + + // Update serverStackItem + serverStackItem.handle = handle + + // Error State + if (route.includes('#error')) { + serverStackItem.route = serverStackItem.route || '/' + } else { + serverStackItem.route = route + } + + // Return updated item + return serverStackItem + } + + serverMiddlewarePaths () { + return this.app.stack.map(({ handle }) => handle._middleware && handle._middleware.entry).filter(Boolean) } renderRoute () { diff --git a/packages/server/test/server.test.js b/packages/server/test/server.test.js index 86aae3ceaf..18f10ae1c7 100644 --- a/packages/server/test/server.test.js +++ b/packages/server/test/server.test.js @@ -61,7 +61,8 @@ describe('server: server', () => { ready: jest.fn(), callHook: jest.fn(), resolver: { - requireModule: jest.fn() + requireModule: jest.fn(), + resolvePath: jest.fn().mockImplementation(p => p) } }) @@ -379,7 +380,7 @@ describe('server: server', () => { expect(server.app.use).toBeCalledWith('/middleware', handler) }) - test('should throw error when module resolves failed', () => { + test('should show error when module require failed', () => { const nuxt = createNuxt() nuxt.options.router = { base: '/' } const server = new Server(nuxt) @@ -388,9 +389,10 @@ describe('server: server', () => { throw error }) - expect(() => server.useMiddleware('test-middleware')).toThrow(error) + server.useMiddleware('test-middleware') + expect(consola.error).toBeCalledTimes(1) - expect(consola.error).toBeCalledWith(error) + expect(consola.error).toBeCalledWith('ServerMiddleware Error:', error) }) test('should only log error when module resolves failed in dev mode', () => { @@ -406,7 +408,7 @@ describe('server: server', () => { server.useMiddleware('test-middleware') expect(consola.error).toBeCalledTimes(1) - expect(consola.error).toBeCalledWith(error) + expect(consola.error).toBeCalledWith('ServerMiddleware Error:', error) }) test('should render route via renderer', () => { diff --git a/packages/utils/src/cjs.js b/packages/utils/src/cjs.js index 8b06c43507..be93e0e50f 100644 --- a/packages/utils/src/cjs.js +++ b/packages/utils/src/cjs.js @@ -3,7 +3,14 @@ export function isExternalDependency (id) { } export function clearRequireCache (id) { - const entry = require.cache[id] + let entry + try { + entry = require.cache[id] + } catch (e) { + delete require.cache[id] + return + } + if (!entry || isExternalDependency(id)) { return } @@ -20,7 +27,14 @@ export function clearRequireCache (id) { } export function scanRequireTree (id, files = new Set()) { - const entry = require.cache[id] + let entry + try { + entry = require.cache[id] + } catch (e) { + files.add(id) + return files + } + if (!entry || isExternalDependency(id) || files.has(id)) { return files } diff --git a/test/fixtures/cli/nuxt.config.js b/test/fixtures/cli/nuxt.config.js index 343897fe33..737cb49a13 100644 --- a/test/fixtures/cli/nuxt.config.js +++ b/test/fixtures/cli/nuxt.config.js @@ -1,6 +1,6 @@ export default { serverMiddleware: [ - '~/middleware.js', + { route: '/empty', handle: '~/middleware.js' }, (req, res, next) => next() ], watch: ['~/custom.file'],