From 3e9eee2549b9886c751f8b5aad4cc88e8f68908a Mon Sep 17 00:00:00 2001 From: Pim Date: Fri, 8 Feb 2019 11:06:47 +0100 Subject: [PATCH] fix: dont force exit when it was explicitly disabled (#4973) * fix: remove slash from warning text * fix: dont force-exit when explicitly disabled chore: add tests for force-exit behaviour * feat: default option value can be fn --- packages/cli/src/command.js | 21 +++++---- packages/cli/src/options/common.js | 7 +-- packages/cli/src/utils/index.js | 4 +- .../unit/__snapshots__/command.test.js.snap | 7 ++- packages/cli/test/unit/build.test.js | 32 +++++++++++++ packages/cli/test/unit/cli.test.js | 2 +- packages/cli/test/unit/dev.test.js | 32 ++++++++++++- packages/cli/test/unit/generate.test.js | 45 +++++++++++++++++++ packages/cli/test/unit/start.test.js | 29 +++++++++++- packages/cli/test/utils/mocking.js | 4 +- 10 files changed, 161 insertions(+), 22 deletions(-) diff --git a/packages/cli/src/command.js b/packages/cli/src/command.js index ec28dea766..0f1d4bb74d 100644 --- a/packages/cli/src/command.js +++ b/packages/cli/src/command.js @@ -13,9 +13,6 @@ export default class NuxtCommand { } this.cmd = cmd - // If the cmd is a server then dont forcibly exit when the cmd has finished - this.isServer = cmd.isServer !== undefined ? cmd.isServer : Boolean(this.cmd.options.hostname) - this._argv = Array.from(argv) this._parsedArgv = null // Lazy evaluate } @@ -48,9 +45,9 @@ export default class NuxtCommand { const runResolve = Promise.resolve(this.cmd.run(this)) - // TODO: For v3 set timeout to 0 when force-exit === true - if (!this.isServer || this.argv['force-exit']) { - runResolve.then(() => forceExit(this.cmd.name, forceExitTimeout)) + if (this.argv['force-exit']) { + const forceExitByUser = this.isUserSuppliedArg('force-exit') + runResolve.then(() => forceExit(this.cmd.name, forceExitByUser ? false : forceExitTimeout)) } return runResolve @@ -102,6 +99,14 @@ export default class NuxtCommand { return new Generator(nuxt, builder) } + isUserSuppliedArg(option) { + return this._argv.includes(`--${option}`) || this._argv.includes(`--no-${option}`) + } + + _getDefaultOptionValue(option) { + return typeof option.default === 'function' ? option.default(this.cmd) : option.default + } + _getMinimistOptions() { const minimistOptions = { alias: {}, @@ -120,7 +125,7 @@ export default class NuxtCommand { minimistOptions[option.type].push(option.alias || name) } if (option.default) { - minimistOptions.default[option.alias || name] = option.default + minimistOptions.default[option.alias || name] = this._getDefaultOptionValue(option) } } @@ -135,7 +140,7 @@ export default class NuxtCommand { const option = this.cmd.options[name] let optionHelp = '--' - optionHelp += option.type === 'boolean' && option.default ? 'no-' : '' + optionHelp += option.type === 'boolean' && this._getDefaultOptionValue(option) ? 'no-' : '' optionHelp += name if (option.alias) { optionHelp += `, -${option.alias}` diff --git a/packages/cli/src/options/common.js b/packages/cli/src/options/common.js index 33909920b2..bc376442b3 100644 --- a/packages/cli/src/options/common.js +++ b/packages/cli/src/options/common.js @@ -28,11 +28,12 @@ export default { } } }, - // TODO: Change this to default: true in Nuxt 3 'force-exit': { type: 'boolean', - default: false, - description: 'Force Nuxt.js to exit after the command has finished (this option has no effect on commands which start a server)' + default(cmd) { + return ['build', 'generate'].includes(cmd.name) + }, + description: 'Whether Nuxt.js should force exit after the command has finished' }, version: { alias: 'v', diff --git a/packages/cli/src/utils/index.js b/packages/cli/src/utils/index.js index a0ea81357e..7050125164 100644 --- a/packages/cli/src/utils/index.js +++ b/packages/cli/src/utils/index.js @@ -140,10 +140,10 @@ export function normalizeArg(arg, defaultValue) { } export function forceExit(cmdName, timeout) { - if (timeout) { + if (timeout !== false) { const exitTimeout = setTimeout(() => { const msg = `The command 'nuxt ${cmdName}' finished but did not exit after ${timeout}s -This is most likely not caused by a bug in Nuxt.js\ +This is most likely not caused by a bug in Nuxt.js Make sure to cleanup all timers and listeners you or your plugins/modules start. Nuxt.js will now force exit diff --git a/packages/cli/test/unit/__snapshots__/command.test.js.snap b/packages/cli/test/unit/__snapshots__/command.test.js.snap index bc5230b09f..96b3942fea 100644 --- a/packages/cli/test/unit/__snapshots__/command.test.js.snap +++ b/packages/cli/test/unit/__snapshots__/command.test.js.snap @@ -18,10 +18,9 @@ exports[`cli/command builds help text 1`] = ` --modern, -m Build/Start app for modern browsers, e.g. server, client and false - --force-exit Force Nuxt.js to exit - after the command has finished (this - option has no effect on commands which - start a server) + --force-exit Whether Nuxt.js + should force exit after the command has + finished --version, -v Display the Nuxt version --help, -h Display this message diff --git a/packages/cli/test/unit/build.test.js b/packages/cli/test/unit/build.test.js index 5c546524ea..946fa2fb11 100644 --- a/packages/cli/test/unit/build.test.js +++ b/packages/cli/test/unit/build.test.js @@ -74,4 +74,36 @@ describe('build', () => { expect(options.modern).toBe(true) }) + + test('build force-exits by default', async () => { + mockGetNuxt() + mockGetBuilder(Promise.resolve()) + + const cmd = NuxtCommand.from(build, ['build', '.']) + await cmd.run() + + expect(utils.forceExit).toHaveBeenCalledTimes(1) + expect(utils.forceExit).toHaveBeenCalledWith('build', 5) + }) + + test('build can set force exit explicitly', async () => { + mockGetNuxt() + mockGetBuilder(Promise.resolve()) + + const cmd = NuxtCommand.from(build, ['build', '.', '--force-exit']) + await cmd.run() + + expect(utils.forceExit).toHaveBeenCalledTimes(1) + expect(utils.forceExit).toHaveBeenCalledWith('build', false) + }) + + test('build can disable force exit explicitly', async () => { + mockGetNuxt() + mockGetBuilder(Promise.resolve()) + + const cmd = NuxtCommand.from(build, ['build', '.', '--no-force-exit']) + await cmd.run() + + expect(utils.forceExit).not.toHaveBeenCalled() + }) }) diff --git a/packages/cli/test/unit/cli.test.js b/packages/cli/test/unit/cli.test.js index 5a7dfa7372..6a192bb8bd 100644 --- a/packages/cli/test/unit/cli.test.js +++ b/packages/cli/test/unit/cli.test.js @@ -6,7 +6,6 @@ jest.mock('../../src/commands') describe('cli', () => { beforeAll(() => { - // TODO: Below spyOn can be removed in v3 when force-exit is default false jest.spyOn(utils, 'forceExit').mockImplementation(() => {}) }) @@ -20,6 +19,7 @@ describe('cli', () => { await run() expect(mockedCommand.run).toHaveBeenCalled() + expect(utils.forceExit).not.toHaveBeenCalled() }) test('sets NODE_ENV=development for dev', async () => { diff --git a/packages/cli/test/unit/dev.test.js b/packages/cli/test/unit/dev.test.js index db5ad0d37e..5faafeab17 100644 --- a/packages/cli/test/unit/dev.test.js +++ b/packages/cli/test/unit/dev.test.js @@ -6,7 +6,6 @@ describe('dev', () => { beforeAll(async () => { dev = await import('../../src/commands/dev').then(m => m.default) - // TODO: Below spyOn can be removed in v3 when force-exit is default false jest.spyOn(utils, 'forceExit').mockImplementation(() => {}) }) @@ -107,4 +106,35 @@ describe('dev', () => { expect(consola.error).toHaveBeenCalledWith(new Error('Listen Error')) }) + + test('dev doesnt force-exit by default', async () => { + mockNuxt() + mockBuilder() + + const cmd = NuxtCommand.from(dev, ['dev', '.']) + await cmd.run() + + expect(utils.forceExit).not.toHaveBeenCalled() + }) + + test('dev can set force exit explicitly', async () => { + mockNuxt() + mockBuilder() + + const cmd = NuxtCommand.from(dev, ['dev', '.', '--force-exit']) + await cmd.run() + + expect(utils.forceExit).toHaveBeenCalledTimes(1) + expect(utils.forceExit).toHaveBeenCalledWith('dev', false) + }) + + test('dev can disable force exit explicitly', async () => { + mockNuxt() + mockBuilder() + + const cmd = NuxtCommand.from(dev, ['dev', '.', '--no-force-exit']) + await cmd.run() + + expect(utils.forceExit).not.toHaveBeenCalled() + }) }) diff --git a/packages/cli/test/unit/generate.test.js b/packages/cli/test/unit/generate.test.js index 1e2f5a40e2..ceef94b8df 100644 --- a/packages/cli/test/unit/generate.test.js +++ b/packages/cli/test/unit/generate.test.js @@ -63,4 +63,49 @@ describe('generate', () => { expect(options.modern).toBe('client') }) + + test('generate with modern mode', async () => { + mockGetNuxt() + mockGetGenerator(Promise.resolve()) + + const cmd = NuxtCommand.from(generate, ['generate', '.', '--m']) + + const options = await cmd.getNuxtConfig() + + await cmd.run() + + expect(options.modern).toBe('client') + }) + + test('generate force-exits by default', async () => { + mockGetNuxt() + mockGetGenerator(Promise.resolve()) + + const cmd = NuxtCommand.from(generate, ['generate', '.']) + await cmd.run() + + expect(utils.forceExit).toHaveBeenCalledTimes(1) + expect(utils.forceExit).toHaveBeenCalledWith('generate', 5) + }) + + test('generate can set force exit explicitly', async () => { + mockGetNuxt() + mockGetGenerator(Promise.resolve()) + + const cmd = NuxtCommand.from(generate, ['generate', '.', '--force-exit']) + await cmd.run() + + expect(utils.forceExit).toHaveBeenCalledTimes(1) + expect(utils.forceExit).toHaveBeenCalledWith('generate', false) + }) + + test('generate can disable force exit explicitly', async () => { + mockGetNuxt() + mockGetGenerator(Promise.resolve()) + + const cmd = NuxtCommand.from(generate, ['generate', '.', '--no-force-exit']) + await cmd.run() + + expect(utils.forceExit).not.toHaveBeenCalled() + }) }) diff --git a/packages/cli/test/unit/start.test.js b/packages/cli/test/unit/start.test.js index e1b3bf13bf..3a30702f7d 100644 --- a/packages/cli/test/unit/start.test.js +++ b/packages/cli/test/unit/start.test.js @@ -7,7 +7,6 @@ describe('start', () => { beforeAll(async () => { start = await import('../../src/commands/start').then(m => m.default) - // TODO: Below spyOn can be removed in v3 when force-exit is default false jest.spyOn(utils, 'forceExit').mockImplementation(() => {}) }) @@ -35,4 +34,32 @@ describe('start', () => { await NuxtCommand.from(start).run() expect(consola.fatal).not.toHaveBeenCalled() }) + + test('start doesnt force-exit by default', async () => { + mockGetNuxtStart() + + const cmd = NuxtCommand.from(start, ['start', '.']) + await cmd.run() + + expect(utils.forceExit).not.toHaveBeenCalled() + }) + + test('start can set force exit explicitly', async () => { + mockGetNuxtStart() + + const cmd = NuxtCommand.from(start, ['start', '.', '--force-exit']) + await cmd.run() + + expect(utils.forceExit).toHaveBeenCalledTimes(1) + expect(utils.forceExit).toHaveBeenCalledWith('start', false) + }) + + test('start can disable force exit explicitly', async () => { + mockGetNuxtStart() + + const cmd = NuxtCommand.from(start, ['start', '.', '--no-force-exit']) + await cmd.run() + + expect(utils.forceExit).not.toHaveBeenCalled() + }) }) diff --git a/packages/cli/test/utils/mocking.js b/packages/cli/test/utils/mocking.js index b30e94fbf6..05836e299d 100644 --- a/packages/cli/test/utils/mocking.js +++ b/packages/cli/test/utils/mocking.js @@ -18,12 +18,12 @@ jest.mock('../../src/imports', () => { } }) -export const mockGetNuxt = (options, implementation) => { +export const mockGetNuxt = (options = {}, implementation) => { Command.prototype.getNuxt = jest.fn().mockImplementationOnce(() => { return Object.assign({ hook: jest.fn(), options - }, implementation || {}) + }, implementation) }) }