Skip compile & packaging if --no-build is set (#560)

* Add copyExistingArtifacts to packageModules * Refactor packageModules * Set service path * Generate artifact name from service * Output artifacts to .webpack before copying * Set artifact name for service packaging * Skip webpack:compile if --no-build is set * Add webpack:package:copyExistingArtifacts hook * Make packageModules & packExternalModules no-op if skipCompile is set * Refactor packageModules * Remove artifact location setting from packageModules * Update cleanup to check this.keepOutputDirectory * Fix path join on Windows Co-authored-by: Miguel A. Calles MBA <44813512+miguel-a-calles-mba@users.noreply.github.com>
serverless-heaven · Jan 25, 2021 · e3edab0 · e3edab0
1 parent 3369e9f
commit e3edab0
Show file tree

Hide file tree

Showing 11 changed files with 405 additions and 79 deletions.
diff --git a/index.js b/index.js
@@ -81,7 +81,7 @@ class ServerlessWebpack {
           },
           package: {
             type: 'entrypoint',
-            lifecycleEvents: [ 'packExternalModules', 'packageModules' ]
+            lifecycleEvents: [ 'packExternalModules', 'packageModules', 'copyExistingArtifacts' ]
           }
         }
       }
@@ -91,7 +91,7 @@ class ServerlessWebpack {
       'before:package:createDeploymentArtifacts': () =>
         BbPromise.bind(this)
           .then(() => this.serverless.pluginManager.spawn('webpack:validate'))
-          .then(() => this.serverless.pluginManager.spawn('webpack:compile'))
+          .then(() => (this.skipCompile ? BbPromise.resolve() : this.serverless.pluginManager.spawn('webpack:compile')))
           .then(() => this.serverless.pluginManager.spawn('webpack:package')),
 
       'after:package:createDeploymentArtifacts': () => BbPromise.bind(this).then(this.cleanup),
@@ -106,10 +106,6 @@ class ServerlessWebpack {
         BbPromise.bind(this)
           .then(() => {
             lib.webpack.isLocal = true;
-            // --no-build override
-            if (this.options.build === false) {
-              this.skipCompile = true;
-            }
 
             return this.serverless.pluginManager.spawn('webpack:validate');
           })
@@ -159,6 +155,8 @@ class ServerlessWebpack {
 
       'webpack:package:packageModules': () => BbPromise.bind(this).then(this.packageModules),
 
+      'webpack:package:copyExistingArtifacts': () => BbPromise.bind(this).then(this.copyExistingArtifacts),
+
       'before:offline:start': () =>
         BbPromise.bind(this)
           .tap(() => {

diff --git a/index.test.js b/index.test.js
@@ -133,6 +133,7 @@ describe('ServerlessWebpack', () => {
       sandbox.stub(slsw, 'watch').returns(BbPromise.resolve());
       sandbox.stub(slsw, 'wpwatch').returns(BbPromise.resolve());
       sandbox.stub(slsw, 'packExternalModules').returns(BbPromise.resolve());
+      sandbox.stub(slsw, 'copyExistingArtifacts').returns(BbPromise.resolve());
       sandbox.stub(slsw, 'prepareRun').returns(BbPromise.resolve());
       sandbox.stub(slsw, 'watchRun').returns(BbPromise.resolve());
       sandbox.stub(slsw, 'validate').returns(BbPromise.resolve());
@@ -145,6 +146,7 @@ describe('ServerlessWebpack', () => {
 
     beforeEach(() => {
       ServerlessWebpack.lib.webpack.isLocal = false;
+      slsw.skipCompile = false;
     });
 
     after(() => {
@@ -169,6 +171,20 @@ describe('ServerlessWebpack', () => {
                 return null;
               });
             });
+
+            it('should skip compile if requested', () => {
+              slsw.skipCompile = true;
+              return expect(slsw.hooks['before:package:createDeploymentArtifacts']()).to.be.fulfilled.then(() => {
+                expect(slsw.serverless.pluginManager.spawn).to.have.been.calledTwice;
+                expect(slsw.serverless.pluginManager.spawn.firstCall).to.have.been.calledWithExactly(
+                  'webpack:validate'
+                );
+                expect(slsw.serverless.pluginManager.spawn.secondCall).to.have.been.calledWithExactly(
+                  'webpack:package'
+                );
+                return null;
+              });
+            });
           }
         },
         {
@@ -237,7 +253,7 @@ describe('ServerlessWebpack', () => {
             });
 
             it('should skip compile if requested', () => {
-              slsw.options.build = false;
+              slsw.skipCompile = true;
               return expect(slsw.hooks['before:invoke:local:invoke']()).to.be.fulfilled.then(() => {
                 expect(slsw.serverless.pluginManager.spawn).to.have.been.calledOnce;
                 expect(slsw.serverless.pluginManager.spawn).to.have.been.calledWithExactly('webpack:validate');
@@ -361,6 +377,17 @@ describe('ServerlessWebpack', () => {
             });
           }
         },
+        {
+          name: 'webpack:package:copyExistingArtifacts',
+          test: () => {
+            it('should call copyExistingArtifacts', () => {
+              return expect(slsw.hooks['webpack:package:copyExistingArtifacts']()).to.be.fulfilled.then(() => {
+                expect(slsw.copyExistingArtifacts).to.have.been.calledOnce;
+                return null;
+              });
+            });
+          }
+        },
         {
           name: 'before:offline:start',
           test: () => {

diff --git a/lib/cleanup.js b/lib/cleanup.js
@@ -7,7 +7,7 @@ module.exports = {
   cleanup() {
     const webpackOutputPath = this.webpackOutputPath;
 
-    const keepOutputDirectory = this.configuration.keepOutputDirectory;
+    const keepOutputDirectory = this.keepOutputDirectory;
     if (!keepOutputDirectory) {
       this.options.verbose && this.serverless.cli.log(`Remove ${webpackOutputPath}`);
       if (this.serverless.utils.dirExistsSync(webpackOutputPath)) {

diff --git a/lib/packExternalModules.js b/lib/packExternalModules.js
@@ -228,6 +228,10 @@ module.exports = {
    * and performance.
    */
   packExternalModules() {
+    if (this.skipCompile) {
+      return BbPromise.resolve();
+    }
+
     const stats = this.compileStats;
 
     const includes = this.configuration.includeModules;

diff --git a/lib/packageModules.js b/lib/packageModules.js
@@ -11,6 +11,8 @@ const semver = require('semver');
 function setArtifactPath(funcName, func, artifactPath) {
   const version = this.serverless.getVersion();
 
+  this.options.verbose && this.serverless.cli.log(`Setting artifact for function '${funcName}' to '${artifactPath}'`);
+
   // Serverless changed the artifact path location in version 1.18
   if (semver.lt(version, '1.18.0')) {
     func.artifact = artifactPath;
@@ -26,7 +28,8 @@ function setArtifactPath(funcName, func, artifactPath) {
 function zip(directory, name) {
   const zip = archiver.create('zip');
   // Create artifact in temp path and move it to the package path (if any) later
-  const artifactFilePath = path.join(this.serverless.config.servicePath, '.serverless', name);
+  // This allows us to persist the webpackOutputPath and re-use the compiled output
+  const artifactFilePath = path.join(this.webpackOutputPath, name);
   this.serverless.utils.writeFileDir(artifactFilePath);
 
   const output = fs.createWriteStream(artifactFilePath);
@@ -69,13 +72,47 @@ function zip(directory, name) {
   });
 }
 
+function getArtifactLocations(name) {
+  const archiveName = `${name}.zip`;
+
+  const webpackArtifact = path.join(this.webpackOutputPath, archiveName);
+  const serverlessArtifact = path.join('.serverless', archiveName);
+
+  return { webpackArtifact, serverlessArtifact };
+}
+
+function copyArtifactByName(artifactName) {
+  const { webpackArtifact, serverlessArtifact } = getArtifactLocations.call(this, artifactName);
+
+  // Make sure the destination dir exists
+  this.serverless.utils.writeFileDir(serverlessArtifact);
+
+  fs.copyFileSync(webpackArtifact, serverlessArtifact);
+}
+
+function setServiceArtifactPath(artifactPath) {
+  _.set(this.serverless, 'service.package.artifact', artifactPath);
+}
+
+function isIndividialPackaging() {
+  return _.get(this.serverless, 'service.package.individually');
+}
+
+function getArtifactName(entryFunction) {
+  return `${entryFunction.funcName || this.serverless.service.getServiceObject().name}.zip`;
+}
+
 module.exports = {
   packageModules() {
+    if (this.skipCompile) {
+      return BbPromise.resolve();
+    }
+
     const stats = this.compileStats;
 
     return BbPromise.mapSeries(stats.stats, (compileStats, index) => {
       const entryFunction = _.get(this.entryFunctions, index, {});
-      const filename = `${entryFunction.funcName || this.serverless.service.getServiceObject().name}.zip`;
+      const filename = getArtifactName.call(this, entryFunction);
       const modulePath = compileStats.compilation.compiler.outputPath;
 
       const startZip = _.now();
@@ -87,37 +124,52 @@ module.exports = {
             this.serverless.cli.log(
               `Zip ${_.isEmpty(entryFunction) ? 'service' : 'function'}: ${modulePath} [${_.now() - startZip} ms]`
             )
-        )
-        .then(artifactPath => {
-          if (_.get(this.serverless, 'service.package.individually')) {
-            setArtifactPath.call(
-              this,
-              entryFunction.funcName,
-              entryFunction.func,
-              path.relative(this.serverless.config.servicePath, artifactPath)
-            );
-          }
-          return artifactPath;
-        });
-    }).then(artifacts => {
-      if (!_.get(this.serverless, 'service.package.individually') && !_.isEmpty(artifacts)) {
-        // Set the service artifact to all functions
-        const allFunctionNames = this.serverless.service.getAllFunctions();
-        _.forEach(allFunctionNames, funcName => {
-          const func = this.serverless.service.getFunction(funcName);
-          setArtifactPath.call(this, funcName, func, path.relative(this.serverless.config.servicePath, artifacts[0]));
-        });
-        // For Google set the service artifact path
-        if (_.get(this.serverless, 'service.provider.name') === 'google') {
-          _.set(
-            this.serverless,
-            'service.package.artifact',
-            path.relative(this.serverless.config.servicePath, artifacts[0])
-          );
-        }
-      }
+        );
+    });
+  },
+
+  copyExistingArtifacts() {
+    this.serverless.cli.log('Copying existing artifacts...');
+    const allFunctionNames = this.serverless.service.getAllFunctions();
+
+    // Copy artifacts to package location
+    if (isIndividialPackaging.call(this)) {
+      _.forEach(allFunctionNames, funcName => copyArtifactByName.call(this, funcName));
+    } else {
+      // Copy service packaged artifact
+      const serviceName = this.serverless.service.getServiceObject().name;
+      copyArtifactByName.call(this, serviceName);
+    }
+
+    _.forEach(allFunctionNames, funcName => {
+      const func = this.serverless.service.getFunction(funcName);
+
+      const archiveName = isIndividialPackaging.call(this) ? funcName : this.serverless.service.getServiceObject().name;
 
-      return null;
+      const { serverlessArtifact } = getArtifactLocations.call(this, archiveName);
+      setArtifactPath.call(this, funcName, func, serverlessArtifact);
     });
+
+    // Set artifact locations
+    if (isIndividialPackaging.call(this)) {
+      _.forEach(allFunctionNames, funcName => {
+        const func = this.serverless.service.getFunction(funcName);
+
+        const archiveName = funcName;
+
+        const { serverlessArtifact } = getArtifactLocations.call(this, archiveName);
+        setArtifactPath.call(this, funcName, func, serverlessArtifact);
+      });
+    } else {
+      const archiveName = this.serverless.service.getServiceObject().name;
+
+      const { serverlessArtifact } = getArtifactLocations.call(this, archiveName);
+
+      if (_.get(this.serverless, 'service.provider.name') === 'google') {
+        setServiceArtifactPath.call(this, serverlessArtifact);
+      }
+    }
+
+    return BbPromise.resolve();
   }
 };
diff --git a/lib/validate.js b/lib/validate.js
@@ -171,7 +171,9 @@ module.exports = {
         this.webpackConfig.output.path = path.join(this.serverless.config.servicePath, this.options.out);
       }
 
-      if (this.skipCompile) {
+      // Skip compilation with --no-build
+      if (this.options.build === false) {
+        this.skipCompile = true;
         this.serverless.cli.log('Skipping build and using existing compiled output');
         if (!fse.pathExistsSync(this.webpackConfig.output.path)) {
           return BbPromise.reject(new this.serverless.classes.Error('No compiled output found'));

diff --git a/tests/cleanup.test.js b/tests/cleanup.test.js
@@ -55,8 +55,7 @@ describe('cleanup', () => {
       {
         serverless,
         options: {},
-        webpackOutputPath: 'my/Output/Path',
-        configuration: {}
+        webpackOutputPath: 'my/Output/Path'
       },
       baseModule
     );
@@ -96,7 +95,7 @@ describe('cleanup', () => {
     fseMock.removeSync.reset();
 
     const configuredModule = _.assign({}, module, {
-      configuration: { keepOutputDirectory: true }
+      keepOutputDirectory: true
     });
     return expect(configuredModule.cleanup()).to.be.fulfilled.then(() => {
       expect(dirExistsSyncStub).to.not.have.been.calledOnce;

diff --git a/tests/mocks/fs.mock.js b/tests/mocks/fs.mock.js
@@ -23,6 +23,7 @@ module.exports.create = sandbox => {
     readFileSync: sandbox.stub(),
     statSync: sinon.stub().returns(statMock), // Persistent stub
     writeFileSync: sandbox.stub(),
+    copyFileSync: sandbox.stub(),
 
     _streamMock: streamMock,
     _statMock: statMock

diff --git a/tests/packExternalModules.test.js b/tests/packExternalModules.test.js
@@ -334,6 +334,24 @@ describe('packExternalModules', () => {
       );
     });
 
+    it('should do nothing if skipCompile is true', () => {
+      module.configuration = new Configuration({
+        webpack: {
+          includeModules: {
+            packagePath: path.join('locals', 'package.json')
+          }
+        }
+      });
+      module.skipCompile = true;
+      return expect(module.packExternalModules()).to.be.fulfilled.then(() =>
+        BbPromise.all([
+          expect(fsExtraMock.copy).to.not.have.been.called,
+          expect(packagerFactoryMock.get).to.not.have.been.called,
+          expect(writeFileSyncStub).to.not.have.been.called
+        ])
+      );
+    });
+
     it('should copy needed package sections if available', () => {
       const originalPackageJSON = {
         name: 'test-service',