From e363e2a04fb20c4cfac34e411df21530e82f3f7a Mon Sep 17 00:00:00 2001 From: Grgur Grisogono Date: Mon, 30 Jul 2018 12:33:41 +0200 Subject: [PATCH 1/5] Fix invocation of the async self-calling function Needed an extra set of parenthesis around the async arrow function to be properly auto-invoked --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 4e2a8c451..f5d7f2ac1 100644 --- a/README.md +++ b/README.md @@ -98,7 +98,7 @@ A basic Webpack promise configuration might look like this: const webpack = require('webpack') const slsw = require('serverless-webpack'); -module.exports = async () => { +module.exports = (async () => { const accountId = await slsw.lib.serverless.providers.aws.getAccountId(); return { entry: './handler.js', @@ -112,7 +112,7 @@ module.exports = async () => { loaders: [ ... ] } }; -}(); +})(); ``` ```js // Version with promises From a4ad236abb4c661cd188b31f8a22b1c9ad344b6b Mon Sep 17 00:00:00 2001 From: unknown Date: Sat, 20 Oct 2018 15:09:13 -0500 Subject: [PATCH 2/5] Small change on readme - Missing parenthesis --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 3904246f1..894eae80b 100644 --- a/README.md +++ b/README.md @@ -136,7 +136,7 @@ module.exports = BbPromise.try(() => { loaders: [ ... ] } })); -}; +}); ``` ### serverless-webpack lib export helper From 720b17be02f20de3dcbd7b94d349adc89f090435 Mon Sep 17 00:00:00 2001 From: Neil Locketz Date: Wed, 30 Jan 2019 09:35:03 -0500 Subject: [PATCH 3/5] Disable colors for non-interactive output --- lib/compile.js | 3 ++- lib/wpwatch.js | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) 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..2345a7242 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() { @@ -23,7 +24,7 @@ module.exports = { const compiler = webpack(this.webpackConfig); const consoleStats = this.webpackConfig.stats || { - colors: true, + colors: tty.isatty(process.stdout.fd), hash: false, version: false, chunks: false, From 36e899aa93c645eeb3fbdd77433bd42c7692de0c Mon Sep 17 00:00:00 2001 From: Zn4rK Date: Sun, 3 Mar 2019 22:57:50 +0100 Subject: [PATCH 4/5] Converted the watcher.stop() to use built in webpack hooks (or plugins) instead --- lib/wpwatch.js | 56 ++++++++++++++++++++------- tests/webpack.mock.js | 16 ++++---- tests/wpwatch.test.js | 88 +++++++++++++++++++++++++++++-------------- 3 files changed, 111 insertions(+), 49 deletions(-) diff --git a/lib/wpwatch.js b/lib/wpwatch.js index 4e1022626..a30753f3c 100644 --- a/lib/wpwatch.js +++ b/lib/wpwatch.js @@ -21,7 +21,44 @@ 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, hash: false, @@ -32,9 +69,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 +103,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..e6aa90d0a 100644 --- a/tests/wpwatch.test.js +++ b/tests/wpwatch.test.js @@ -142,54 +142,86 @@ 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 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 From 198dbdf3325261b4004d314c84331ee49277d540 Mon Sep 17 00:00:00 2001 From: Zn4rK Date: Mon, 4 Mar 2019 13:56:10 +0100 Subject: [PATCH 5/5] More code coverage --- tests/wpwatch.test.js | 51 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/tests/wpwatch.test.js b/tests/wpwatch.test.js index e6aa90d0a..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(); @@ -207,6 +220,44 @@ describe('wpwatch', function() { )); }); + 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;