From 97666b319da2b82959218a350a6a26e05bce049b Mon Sep 17 00:00:00 2001 From: Dave Welch Date: Mon, 5 Feb 2018 10:15:00 -0700 Subject: [PATCH 1/7] #315 - Add watch-compile event for more fine grain event control while doing local development --- index.js | 6 ++++++ lib/wpwatch.js | 2 ++ 2 files changed, 8 insertions(+) diff --git a/index.js b/index.js index 5b5e47171..9d7881bc3 100644 --- a/index.js +++ b/index.js @@ -72,6 +72,12 @@ class ServerlessWebpack { 'compile', ], }, + "watch-compile": { + type: 'entrypoint', + lifecycleEvents: [ + 'watch-compile', + ], + }, package: { type: 'entrypoint', lifecycleEvents: [ diff --git a/lib/wpwatch.js b/lib/wpwatch.js index 2325cfa6b..961858437 100644 --- a/lib/wpwatch.js +++ b/lib/wpwatch.js @@ -51,6 +51,8 @@ module.exports = { if (firstRun) { firstRun = false; callback(); + }else{ + this.serverless.pluginManager.spawn('webpack:watch-compile') } }); }; From b14bc9c8c8006d07ced8d0c5193ed36c6a6735be Mon Sep 17 00:00:00 2001 From: Dave Welch Date: Tue, 6 Feb 2018 07:18:38 -0700 Subject: [PATCH 2/7] 319 - Added test coverage + renamed hooks, per @HyperBrain recommendations --- index.js | 16 ++++++++++------ lib/wpwatch.js | 2 +- tests/run.test.js | 23 +++++++++++++++++++++++ tests/wpwatch.test.js | 2 +- 4 files changed, 35 insertions(+), 8 deletions(-) diff --git a/index.js b/index.js index 9d7881bc3..7d99d55fc 100644 --- a/index.js +++ b/index.js @@ -71,12 +71,14 @@ class ServerlessWebpack { lifecycleEvents: [ 'compile', ], - }, - "watch-compile": { - type: 'entrypoint', - lifecycleEvents: [ - 'watch-compile', - ], + commands: { + watch: { + type: 'entrypoint', + lifecycleEvents: [ + 'compile' + ] + } + } }, package: { type: 'entrypoint', @@ -145,6 +147,8 @@ class ServerlessWebpack { 'webpack:compile:compile': () => BbPromise.bind(this) .then(this.compile), + 'webpack:compile:watch:compile': () => BbPromise.resolve(), + 'webpack:package:packExternalModules': () => BbPromise.bind(this) .then(this.packExternalModules), diff --git a/lib/wpwatch.js b/lib/wpwatch.js index 961858437..d603d5bac 100644 --- a/lib/wpwatch.js +++ b/lib/wpwatch.js @@ -52,7 +52,7 @@ module.exports = { firstRun = false; callback(); }else{ - this.serverless.pluginManager.spawn('webpack:watch-compile') + this.serverless.pluginManager.spawn('webpack:compile:watch') } }); }; diff --git a/tests/run.test.js b/tests/run.test.js index 2de0b8860..65e9c071f 100644 --- a/tests/run.test.js +++ b/tests/run.test.js @@ -87,6 +87,29 @@ describe('run', () => { expect(module.isWatching).to.be.true; }); + it('should not spawn on watch first run', () => { + module.isWatching = false; + const watch = module.watch.bind(module); + webpackMock.compilerMock.watch = sandbox.stub().yields(null, {}); + _.set(module, 'options.function', 'myFunction'); + + watch('compile:watch:compile'); + expect(spawnStub).to.not.have.been.called; + expect(module.isWatching).to.be.true; + }); + + it('should spawn on watch second run', () => { + module.isWatching = false; + const watch = module.watch.bind(module); + webpackMock.compilerMock.watch = sandbox.stub().yields(null, {}); + _.set(module, 'options.function', 'myFunction'); + + watch('compile:watch:compile'); + watch('compile:watch:compile'); + expect(spawnStub).to.have.been.calledOnce; + expect(module.isWatching).to.be.true; + }); + it('should spawn invoke local on subsequent runs', () => { module.isWatching = true; const watch = module.watch.bind(module); diff --git a/tests/wpwatch.test.js b/tests/wpwatch.test.js index 4645c0437..25524e017 100644 --- a/tests/wpwatch.test.js +++ b/tests/wpwatch.test.js @@ -160,7 +160,7 @@ describe('wpwatch', function() { return expect(wpwatch()).to.be.fulfilled .then(() => BbPromise.delay(3000)) .then(() => BbPromise.join( - expect(spawnStub).to.not.have.been.called, + expect(spawnStub).to.have.been.calledOnce, expect(webpackMock.compilerMock.watch).to.have.been.calledOnce, expect(watchCallbackSpy).to.have.been.calledTwice )); From 599cb4bc6fe9571bdd8c3e05442bf69eba57f39b Mon Sep 17 00:00:00 2001 From: Dave Welch Date: Tue, 6 Feb 2018 07:26:10 -0700 Subject: [PATCH 3/7] lint fix :) --- lib/wpwatch.js | 2 +- package-lock.json | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/wpwatch.js b/lib/wpwatch.js index d603d5bac..20f846cf4 100644 --- a/lib/wpwatch.js +++ b/lib/wpwatch.js @@ -52,7 +52,7 @@ module.exports = { firstRun = false; callback(); }else{ - this.serverless.pluginManager.spawn('webpack:compile:watch') + this.serverless.pluginManager.spawn('webpack:compile:watch'); } }); }; diff --git a/package-lock.json b/package-lock.json index 0fc9aee75..b2f974a98 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5255,6 +5255,14 @@ "integrity": "sha1-qPbq7KkGdMMz58Q5U/J1tFFRBpU=", "dev": true }, + "string_decoder": { + "version": "1.0.3", + "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-1.0.3.tgz", + "integrity": "sha1-D8Z9fBQYJd6UKC3VNr7GubzoYKs=", + "requires": { + "safe-buffer": "5.1.1" + } + }, "string-width": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/string-width/-/string-width-1.0.2.tgz", @@ -5266,14 +5274,6 @@ "strip-ansi": "3.0.1" } }, - "string_decoder": { - "version": "1.0.3", - "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-1.0.3.tgz", - "integrity": "sha1-D8Z9fBQYJd6UKC3VNr7GubzoYKs=", - "requires": { - "safe-buffer": "5.1.1" - } - }, "stringstream": { "version": "0.0.5", "resolved": "https://registry.npmjs.org/stringstream/-/stringstream-0.0.5.tgz", From 7474d3c625048e642a166c13cdcd8e91ee02f51e Mon Sep 17 00:00:00 2001 From: Dave Welch Date: Tue, 6 Feb 2018 10:32:52 -0700 Subject: [PATCH 4/7] Documentation for compile:watch:compile hook --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 60f20e160..234d7d944 100644 --- a/README.md +++ b/README.md @@ -467,6 +467,7 @@ All events (H) can be hooked by a plugin. -> webpack:validate:validate (H) -> webpack:compile -> webpack:compile:compile (H) + -> webpack:compile:watch:compile (H) -> webpack:package -> webpack:package:packExternalModules (H) -> webpack:package:packageModules (H) From f4f075926041cbefbea7c44ddccc3d98a769b73e Mon Sep 17 00:00:00 2001 From: Frank Schmid Date: Wed, 7 Feb 2018 13:37:01 +0100 Subject: [PATCH 5/7] Fixed race condition when triggering watch event --- lib/wpwatch.js | 33 ++++++++++++++++++++++++++++----- package-lock.json | 16 ++++++++-------- 2 files changed, 36 insertions(+), 13 deletions(-) diff --git a/lib/wpwatch.js b/lib/wpwatch.js index 20f846cf4..2da19d2a8 100644 --- a/lib/wpwatch.js +++ b/lib/wpwatch.js @@ -31,9 +31,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; - compiler.watch(watchOptions, (err, stats) => { + const watcher = compiler.watch(watchOptions, (err, stats) => { if (err) { if (firstRun) { firstRun = false; @@ -42,17 +43,39 @@ module.exports = { throw err; } + process.env.SLS_DEBUG && this.serverless.cli.log(`Webpack watch invoke: HASH NEW=${stats.hash} CUR=${lastHash}`); + + // If the file hash did not change there were no effective code changes detected (comment only changed do not change the compile hash!) + // See here: https://webpack.js.org/api/node/#watching (note below watching) + if (stats && stats.hash === lastHash) { + if (firstRun) { + firstRun = false; + callback(); + } + return; + } + if (stats) { + lastHash = stats.hash; this.serverless.cli.consoleLog(stats.toString(consoleStats)); } - this.serverless.cli.log('Watching for changes...'); - if (firstRun) { firstRun = false; + this.serverless.cli.log('Watching for changes...'); callback(); - }else{ - this.serverless.pluginManager.spawn('webpack:compile:watch'); + } 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...')); + }); + }); } }); }; diff --git a/package-lock.json b/package-lock.json index b2f974a98..0fc9aee75 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5255,14 +5255,6 @@ "integrity": "sha1-qPbq7KkGdMMz58Q5U/J1tFFRBpU=", "dev": true }, - "string_decoder": { - "version": "1.0.3", - "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-1.0.3.tgz", - "integrity": "sha1-D8Z9fBQYJd6UKC3VNr7GubzoYKs=", - "requires": { - "safe-buffer": "5.1.1" - } - }, "string-width": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/string-width/-/string-width-1.0.2.tgz", @@ -5274,6 +5266,14 @@ "strip-ansi": "3.0.1" } }, + "string_decoder": { + "version": "1.0.3", + "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-1.0.3.tgz", + "integrity": "sha1-D8Z9fBQYJd6UKC3VNr7GubzoYKs=", + "requires": { + "safe-buffer": "5.1.1" + } + }, "stringstream": { "version": "0.0.5", "resolved": "https://registry.npmjs.org/stringstream/-/stringstream-0.0.5.tgz", From b2882c8e1afb1fd67315fc8fddc5e988d49d78b1 Mon Sep 17 00:00:00 2001 From: Frank Schmid Date: Wed, 7 Feb 2018 13:53:09 +0100 Subject: [PATCH 6/7] Fixed unit test and comment --- lib/wpwatch.js | 3 ++- tests/wpwatch.test.js | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/wpwatch.js b/lib/wpwatch.js index 2da19d2a8..4e1022626 100644 --- a/lib/wpwatch.js +++ b/lib/wpwatch.js @@ -45,7 +45,8 @@ module.exports = { process.env.SLS_DEBUG && this.serverless.cli.log(`Webpack watch invoke: HASH NEW=${stats.hash} CUR=${lastHash}`); - // If the file hash did not change there were no effective code changes detected (comment only changed do not change the compile hash!) + // If the file hash did not change there were no effective code changes detected + // (comment changes do not change the compile hash and do not account for a rebuild!) // See here: https://webpack.js.org/api/node/#watching (note below watching) if (stats && stats.hash === lastHash) { if (firstRun) { diff --git a/tests/wpwatch.test.js b/tests/wpwatch.test.js index 25524e017..4645c0437 100644 --- a/tests/wpwatch.test.js +++ b/tests/wpwatch.test.js @@ -160,7 +160,7 @@ describe('wpwatch', function() { return expect(wpwatch()).to.be.fulfilled .then(() => BbPromise.delay(3000)) .then(() => BbPromise.join( - expect(spawnStub).to.have.been.calledOnce, + expect(spawnStub).to.not.have.been.called, expect(webpackMock.compilerMock.watch).to.have.been.calledOnce, expect(watchCallbackSpy).to.have.been.calledTwice )); From 352484981bf7fbf4e65ff5f7d5c4cabaea522d54 Mon Sep 17 00:00:00 2001 From: Frank Schmid Date: Wed, 7 Feb 2018 14:57:32 +0100 Subject: [PATCH 7/7] Test new behavior in wpwatch unit tests --- tests/webpack.mock.js | 12 +++++++++--- tests/wpwatch.test.js | 15 +++++++++------ 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/tests/webpack.mock.js b/tests/webpack.mock.js index 1939f3825..68b612da8 100644 --- a/tests/webpack.mock.js +++ b/tests/webpack.mock.js @@ -12,17 +12,23 @@ const StatsMock = () => ({ toString: sinon.stub().returns('testStats'), }); -const CompilerMock = (sandbox, statsMock) => ({ +const WatchMock = sandbox => ({ + close: sandbox.stub().callsFake(cb => cb()) +}); + +const CompilerMock = (sandbox, statsMock, watchMock) => ({ run: sandbox.stub().yields(null, statsMock), - watch: sandbox.stub().yields(null, statsMock) + watch: sandbox.stub().returns(watchMock).yields(null, statsMock) }); const webpackMock = sandbox => { const statsMock = StatsMock(sandbox); - const compilerMock = CompilerMock(sandbox, statsMock); + const watchMock = WatchMock(sandbox); + const compilerMock = CompilerMock(sandbox, statsMock, watchMock); 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 4645c0437..3f278226e 100644 --- a/tests/wpwatch.test.js +++ b/tests/wpwatch.test.js @@ -145,23 +145,26 @@ describe('wpwatch', function() { it('should call callback on subsequent runs', () => { const wpwatch = module.wpwatch.bind(module); let watchCallbackSpy; - webpackMock.compilerMock.watch.callsFake((options, 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(() => { - watchCallbackSpy(null, { call: 2 }); + process.nextTick(() => watchCallbackSpy(null, { call: 2, hash: '2' })); }, 2000); - process.nextTick(() => watchCallbackSpy(null, { call: 1 })); + process.nextTick(() => watchCallbackSpy(null, { call: 1, hash: '1' })); + return webpackMock.watchMock; }); spawnStub.resolves(); return expect(wpwatch()).to.be.fulfilled .then(() => BbPromise.delay(3000)) .then(() => BbPromise.join( - expect(spawnStub).to.not.have.been.called, - expect(webpackMock.compilerMock.watch).to.have.been.calledOnce, + 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 )); }); @@ -181,7 +184,7 @@ describe('wpwatch', function() { // Ignore the exception. The spy will record it. } }, 2000); - process.nextTick(() => watchCallbackSpy(null, { call: 1 })); + process.nextTick(() => watchCallbackSpy(null, { call: 3, hash: '3' })); }); spawnStub.resolves();