Repository: serverless-heaven/serverless-webpack
Pull request #319 (2018-02-05 17:48:16)

#315 - Add watch-compile event
Quick shoutout: I'm new to plugin development and not sure the best way to test / lint / etc, but happy to make changes and do the legwork as needed.

## What did you implement:

We've got a plugin in development that needs access to the post-compiled code for our handler, but in local development but there's no way to detect compiled changes if using watch. We've toyed with adding a new event on `lib/wpwatch`
that plugins could hook on as needed. 

See an example project at https://github.com/dwelch2344/serverless-webpack-watch-compile-demo and the issue #315 
filed at https://github.com/serverless-heaven/serverless-webpack/issues/315


## How did you implement it:

Added a `watch-compile` definition in `index.js` and a `serverless.spawn` of said event in `lib/wpwatch.js` 

## How can we verify it:

* Install locally to your npm 
* Clone this project: https://github.com/dwelch2344/serverless-webpack-watch-compile-demo
* Adjust the project's `package.json` to use your local version
* run `npm run debug` and observe the plugin logs 15 times:
```
→ npm run debug

> serverless-webpack-watch-compile@1.1.0 debug /tmp/serverless-webpack-watch-compile
> node --inspect ./node_modules/.bin/serverless offline -P 5000 -s dev

Debugger listening on ws://127.0.0.1:9229/8ee583a5-086a-407b-829d-c1a971a4d39f
For help see https://nodejs.org/en/docs/inspector
Serverless: Running after a compiled change detected
Serverless: Running after a compiled change detected
Serverless: Running after a compiled change detected
Serverless: Running after a compiled change detected
Serverless: Running after a compiled change detected
Serverless: Running after a compiled change detected
Serverless: Running after a compiled change detected
Serverless: Running after a compiled change detected
Serverless: Running after a compiled change detected
Serverless: Running after a compiled change detected
Serverless: Running after a compiled change detected
Serverless: Running after a compiled change detected
Serverless: Running after a compiled change detected
Serverless: Running after a compiled change detected
Serverless: Running after a compiled change detected
Serverless: Bundling with Webpack...
Time: 519ms
         Asset     Size  Chunks             Chunk Names
    handler.js  5.52 kB       0  [emitted]  handler
handler.js.map  5.38 kB       0  [emitted]  handler
   [0] ./handler.js 2.35 kB {0} [built]
   [1] external "babel-runtime/core-js/promise" 42 bytes {0} [not cacheable]
   [2] external "babel-runtime/helpers/objectWithoutProperties" 42 bytes {0} [not cacheable]
   [3] external "babel-runtime/regenerator" 42 bytes {0} [not cacheable]
   [4] external "babel-runtime/core-js/json/stringify" 42 bytes {0} [not cacheable]
   [5] external "babel-runtime/helpers/asyncToGenerator" 42 bytes {0} [not cacheable]
   [6] external "source-map-support/register" 42 bytes {0} [not cacheable]
Serverless: Watching for changes...
Serverless: Starting Offline: dev/us-east-1.

Serverless: Routes for hello:
Serverless: GET /hello

Serverless: Offline listening on http://localhost:5000
```

* Now make a change in `handler.js`, and observe the plugin logs again 

```
Time: 88ms
         Asset     Size  Chunks             Chunk Names
    handler.js  5.52 kB       0  [emitted]  handler
handler.js.map  5.38 kB       0  [emitted]  handler
   [0] ./handler.js 2.36 kB {0} [built]
   [1] external "babel-runtime/core-js/promise" 42 bytes {0} [not cacheable]
   [2] external "babel-runtime/helpers/objectWithoutProperties" 42 bytes {0} [not cacheable]
   [3] external "babel-runtime/regenerator" 42 bytes {0} [not cacheable]
   [4] external "babel-runtime/core-js/json/stringify" 42 bytes {0} [not cacheable]
   [5] external "babel-runtime/helpers/asyncToGenerator" 42 bytes {0} [not cacheable]
   [6] external "source-map-support/register" 42 bytes {0} [not cacheable]
Serverless: Watching for changes...
Serverless: Running after a compiled change detected
Serverless: Running after a compiled change detected
Serverless: Running after a compiled change detected
Serverless: Running after a compiled change detected
Serverless: Running after a compiled change detected
Serverless: Running after a compiled change detected
Serverless: Running after a compiled change detected
Serverless: Running after a compiled change detected
Serverless: Running after a compiled change detected
Serverless: Running after a compiled change detected
Serverless: Running after a compiled change detected
Serverless: Running after a compiled change detected
Serverless: Running after a compiled change detected
Serverless: Running after a compiled change detected
Serverless: Running after a compiled change detected
```

* Verify your change is there and things worked as expected

## Todos:

- [ ] Write tests
- [ ] Write documentation
- [ ] Fix linting errors
- [ ] Make sure code coverage hasn't dropped
- [ ] Provide verification config / commands / resources
- [X] Enable "Allow edits from maintainers" for this PR
- [ ] Update the messages below

***Is this ready for review?:*** YES
***Is it a breaking change?:*** NO



Patch:
diff --git a/README.md b/README.md
index 60f20e1..234d7d9 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)
diff --git a/index.js b/index.js
index 5b5e471..7d99d55 100644
--- a/index.js
+++ b/index.js
@@ -71,6 +71,14 @@ class ServerlessWebpack {
             lifecycleEvents: [
               'compile',
             ],
+            commands: {
+              watch: {
+                type: 'entrypoint',
+                lifecycleEvents: [
+                  'compile'
+                ]
+              }
+            }
           },
           package: {
             type: 'entrypoint',
@@ -139,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 2325cfa..4e10226 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,15 +43,40 @@ 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 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) {
+            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 {
+          // 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...'));
+            });
+          });
         }
       });
     };


Test patch:
diff --git a/tests/run.test.js b/tests/run.test.js
index 2de0b88..65e9c07 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/webpack.mock.js b/tests/webpack.mock.js
index 1939f38..68b612d 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 4645c04..3f27822 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();