Merge branch 'master' into remove-warning

README.md

@@ -137,7 +137,7 @@ module.exports = BbPromise.try(() => {
       loaders: [ ... ]
     }
   }));
-};
+});
 ```
 
 ### serverless-webpack lib export helper

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,

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...'));
         }
       });
     };

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;
 };
 

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