diff --git a/README.md b/README.md index a421e850c..ad26152df 100644 --- a/README.md +++ b/README.md @@ -137,7 +137,7 @@ module.exports = BbPromise.try(() => { loaders: [ ... ] } })); -}; +}); ``` ### serverless-webpack lib export helper diff --git a/lib/compile.js b/lib/compile.js index 679bdee5d..64da03dd0 100644 --- a/lib/compile.js +++ b/lib/compile.js @@ -3,6 +3,7 @@ const _ = require('lodash'); const BbPromise = require('bluebird'); const webpack = require('webpack'); +const tty = require('tty'); module.exports = { compile() { @@ -20,7 +21,7 @@ module.exports = { const compileOutputPaths = []; const consoleStats = this.webpackConfig.stats || _.get(this, 'webpackConfig[0].stats') || { - colors: true, + colors: tty.isatty(process.stdout.fd), hash: false, version: false, chunks: false, diff --git a/lib/wpwatch.js b/lib/wpwatch.js index 4e1022626..e2a33728d 100644 --- a/lib/wpwatch.js +++ b/lib/wpwatch.js @@ -3,6 +3,7 @@ const _ = require('lodash'); const BbPromise = require('bluebird'); const webpack = require('webpack'); +const tty = require('tty'); module.exports = { wpwatch() { @@ -21,9 +22,46 @@ module.exports = { this.serverless.cli.log(`Enabled polling (${watchOptions.poll} ms)`); } + let currentCompileWatch = null; + + // This allows us to hold the compile until "webpack:compile:watch" has resolved + const beforeCompile = () => ( + new BbPromise((resolve) => { + // eslint-disable-next-line promise/catch-or-return + BbPromise.resolve(currentCompileWatch) + // Forwarding the error to the then so we don't display it twice + // (once when it was originally thrown, and once when the promise rejects) + .catch(error => error) + .then((error) => { + if (error) { + return null; + } + + currentCompileWatch = null; + resolve(); + return null; + }); + }) + ); + const compiler = webpack(this.webpackConfig); + + // Determine if we can use hooks or if we should fallback to the plugin api + const hasHooks = compiler.hooks && compiler.hooks.beforeCompile; + const hasPlugins = compiler.plugin; + const canEmit = hasHooks || hasPlugins; + + if (hasHooks) { + compiler.hooks.beforeCompile.tapPromise('webpack:compile:watch', beforeCompile); + } else if (hasPlugins) { + compiler.plugin('before-compile', (compilationParams, callback) => { + // eslint-disable-next-line promise/no-callback-in-promise + beforeCompile().then(callback).catch(_.noop); + }); + } + const consoleStats = this.webpackConfig.stats || { - colors: true, + colors: tty.isatty(process.stdout.fd), hash: false, version: false, chunks: false, @@ -32,9 +70,10 @@ module.exports = { // This starts the watch and waits for the immediate compile that follows to end or fail. let lastHash = null; + const startWatch = (callback) => { let firstRun = true; - const watcher = compiler.watch(watchOptions, (err, stats) => { + compiler.watch(watchOptions, (err, stats) => { if (err) { if (firstRun) { firstRun = false; @@ -65,18 +104,10 @@ module.exports = { firstRun = false; this.serverless.cli.log('Watching for changes...'); callback(); - } else { - // We will close the watcher while the compile event is triggered and resume afterwards to prevent race conditions. - watcher.close(() => { - return this.serverless.pluginManager.spawn('webpack:compile:watch') - .then(() => { - // Resume watching after we triggered the compile:watch event - return BbPromise.fromCallback(cb => { - startWatch(cb); - }) - .then(() => this.serverless.cli.log('Watching for changes...')); - }); - }); + } else if (canEmit && currentCompileWatch === null) { + currentCompileWatch = BbPromise.resolve( + this.serverless.pluginManager.spawn('webpack:compile:watch') + ).then(() => this.serverless.cli.log('Watching for changes...')); } }); }; diff --git a/tests/webpack.mock.js b/tests/webpack.mock.js index 68b612da8..9f8f124d6 100644 --- a/tests/webpack.mock.js +++ b/tests/webpack.mock.js @@ -12,23 +12,23 @@ const StatsMock = () => ({ toString: sinon.stub().returns('testStats'), }); -const WatchMock = sandbox => ({ - close: sandbox.stub().callsFake(cb => cb()) -}); -const CompilerMock = (sandbox, statsMock, watchMock) => ({ +const CompilerMock = (sandbox, statsMock) => ({ run: sandbox.stub().yields(null, statsMock), - watch: sandbox.stub().returns(watchMock).yields(null, statsMock) + watch: sandbox.stub().yields(null, statsMock), + hooks: { + beforeCompile: { + tapPromise: sandbox.stub() + } + }, }); const webpackMock = sandbox => { const statsMock = StatsMock(sandbox); - const watchMock = WatchMock(sandbox); - const compilerMock = CompilerMock(sandbox, statsMock, watchMock); + const compilerMock = CompilerMock(sandbox, statsMock); const mock = sinon.stub().returns(compilerMock); mock.compilerMock = compilerMock; mock.statsMock = statsMock; - mock.watchMock = watchMock; return mock; }; diff --git a/tests/wpwatch.test.js b/tests/wpwatch.test.js index 3f278226e..05afb0505 100644 --- a/tests/wpwatch.test.js +++ b/tests/wpwatch.test.js @@ -102,6 +102,19 @@ describe('wpwatch', function() { )); }); + it('should still enter watch mode and return if lastHash is the same as previous', () => { + const wpwatch = module.wpwatch.bind(module); + webpackMock.compilerMock.watch.yields(null, { hash: null }); + + return expect(wpwatch()).to.be.fulfilled + .then(() => BbPromise.join( + expect(spawnStub).to.not.have.been.called, + expect(webpackMock.compilerMock.watch).to.have.been.calledOnce, + expect(spawnStub).to.not.have.been.called + )); + }); + + it('should work if no stats are returned', () => { const wpwatch = module.wpwatch.bind(module); webpackMock.compilerMock.watch.yields(); @@ -142,54 +155,124 @@ describe('wpwatch', function() { )); }); - it('should call callback on subsequent runs', () => { + it('should spawn webpack:compile:watch on subsequent runs', () => { const wpwatch = module.wpwatch.bind(module); let watchCallbackSpy; + let beforeCompileCallbackSpy; + + spawnStub.resolves(); + + webpackMock.compilerMock.hooks.beforeCompile.tapPromise.callsFake((options, cb) => { + beforeCompileCallbackSpy = sandbox.spy(cb); + }); + webpackMock.compilerMock.watch.onFirstCall().callsFake((options, cb) => { - // We'll spy the callback registered for watch watchCallbackSpy = sandbox.spy(cb); - - // Schedule second call after 2 seconds - setTimeout(() => { - process.nextTick(() => watchCallbackSpy(null, { call: 2, hash: '2' })); - }, 2000); - process.nextTick(() => watchCallbackSpy(null, { call: 1, hash: '1' })); - return webpackMock.watchMock; + watchCallbackSpy(null, { call: 1, hash: '1' }); + watchCallbackSpy(null, { call: 2, hash: '2' }); + + // We only call this once, to simulate that promises that might take longer to resolve + // don't cause a re-emit to avoid race-conditions. + beforeCompileCallbackSpy(); + watchCallbackSpy(null, { call: 3, hash: '3' }); + watchCallbackSpy(null, { call: 3, hash: '4' }); }); + + return expect(wpwatch()).to.be.fulfilled.then( + () => BbPromise.join( + expect(watchCallbackSpy).to.have.been.callCount(4), + expect(spawnStub).to.have.been.calledOnce, + expect(spawnStub).to.have.been.calledWithExactly('webpack:compile:watch') + ) + ); + }); + + it('should spawn more webpack:compile:watch when previous is resolved', () => { + const wpwatch = module.wpwatch.bind(module); + let watchCallbackSpy; + let beforeCompileCallbackSpy; + let beforeCompileCallbackSpyPromise; + spawnStub.resolves(); + webpackMock.compilerMock.hooks.beforeCompile.tapPromise.callsFake((options, cb) => { + beforeCompileCallbackSpy = sandbox.spy(cb); + }); + + webpackMock.compilerMock.watch.onFirstCall().callsFake((options, cb) => { + watchCallbackSpy = sandbox.spy(cb); + + watchCallbackSpy(null, { call: 1, hash: '1' }); + watchCallbackSpy(null, { call: 2, hash: '2' }); + + // eslint-disable-next-line promise/always-return,promise/catch-or-return + beforeCompileCallbackSpyPromise = beforeCompileCallbackSpy().then(() => { + watchCallbackSpy(null, { call: 3, hash: '3' }); + }); + }); + return expect(wpwatch()).to.be.fulfilled - .then(() => BbPromise.delay(3000)) - .then(() => BbPromise.join( - expect(spawnStub).to.have.been.calledOnce, - expect(spawnStub).to.have.been.calledWithExactly('webpack:compile:watch'), - expect(webpackMock.compilerMock.watch).to.have.been.calledTwice, - expect(webpackMock.watchMock.close).to.have.been.calledOnce, - expect(watchCallbackSpy).to.have.been.calledTwice + .then(() => beforeCompileCallbackSpyPromise) + .then(() => BbPromise.join( + expect(watchCallbackSpy).to.have.been.calledThrice, + expect(spawnStub).to.have.been.calledTwice, + expect(spawnStub).to.have.been.calledWithExactly('webpack:compile:watch') + )); + }); + + it('should use plugins for webpack:compile:watch if hooks doesn\'t exist', () => { + const wpwatch = module.wpwatch.bind(module); + sandbox.stub(webpackMock.compilerMock, 'hooks').value(false); + + webpackMock.compilerMock.plugin = sandbox.stub().yields(null, _.noop); + webpackMock.compilerMock.watch.yields(null, {}); + + return expect(wpwatch()).to.be.fulfilled.then(() => ( + expect(webpackMock.compilerMock.plugin).to.have.been.calledOnce + )); + }); + + it('should not resolve before compile if it has an error', () => { + const wpwatch = module.wpwatch.bind(module); + spawnStub.returns(BbPromise.reject(new Error('actual error'))); + + let beforeCompileCallbackSpy; + webpackMock.compilerMock.hooks.beforeCompile.tapPromise.callsFake((options, cb) => { + beforeCompileCallbackSpy = sandbox.spy(cb); + }); + + let doesResolve = false; + webpackMock.compilerMock.watch.onFirstCall().callsFake((options, cb) => { + cb(null, { call: 1, hash: '1' }); + cb(null, { call: 2, hash: '2' }); + + // eslint-disable-next-line promise/catch-or-return,promise/always-return + beforeCompileCallbackSpy().then(() => { + // We don't expect this to be set to true + doesResolve = true; + }); + }); + + return expect(wpwatch()).to.be.fulfilled.then(() => ( + expect(doesResolve).to.be.false )); }); it('should throw if compile fails on subsequent runs', () => { const wpwatch = module.wpwatch.bind(module); let watchCallbackSpy; + + spawnStub.resolves(); + webpackMock.compilerMock.watch.callsFake((options, cb) => { // We'll spy the callback registered for watch watchCallbackSpy = sandbox.spy(cb); - // Schedule second call after 2 seconds - setTimeout(() => { - try { - watchCallbackSpy(new Error('Compile failed')); - } catch (e) { - // Ignore the exception. The spy will record it. - } - }, 2000); - process.nextTick(() => watchCallbackSpy(null, { call: 3, hash: '3' })); + watchCallbackSpy(null, { call: 3, hash: '3' }); + watchCallbackSpy(new Error('Compile failed')); }); - spawnStub.resolves(); return expect(wpwatch()).to.be.fulfilled - .then(() => BbPromise.delay(3000)) .then(() => BbPromise.join( expect(watchCallbackSpy).to.have.been.calledTwice, expect(watchCallbackSpy.secondCall.threw()).to.be.true