From ce870baf583481894147bfd5714e812a4b83cd62 Mon Sep 17 00:00:00 2001 From: Emilien Escalle Date: Wed, 9 Jan 2019 12:34:10 +0100 Subject: [PATCH 1/7] Fixes for webpack 5 support * Uses `modulesIterable` instead of `forEachModule` (already deprecated in version 4) * Iterate over chunks with `for ... of`` because stats.compilation.chunks is a `Set` --- lib/packExternalModules.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/packExternalModules.js b/lib/packExternalModules.js index 6b1fc9ef45..e3897abd43 100644 --- a/lib/packExternalModules.js +++ b/lib/packExternalModules.js @@ -155,17 +155,17 @@ function findExternalOrigin(issuer) { function getExternalModules(stats) { const externals = new Set(); - _.forEach(stats.compilation.chunks, chunk => { + for (let chunk of stats.compilation.chunks) { // Explore each module within the chunk (built inputs): - chunk.forEachModule(module => { + for (const module of chunk.modulesIterable) { if (isExternalModule(module)) { externals.add({ origin: _.get(findExternalOrigin(module.issuer), 'rawRequest'), external: getExternalModuleName(module) }); } - }); - }); + }; + }; return Array.from(externals); } From 920a83455009f7b68c0488ce620b7820e278d94d Mon Sep 17 00:00:00 2001 From: "emilien.escalle" Date: Wed, 9 Jan 2019 13:18:31 +0100 Subject: [PATCH 2/7] Fix tests --- lib/packExternalModules.js | 279 ++++++++++--------- tests/packExternalModules.test.js | 444 +++++++++++++++--------------- 2 files changed, 365 insertions(+), 358 deletions(-) diff --git a/lib/packExternalModules.js b/lib/packExternalModules.js index e3897abd43..aad7a8f994 100644 --- a/lib/packExternalModules.js +++ b/lib/packExternalModules.js @@ -153,9 +153,16 @@ function findExternalOrigin(issuer) { } function getExternalModules(stats) { - const externals = new Set(); + if (!stats.compilation.chunks) { + return []; + } + const externals = new Set(); for (let chunk of stats.compilation.chunks) { + if (!chunk.modulesIterable) { + continue; + } + // Explore each module within the chunk (built inputs): for (const module of chunk.modulesIterable) { if (isExternalModule(module)) { @@ -206,146 +213,146 @@ module.exports = { // Determine and create packager return BbPromise.try(() => Packagers.get.call(this, this.configuration.packager)) - .then(packager => { - // Fetch needed original package.json sections - const sectionNames = packager.copyPackageSectionNames; - const packageJson = this.serverless.utils.readFileSync(packageJsonPath); - const packageSections = _.pick(packageJson, sectionNames); - if (!_.isEmpty(packageSections)) { - this.options.verbose && this.serverless.cli.log(`Using package.json sections ${_.join(_.keys(packageSections), ', ')}`); - } + .then(packager => { + // Fetch needed original package.json sections + const sectionNames = packager.copyPackageSectionNames; + const packageJson = this.serverless.utils.readFileSync(packageJsonPath); + const packageSections = _.pick(packageJson, sectionNames); + if (!_.isEmpty(packageSections)) { + this.options.verbose && this.serverless.cli.log(`Using package.json sections ${_.join(_.keys(packageSections), ', ')}`); + } - // Get first level dependency graph - this.options.verbose && this.serverless.cli.log(`Fetch dependency graph from ${packageJsonPath}`); + // Get first level dependency graph + this.options.verbose && this.serverless.cli.log(`Fetch dependency graph from ${packageJsonPath}`); - return packager.getProdDependencies(path.dirname(packageJsonPath), 1) - .then(dependencyGraph => { - const problems = _.get(dependencyGraph, 'problems', []); - if (this.options.verbose && !_.isEmpty(problems)) { - this.serverless.cli.log(`Ignoring ${_.size(problems)} NPM errors:`); - _.forEach(problems, problem => { - this.serverless.cli.log(`=> ${problem}`); - }); - } - - // (1) Generate dependency composition - const compositeModules = _.uniq(_.flatMap(stats.stats, compileStats => { - const externalModules = _.concat( - getExternalModules.call(this, compileStats), - _.map(packageForceIncludes, whitelistedPackage => ({ external: whitelistedPackage })) - ); - return getProdModules.call(this, externalModules, packagePath, dependencyGraph, packageForceExcludes); - })); - removeExcludedModules.call(this, compositeModules, packageForceExcludes, true); - - if (_.isEmpty(compositeModules)) { - // The compiled code does not reference any external modules at all - this.serverless.cli.log('No external modules needed'); - return BbPromise.resolve(); - } - - // (1.a) Install all needed modules - const compositeModulePath = path.join(this.webpackOutputPath, 'dependencies'); - const compositePackageJson = path.join(compositeModulePath, 'package.json'); - - // (1.a.1) Create a package.json - const compositePackage = _.defaults({ - name: this.serverless.service.service, - version: '1.0.0', - description: `Packaged externals for ${this.serverless.service.service}`, - private: true, - scripts: packageScripts - }, packageSections); - const relPath = path.relative(compositeModulePath, path.dirname(packageJsonPath)); - addModulesToPackageJson(compositeModules, compositePackage, relPath); - this.serverless.utils.writeFileSync(compositePackageJson, JSON.stringify(compositePackage, null, 2)); - - // (1.a.2) Copy package-lock.json if it exists, to prevent unwanted upgrades - const packageLockPath = path.join(path.dirname(packageJsonPath), packager.lockfileName); - let hasPackageLock = false; - return BbPromise.fromCallback(cb => fse.pathExists(packageLockPath, cb)) - .then(exists => { - if (exists) { - this.serverless.cli.log('Package lock found - Using locked versions'); - try { - let packageLockFile = this.serverless.utils.readFileSync(packageLockPath); - packageLockFile = packager.rebaseLockfile(relPath, packageLockFile); - if (_.isObject(packageLockFile)) { - packageLockFile = JSON.stringify(packageLockFile, null, 2); - } - - this.serverless.utils.writeFileSync(path.join(compositeModulePath, packager.lockfileName), packageLockFile); - hasPackageLock = true; - } catch(err) { - this.serverless.cli.log(`Warning: Could not read lock file: ${err.message}`); + return packager.getProdDependencies(path.dirname(packageJsonPath), 1) + .then(dependencyGraph => { + const problems = _.get(dependencyGraph, 'problems', []); + if (this.options.verbose && !_.isEmpty(problems)) { + this.serverless.cli.log(`Ignoring ${_.size(problems)} NPM errors:`); + _.forEach(problems, problem => { + this.serverless.cli.log(`=> ${problem}`); + }); } - } - return BbPromise.resolve(); - }) - .then(() => { - const start = _.now(); - this.serverless.cli.log('Packing external modules: ' + compositeModules.join(', ')); - return packager.install(compositeModulePath, this.configuration.packagerOptions) - .then(() => this.options.verbose && this.serverless.cli.log(`Package took [${_.now() - start} ms]`)) - .return(stats.stats); - }) - .mapSeries(compileStats => { - const modulePath = compileStats.compilation.compiler.outputPath; - - // Create package.json - const modulePackageJson = path.join(modulePath, 'package.json'); - const modulePackage = _.defaults({ - name: this.serverless.service.service, - version: '1.0.0', - description: `Packaged externals for ${this.serverless.service.service}`, - private: true, - scripts: packageScripts, - dependencies: {} - }, packageSections); - const prodModules = getProdModules.call(this, - _.concat( - getExternalModules.call(this, compileStats), - _.map(packageForceIncludes, whitelistedPackage => ({ external: whitelistedPackage })) - ), packagePath, dependencyGraph, packageForceExcludes); - removeExcludedModules.call(this, prodModules, packageForceExcludes); - const relPath = path.relative(modulePath, path.dirname(packageJsonPath)); - addModulesToPackageJson(prodModules, modulePackage, relPath); - this.serverless.utils.writeFileSync(modulePackageJson, JSON.stringify(modulePackage, null, 2)); - - // GOOGLE: Copy modules only if not google-cloud-functions - // GCF Auto installs the package json - if (_.get(this.serverless, 'service.provider.name') === 'google') { - return BbPromise.resolve(); - } - - const startCopy = _.now(); - return BbPromise.try(() => { - // Only copy dependency modules if demanded by packager - if (packager.mustCopyModules) { - return BbPromise.fromCallback(callback => fse.copy(path.join(compositeModulePath, 'node_modules'), path.join(modulePath, 'node_modules'), callback)); + + // (1) Generate dependency composition + const compositeModules = _.uniq(_.flatMap(stats.stats, compileStats => { + const externalModules = _.concat( + getExternalModules.call(this, compileStats), + _.map(packageForceIncludes, whitelistedPackage => ({ external: whitelistedPackage })) + ); + return getProdModules.call(this, externalModules, packagePath, dependencyGraph, packageForceExcludes); + })); + removeExcludedModules.call(this, compositeModules, packageForceExcludes, true); + + if (_.isEmpty(compositeModules)) { + // The compiled code does not reference any external modules at all + this.serverless.cli.log('No external modules needed'); + return BbPromise.resolve(); } - return BbPromise.resolve(); - }) - .then(() => hasPackageLock ? - BbPromise.fromCallback(callback => fse.copy(path.join(compositeModulePath, packager.lockfileName), path.join(modulePath, packager.lockfileName), callback)) : - BbPromise.resolve() - ) - .tap(() => this.options.verbose && this.serverless.cli.log(`Copy modules: ${modulePath} [${_.now() - startCopy} ms]`)) - .then(() => { - // Prune extraneous packages - removes not needed ones - const startPrune = _.now(); - return packager.prune(modulePath, this.configuration.packagerOptions) - .tap(() => this.options.verbose && this.serverless.cli.log(`Prune: ${modulePath} [${_.now() - startPrune} ms]`)); - }) - .then(() => { - // Prune extraneous packages - removes not needed ones - const startRunScripts = _.now(); - return packager.runScripts(modulePath, _.keys(packageScripts)) - .tap(() => this.options.verbose && this.serverless.cli.log(`Run scripts: ${modulePath} [${_.now() - startRunScripts} ms]`)); + + // (1.a) Install all needed modules + const compositeModulePath = path.join(this.webpackOutputPath, 'dependencies'); + const compositePackageJson = path.join(compositeModulePath, 'package.json'); + + // (1.a.1) Create a package.json + const compositePackage = _.defaults({ + name: this.serverless.service.service, + version: '1.0.0', + description: `Packaged externals for ${this.serverless.service.service}`, + private: true, + scripts: packageScripts + }, packageSections); + const relPath = path.relative(compositeModulePath, path.dirname(packageJsonPath)); + addModulesToPackageJson(compositeModules, compositePackage, relPath); + this.serverless.utils.writeFileSync(compositePackageJson, JSON.stringify(compositePackage, null, 2)); + + // (1.a.2) Copy package-lock.json if it exists, to prevent unwanted upgrades + const packageLockPath = path.join(path.dirname(packageJsonPath), packager.lockfileName); + let hasPackageLock = false; + return BbPromise.fromCallback(cb => fse.pathExists(packageLockPath, cb)) + .then(exists => { + if (exists) { + this.serverless.cli.log('Package lock found - Using locked versions'); + try { + let packageLockFile = this.serverless.utils.readFileSync(packageLockPath); + packageLockFile = packager.rebaseLockfile(relPath, packageLockFile); + if (_.isObject(packageLockFile)) { + packageLockFile = JSON.stringify(packageLockFile, null, 2); + } + + this.serverless.utils.writeFileSync(path.join(compositeModulePath, packager.lockfileName), packageLockFile); + hasPackageLock = true; + } catch (err) { + this.serverless.cli.log(`Warning: Could not read lock file: ${err.message}`); + } + } + return BbPromise.resolve(); + }) + .then(() => { + const start = _.now(); + this.serverless.cli.log('Packing external modules: ' + compositeModules.join(', ')); + return packager.install(compositeModulePath, this.configuration.packagerOptions) + .then(() => this.options.verbose && this.serverless.cli.log(`Package took [${_.now() - start} ms]`)) + .return(stats.stats); + }) + .mapSeries(compileStats => { + const modulePath = compileStats.compilation.compiler.outputPath; + + // Create package.json + const modulePackageJson = path.join(modulePath, 'package.json'); + const modulePackage = _.defaults({ + name: this.serverless.service.service, + version: '1.0.0', + description: `Packaged externals for ${this.serverless.service.service}`, + private: true, + scripts: packageScripts, + dependencies: {} + }, packageSections); + const prodModules = getProdModules.call(this, + _.concat( + getExternalModules.call(this, compileStats), + _.map(packageForceIncludes, whitelistedPackage => ({ external: whitelistedPackage })) + ), packagePath, dependencyGraph, packageForceExcludes); + removeExcludedModules.call(this, prodModules, packageForceExcludes); + const relPath = path.relative(modulePath, path.dirname(packageJsonPath)); + addModulesToPackageJson(prodModules, modulePackage, relPath); + this.serverless.utils.writeFileSync(modulePackageJson, JSON.stringify(modulePackage, null, 2)); + + // GOOGLE: Copy modules only if not google-cloud-functions + // GCF Auto installs the package json + if (_.get(this.serverless, 'service.provider.name') === 'google') { + return BbPromise.resolve(); + } + + const startCopy = _.now(); + return BbPromise.try(() => { + // Only copy dependency modules if demanded by packager + if (packager.mustCopyModules) { + return BbPromise.fromCallback(callback => fse.copy(path.join(compositeModulePath, 'node_modules'), path.join(modulePath, 'node_modules'), callback)); + } + return BbPromise.resolve(); + }) + .then(() => hasPackageLock ? + BbPromise.fromCallback(callback => fse.copy(path.join(compositeModulePath, packager.lockfileName), path.join(modulePath, packager.lockfileName), callback)) : + BbPromise.resolve() + ) + .tap(() => this.options.verbose && this.serverless.cli.log(`Copy modules: ${modulePath} [${_.now() - startCopy} ms]`)) + .then(() => { + // Prune extraneous packages - removes not needed ones + const startPrune = _.now(); + return packager.prune(modulePath, this.configuration.packagerOptions) + .tap(() => this.options.verbose && this.serverless.cli.log(`Prune: ${modulePath} [${_.now() - startPrune} ms]`)); + }) + .then(() => { + // Prune extraneous packages - removes not needed ones + const startRunScripts = _.now(); + return packager.runScripts(modulePath, _.keys(packageScripts)) + .tap(() => this.options.verbose && this.serverless.cli.log(`Run scripts: ${modulePath} [${_.now() - startRunScripts} ms]`)); + }); + }) + .return(); }); - }) - .return(); }); - }); } }; diff --git a/tests/packExternalModules.test.js b/tests/packExternalModules.test.js index c608396ae2..5bc9d612d2 100644 --- a/tests/packExternalModules.test.js +++ b/tests/packExternalModules.test.js @@ -25,8 +25,8 @@ class ChunkMock { this._modules = modules; } - forEachModule(fn) { - _.forEach(this._modules, fn); + get modulesIterable() { + return this._modules; } } @@ -34,7 +34,7 @@ const packagerMockFactory = { create(sandbox) { const packagerMock = { lockfileName: 'mocked-lock.json', - copyPackageSectionNames: [ 'section1', 'section2' ], + copyPackageSectionNames: ['section1', 'section2'], mustCopyModules: true, rebaseLockfile: sandbox.stub(), getProdDependencies: sandbox.stub(), @@ -106,10 +106,10 @@ describe('packExternalModules', () => { options: { verbose: true }, - configuration: new Configuration({ + configuration: new Configuration({ webpack: { includeModules: true - } + } }) }, baseModule); }); @@ -316,11 +316,11 @@ describe('packExternalModules', () => { module.configuration = new Configuration(); module.compileStats = { stats: [] }; 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, - ])); + .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', () => { @@ -331,7 +331,7 @@ describe('packExternalModules', () => { private: true, scripts: {}, section1: { - value: 'myValue' + value: 'myValue' }, dependencies: { '@scoped/vendor': '1.0.0', @@ -377,19 +377,19 @@ describe('packExternalModules', () => { packagerMock.runScripts.returns(BbPromise.resolve()); module.compileStats = stats; return expect(module.packExternalModules()).to.be.fulfilled - .then(() => BbPromise.all([ - // The module package JSON and the composite one should have been stored - expect(writeFileSyncStub).to.have.been.calledTwice, - expect(writeFileSyncStub.firstCall.args[1]).to.equal(JSON.stringify(expectedCompositePackageJSON, null, 2)), - expect(writeFileSyncStub.secondCall.args[1]).to.equal(JSON.stringify(expectedPackageJSON, null, 2)), - // The modules should have been copied - expect(fsExtraMock.copy).to.have.been.calledOnce, - // npm ls and npm prune should have been called - expect(packagerMock.getProdDependencies).to.have.been.calledOnce, - expect(packagerMock.install).to.have.been.calledOnce, - expect(packagerMock.prune).to.have.been.calledOnce, - expect(packagerMock.runScripts).to.have.been.calledOnce, - ])); + .then(() => BbPromise.all([ + // The module package JSON and the composite one should have been stored + expect(writeFileSyncStub).to.have.been.calledTwice, + expect(writeFileSyncStub.firstCall.args[1]).to.equal(JSON.stringify(expectedCompositePackageJSON, null, 2)), + expect(writeFileSyncStub.secondCall.args[1]).to.equal(JSON.stringify(expectedPackageJSON, null, 2)), + // The modules should have been copied + expect(fsExtraMock.copy).to.have.been.calledOnce, + // npm ls and npm prune should have been called + expect(packagerMock.getProdDependencies).to.have.been.calledOnce, + expect(packagerMock.install).to.have.been.calledOnce, + expect(packagerMock.prune).to.have.been.calledOnce, + expect(packagerMock.runScripts).to.have.been.calledOnce, + ])); }); it('should install external modules', () => { @@ -427,19 +427,19 @@ describe('packExternalModules', () => { packagerMock.runScripts.returns(BbPromise.resolve()); module.compileStats = stats; return expect(module.packExternalModules()).to.be.fulfilled - .then(() => BbPromise.all([ - // The module package JSON and the composite one should have been stored - expect(writeFileSyncStub).to.have.been.calledTwice, - expect(writeFileSyncStub.firstCall.args[1]).to.equal(JSON.stringify(expectedCompositePackageJSON, null, 2)), - expect(writeFileSyncStub.secondCall.args[1]).to.equal(JSON.stringify(expectedPackageJSON, null, 2)), - // The modules should have been copied - expect(fsExtraMock.copy).to.have.been.calledOnce, - // npm ls and npm prune should have been called - expect(packagerMock.getProdDependencies).to.have.been.calledOnce, - expect(packagerMock.install).to.have.been.calledOnce, - expect(packagerMock.prune).to.have.been.calledOnce, - expect(packagerMock.runScripts).to.have.been.calledOnce, - ])); + .then(() => BbPromise.all([ + // The module package JSON and the composite one should have been stored + expect(writeFileSyncStub).to.have.been.calledTwice, + expect(writeFileSyncStub.firstCall.args[1]).to.equal(JSON.stringify(expectedCompositePackageJSON, null, 2)), + expect(writeFileSyncStub.secondCall.args[1]).to.equal(JSON.stringify(expectedPackageJSON, null, 2)), + // The modules should have been copied + expect(fsExtraMock.copy).to.have.been.calledOnce, + // npm ls and npm prune should have been called + expect(packagerMock.getProdDependencies).to.have.been.calledOnce, + expect(packagerMock.install).to.have.been.calledOnce, + expect(packagerMock.prune).to.have.been.calledOnce, + expect(packagerMock.runScripts).to.have.been.calledOnce, + ])); }); it('should rebase file references', () => { @@ -513,25 +513,25 @@ describe('packExternalModules', () => { mockery.registerMock(path.join(process.cwd(), 'locals', 'package.json'), packageLocalRefMock); return expect(module.packExternalModules()).to.be.fulfilled - .then(() => BbPromise.all([ - // The module package JSON and the composite one should have been stored - expect(writeFileSyncStub).to.have.been.calledThrice, - expect(writeFileSyncStub.firstCall.args[1]).to.equal(JSON.stringify(expectedCompositePackageJSON, null, 2)), - expect(writeFileSyncStub.thirdCall.args[1]).to.equal(JSON.stringify(expectedPackageJSON, null, 2)), - // The modules and the lock file should have been copied - expect(fsExtraMock.copy).to.have.been.calledTwice, - // Lock file rebase should have been called - expect(packagerMock.rebaseLockfile).to.have.been.calledOnce, - expect(packagerMock.rebaseLockfile).to.have.been.calledWith(sinon.match.any, sinon.match(fakePackageLockJSON)), - // npm ls and npm prune should have been called - expect(packagerMock.getProdDependencies).to.have.been.calledOnce, - expect(packagerMock.install).to.have.been.calledOnce, - expect(packagerMock.prune).to.have.been.calledOnce, - expect(packagerMock.runScripts).to.have.been.calledOnce, - ])) - .finally(() => { - process.cwd.restore(); - }); + .then(() => BbPromise.all([ + // The module package JSON and the composite one should have been stored + expect(writeFileSyncStub).to.have.been.calledThrice, + expect(writeFileSyncStub.firstCall.args[1]).to.equal(JSON.stringify(expectedCompositePackageJSON, null, 2)), + expect(writeFileSyncStub.thirdCall.args[1]).to.equal(JSON.stringify(expectedPackageJSON, null, 2)), + // The modules and the lock file should have been copied + expect(fsExtraMock.copy).to.have.been.calledTwice, + // Lock file rebase should have been called + expect(packagerMock.rebaseLockfile).to.have.been.calledOnce, + expect(packagerMock.rebaseLockfile).to.have.been.calledWith(sinon.match.any, sinon.match(fakePackageLockJSON)), + // npm ls and npm prune should have been called + expect(packagerMock.getProdDependencies).to.have.been.calledOnce, + expect(packagerMock.install).to.have.been.calledOnce, + expect(packagerMock.prune).to.have.been.calledOnce, + expect(packagerMock.runScripts).to.have.been.calledOnce, + ])) + .finally(() => { + process.cwd.restore(); + }); }); it('should skip module copy for Google provider', () => { @@ -570,19 +570,19 @@ describe('packExternalModules', () => { packagerMock.runScripts.returns(BbPromise.resolve()); module.compileStats = stats; return expect(module.packExternalModules()).to.be.fulfilled - .then(() => BbPromise.all([ - // The module package JSON and the composite one should have been stored - expect(writeFileSyncStub).to.have.been.calledTwice, - expect(writeFileSyncStub.firstCall.args[1]).to.equal(JSON.stringify(expectedCompositePackageJSON, null, 2)), - expect(writeFileSyncStub.secondCall.args[1]).to.equal(JSON.stringify(expectedPackageJSON, null, 2)), - // The modules should have been copied - expect(fsExtraMock.copy).to.have.not.been.called, - // npm ls and npm prune should have been called - expect(packagerMock.getProdDependencies).to.have.been.calledOnce, - expect(packagerMock.install).to.have.been.calledOnce, - expect(packagerMock.prune).to.not.have.been.called, - expect(packagerMock.runScripts).to.not.have.been.called, - ])); + .then(() => BbPromise.all([ + // The module package JSON and the composite one should have been stored + expect(writeFileSyncStub).to.have.been.calledTwice, + expect(writeFileSyncStub.firstCall.args[1]).to.equal(JSON.stringify(expectedCompositePackageJSON, null, 2)), + expect(writeFileSyncStub.secondCall.args[1]).to.equal(JSON.stringify(expectedPackageJSON, null, 2)), + // The modules should have been copied + expect(fsExtraMock.copy).to.have.not.been.called, + // npm ls and npm prune should have been called + expect(packagerMock.getProdDependencies).to.have.been.calledOnce, + expect(packagerMock.install).to.have.been.calledOnce, + expect(packagerMock.prune).to.not.have.been.called, + expect(packagerMock.runScripts).to.not.have.been.called, + ])); }); it('should reject if packager install fails', () => { @@ -595,13 +595,13 @@ describe('packExternalModules', () => { packagerMock.runScripts.returns(BbPromise.resolve()); module.compileStats = stats; return expect(module.packExternalModules()).to.be.rejectedWith('npm install failed') - .then(() => BbPromise.all([ - // npm ls and npm install should have been called - expect(packagerMock.getProdDependencies).to.have.been.calledOnce, - expect(packagerMock.install).to.have.been.calledOnce, - expect(packagerMock.prune).to.not.have.been.called, - expect(packagerMock.runScripts).to.not.have.been.called, - ])); + .then(() => BbPromise.all([ + // npm ls and npm install should have been called + expect(packagerMock.getProdDependencies).to.have.been.calledOnce, + expect(packagerMock.install).to.have.been.calledOnce, + expect(packagerMock.prune).to.not.have.been.called, + expect(packagerMock.runScripts).to.not.have.been.called, + ])); }); it('should reject if packager returns a critical error', () => { @@ -611,17 +611,17 @@ describe('packExternalModules', () => { packagerMock.getProdDependencies.callsFake(() => BbPromise.reject(new Error('something went wrong'))); module.compileStats = stats; return expect(module.packExternalModules()).to.be.rejectedWith('something went wrong') - .then(() => BbPromise.all([ - // The module package JSON and the composite one should have been stored - expect(writeFileSyncStub).to.not.have.been.called, - // The modules should have been copied - expect(fsExtraMock.copy).to.not.have.been.called, - // npm ls and npm prune should have been called - expect(packagerMock.getProdDependencies).to.have.been.calledOnce, - expect(packagerMock.install).to.not.have.been.called, - expect(packagerMock.prune).to.not.have.been.called, - expect(packagerMock.runScripts).to.not.have.been.called, - ])); + .then(() => BbPromise.all([ + // The module package JSON and the composite one should have been stored + expect(writeFileSyncStub).to.not.have.been.called, + // The modules should have been copied + expect(fsExtraMock.copy).to.not.have.been.called, + // npm ls and npm prune should have been called + expect(packagerMock.getProdDependencies).to.have.been.calledOnce, + expect(packagerMock.install).to.not.have.been.called, + expect(packagerMock.prune).to.not.have.been.called, + expect(packagerMock.runScripts).to.not.have.been.called, + ])); }); it('should not install modules if no external modules are reported', () => { @@ -630,17 +630,17 @@ describe('packExternalModules', () => { packagerMock.getProdDependencies.returns(BbPromise.resolve()); module.compileStats = noExtStats; return expect(module.packExternalModules()).to.be.fulfilled - .then(() => BbPromise.all([ - // The module package JSON and the composite one should have been stored - expect(writeFileSyncStub).to.not.have.been.called, - // The modules should have been copied - expect(fsExtraMock.copy).to.not.have.been.called, - // npm install and npm prune should not have been called - expect(packagerMock.getProdDependencies).to.have.been.calledOnce, - expect(packagerMock.install).to.not.have.been.called, - expect(packagerMock.prune).to.not.have.been.called, - expect(packagerMock.runScripts).to.not.have.been.called, - ])); + .then(() => BbPromise.all([ + // The module package JSON and the composite one should have been stored + expect(writeFileSyncStub).to.not.have.been.called, + // The modules should have been copied + expect(fsExtraMock.copy).to.not.have.been.called, + // npm install and npm prune should not have been called + expect(packagerMock.getProdDependencies).to.have.been.calledOnce, + expect(packagerMock.install).to.not.have.been.called, + expect(packagerMock.prune).to.not.have.been.called, + expect(packagerMock.runScripts).to.not.have.been.called, + ])); }); it('should report ignored packager problems in verbose mode', () => { @@ -658,12 +658,12 @@ describe('packExternalModules', () => { packagerMock.runScripts.returns(BbPromise.resolve()); module.compileStats = stats; return expect(module.packExternalModules()).to.be.fulfilled - .then(() => { - expect(packagerMock.getProdDependencies).to.have.been.calledOnce; - expect(serverless.cli.log).to.have.been.calledWith('=> Problem 1'); - expect(serverless.cli.log).to.have.been.calledWith('=> Problem 2'); - return null; - }); + .then(() => { + expect(packagerMock.getProdDependencies).to.have.been.calledOnce; + expect(serverless.cli.log).to.have.been.calledWith('=> Problem 1'); + expect(serverless.cli.log).to.have.been.calledWith('=> Problem 2'); + return null; + }); }); it('should install external modules when forced', () => { @@ -709,19 +709,19 @@ describe('packExternalModules', () => { packagerMock.runScripts.returns(BbPromise.resolve()); module.compileStats = stats; return expect(module.packExternalModules()).to.be.fulfilled - .then(() => BbPromise.all([ - // The module package JSON and the composite one should have been stored - expect(writeFileSyncStub).to.have.been.calledTwice, - expect(writeFileSyncStub.firstCall.args[1]).to.equal(JSON.stringify(expectedCompositePackageJSON, null, 2)), - expect(writeFileSyncStub.secondCall.args[1]).to.equal(JSON.stringify(expectedPackageJSON, null, 2)), - // The modules should have been copied - expect(fsExtraMock.copy).to.have.been.calledOnce, - // npm ls and npm prune should have been called - expect(packagerMock.getProdDependencies).to.have.been.calledOnce, - expect(packagerMock.install).to.have.been.calledOnce, - expect(packagerMock.prune).to.have.been.calledOnce, - expect(packagerMock.runScripts).to.have.been.calledOnce, - ])); + .then(() => BbPromise.all([ + // The module package JSON and the composite one should have been stored + expect(writeFileSyncStub).to.have.been.calledTwice, + expect(writeFileSyncStub.firstCall.args[1]).to.equal(JSON.stringify(expectedCompositePackageJSON, null, 2)), + expect(writeFileSyncStub.secondCall.args[1]).to.equal(JSON.stringify(expectedPackageJSON, null, 2)), + // The modules should have been copied + expect(fsExtraMock.copy).to.have.been.calledOnce, + // npm ls and npm prune should have been called + expect(packagerMock.getProdDependencies).to.have.been.calledOnce, + expect(packagerMock.install).to.have.been.calledOnce, + expect(packagerMock.prune).to.have.been.calledOnce, + expect(packagerMock.runScripts).to.have.been.calledOnce, + ])); }); it('should add forced external modules without version when not in production dependencies', () => { @@ -767,19 +767,19 @@ describe('packExternalModules', () => { packagerMock.runScripts.returns(BbPromise.resolve()); module.compileStats = stats; return expect(module.packExternalModules()).to.be.fulfilled - .then(() => BbPromise.all([ - // The module package JSON and the composite one should have been stored - expect(writeFileSyncStub).to.have.been.calledTwice, - expect(writeFileSyncStub.firstCall.args[1]).to.equal(JSON.stringify(expectedCompositePackageJSON, null, 2)), - expect(writeFileSyncStub.secondCall.args[1]).to.equal(JSON.stringify(expectedPackageJSON, null, 2)), - // The modules should have been copied - expect(fsExtraMock.copy).to.have.been.calledOnce, - // npm ls and npm prune should have been called - expect(packagerMock.getProdDependencies).to.have.been.calledOnce, - expect(packagerMock.install).to.have.been.calledOnce, - expect(packagerMock.prune).to.have.been.calledOnce, - expect(packagerMock.runScripts).to.have.been.calledOnce, - ])); + .then(() => BbPromise.all([ + // The module package JSON and the composite one should have been stored + expect(writeFileSyncStub).to.have.been.calledTwice, + expect(writeFileSyncStub.firstCall.args[1]).to.equal(JSON.stringify(expectedCompositePackageJSON, null, 2)), + expect(writeFileSyncStub.secondCall.args[1]).to.equal(JSON.stringify(expectedPackageJSON, null, 2)), + // The modules should have been copied + expect(fsExtraMock.copy).to.have.been.calledOnce, + // npm ls and npm prune should have been called + expect(packagerMock.getProdDependencies).to.have.been.calledOnce, + expect(packagerMock.install).to.have.been.calledOnce, + expect(packagerMock.prune).to.have.been.calledOnce, + expect(packagerMock.runScripts).to.have.been.calledOnce, + ])); }); it('should exclude external modules when forced', () => { @@ -824,19 +824,19 @@ describe('packExternalModules', () => { packagerMock.runScripts.returns(BbPromise.resolve()); module.compileStats = stats; return expect(module.packExternalModules()).to.be.fulfilled - .then(() => BbPromise.all([ - // The module package JSON and the composite one should have been stored - expect(writeFileSyncStub).to.have.been.calledTwice, - expect(writeFileSyncStub.firstCall.args[1]).to.equal(JSON.stringify(expectedCompositePackageJSON, null, 2)), - expect(writeFileSyncStub.secondCall.args[1]).to.equal(JSON.stringify(expectedPackageJSON, null, 2)), - // The modules should have been copied - expect(fsExtraMock.copy).to.have.been.calledOnce, - // npm ls and npm prune should have been called - expect(packagerMock.getProdDependencies).to.have.been.calledOnce, - expect(packagerMock.install).to.have.been.calledOnce, - expect(packagerMock.prune).to.have.been.calledOnce, - expect(packagerMock.runScripts).to.have.been.calledOnce, - ])); + .then(() => BbPromise.all([ + // The module package JSON and the composite one should have been stored + expect(writeFileSyncStub).to.have.been.calledTwice, + expect(writeFileSyncStub.firstCall.args[1]).to.equal(JSON.stringify(expectedCompositePackageJSON, null, 2)), + expect(writeFileSyncStub.secondCall.args[1]).to.equal(JSON.stringify(expectedPackageJSON, null, 2)), + // The modules should have been copied + expect(fsExtraMock.copy).to.have.been.calledOnce, + // npm ls and npm prune should have been called + expect(packagerMock.getProdDependencies).to.have.been.calledOnce, + expect(packagerMock.install).to.have.been.calledOnce, + expect(packagerMock.prune).to.have.been.calledOnce, + expect(packagerMock.runScripts).to.have.been.calledOnce, + ])); }); it('should reject if devDependency is required at runtime', () => { @@ -849,14 +849,14 @@ describe('packExternalModules', () => { packagerMock.runScripts.returns(BbPromise.resolve()); module.compileStats = statsWithDevDependency; return expect(module.packExternalModules()).to.be.rejectedWith('Serverless-webpack dependency error: eslint.') - .then(() => BbPromise.all([ - expect(module.serverless.cli.log).to.have.been.calledWith(sinon.match(/ERROR: Runtime dependency 'eslint' found in devDependencies/)), - // npm ls and npm install should have been called - expect(packagerMock.getProdDependencies).to.have.been.calledOnce, - expect(packagerMock.install).to.not.have.been.called, - expect(packagerMock.prune).to.not.have.been.called, - expect(packagerMock.runScripts).to.not.have.been.called, - ])); + .then(() => BbPromise.all([ + expect(module.serverless.cli.log).to.have.been.calledWith(sinon.match(/ERROR: Runtime dependency 'eslint' found in devDependencies/)), + // npm ls and npm install should have been called + expect(packagerMock.getProdDependencies).to.have.been.calledOnce, + expect(packagerMock.install).to.not.have.been.called, + expect(packagerMock.prune).to.not.have.been.called, + expect(packagerMock.runScripts).to.not.have.been.called, + ])); }); it('should ignore aws-sdk if set only in devDependencies', () => { @@ -877,14 +877,14 @@ describe('packExternalModules', () => { module.compileStats = statsWithIgnoredDevDependency; mockery.registerMock(path.join(process.cwd(), 'ignoreDevDeps', 'package.json'), packageIgnoredDevDepsMock); return expect(module.packExternalModules()).to.be.fulfilled - .then(() => BbPromise.all([ - expect(module.serverless.cli.log).to.have.been.calledWith(sinon.match(/INFO: Runtime dependency 'aws-sdk' found in devDependencies/)), - // npm ls and npm install should have been called - expect(packagerMock.getProdDependencies).to.have.been.calledOnce, - expect(packagerMock.install).to.have.been.calledOnce, - expect(packagerMock.prune).to.have.been.calledOnce, - expect(packagerMock.runScripts).to.have.been.calledOnce, - ])); + .then(() => BbPromise.all([ + expect(module.serverless.cli.log).to.have.been.calledWith(sinon.match(/INFO: Runtime dependency 'aws-sdk' found in devDependencies/)), + // npm ls and npm install should have been called + expect(packagerMock.getProdDependencies).to.have.been.calledOnce, + expect(packagerMock.install).to.have.been.calledOnce, + expect(packagerMock.prune).to.have.been.calledOnce, + expect(packagerMock.runScripts).to.have.been.calledOnce, + ])); }); it('should succeed if devDependency is required at runtime but forcefully excluded', () => { @@ -904,13 +904,13 @@ describe('packExternalModules', () => { packagerMock.runScripts.returns(BbPromise.resolve()); module.compileStats = statsWithDevDependency; return expect(module.packExternalModules()).to.be.fulfilled - .then(() => BbPromise.all([ - // npm ls and npm install should have been called - expect(packagerMock.getProdDependencies).to.have.been.calledOnce, - expect(packagerMock.install).to.have.been.calledOnce, - expect(packagerMock.prune).to.have.been.calledOnce, - expect(packagerMock.runScripts).to.have.been.calledOnce, - ])); + .then(() => BbPromise.all([ + // npm ls and npm install should have been called + expect(packagerMock.getProdDependencies).to.have.been.calledOnce, + expect(packagerMock.install).to.have.been.calledOnce, + expect(packagerMock.prune).to.have.been.calledOnce, + expect(packagerMock.runScripts).to.have.been.calledOnce, + ])); }); it('should read package-lock if found', () => { @@ -951,20 +951,20 @@ describe('packExternalModules', () => { packagerMock.runScripts.returns(BbPromise.resolve()); module.compileStats = stats; return expect(module.packExternalModules()).to.be.fulfilled - .then(() => BbPromise.all([ - // The module package JSON and the composite one should have been stored - expect(writeFileSyncStub).to.have.been.calledThrice, - expect(writeFileSyncStub.firstCall.args[1]).to.equal(JSON.stringify(expectedCompositePackageJSON, null, 2)), - expect(writeFileSyncStub.secondCall.args[1]).to.equal(JSON.stringify({ info: 'lockfile' }, null, 2)), - expect(writeFileSyncStub.thirdCall.args[1]).to.equal(JSON.stringify(expectedPackageJSON, null, 2)), - // The modules and the lock file should have been copied - expect(fsExtraMock.copy).to.have.been.calledTwice, - // npm ls and npm prune should have been called - expect(packagerMock.getProdDependencies).to.have.been.calledOnce, - expect(packagerMock.install).to.have.been.calledOnce, - expect(packagerMock.prune).to.have.been.calledOnce, - expect(packagerMock.runScripts).to.have.been.calledOnce, - ])); + .then(() => BbPromise.all([ + // The module package JSON and the composite one should have been stored + expect(writeFileSyncStub).to.have.been.calledThrice, + expect(writeFileSyncStub.firstCall.args[1]).to.equal(JSON.stringify(expectedCompositePackageJSON, null, 2)), + expect(writeFileSyncStub.secondCall.args[1]).to.equal(JSON.stringify({ info: 'lockfile' }, null, 2)), + expect(writeFileSyncStub.thirdCall.args[1]).to.equal(JSON.stringify(expectedPackageJSON, null, 2)), + // The modules and the lock file should have been copied + expect(fsExtraMock.copy).to.have.been.calledTwice, + // npm ls and npm prune should have been called + expect(packagerMock.getProdDependencies).to.have.been.calledOnce, + expect(packagerMock.install).to.have.been.calledOnce, + expect(packagerMock.prune).to.have.been.calledOnce, + expect(packagerMock.runScripts).to.have.been.calledOnce, + ])); }); it('should continue if package-lock cannot be read', () => { @@ -1004,19 +1004,19 @@ describe('packExternalModules', () => { packagerMock.runScripts.returns(BbPromise.resolve()); module.compileStats = stats; return expect(module.packExternalModules()).to.be.fulfilled - .then(() => BbPromise.all([ - // The module package JSON and the composite one should have been stored - expect(writeFileSyncStub).to.have.been.calledTwice, - expect(writeFileSyncStub.firstCall.args[1]).to.equal(JSON.stringify(expectedCompositePackageJSON, null, 2)), - expect(writeFileSyncStub.secondCall.args[1]).to.equal(JSON.stringify(expectedPackageJSON, null, 2)), - // The modules should have been copied - expect(fsExtraMock.copy).to.have.been.calledOnce, - // npm ls and npm prune should have been called - expect(packagerMock.getProdDependencies).to.have.been.calledOnce, - expect(packagerMock.install).to.have.been.calledOnce, - expect(packagerMock.prune).to.have.been.calledOnce, - expect(packagerMock.runScripts).to.have.been.calledOnce, - ])); + .then(() => BbPromise.all([ + // The module package JSON and the composite one should have been stored + expect(writeFileSyncStub).to.have.been.calledTwice, + expect(writeFileSyncStub.firstCall.args[1]).to.equal(JSON.stringify(expectedCompositePackageJSON, null, 2)), + expect(writeFileSyncStub.secondCall.args[1]).to.equal(JSON.stringify(expectedPackageJSON, null, 2)), + // The modules should have been copied + expect(fsExtraMock.copy).to.have.been.calledOnce, + // npm ls and npm prune should have been called + expect(packagerMock.getProdDependencies).to.have.been.calledOnce, + expect(packagerMock.install).to.have.been.calledOnce, + expect(packagerMock.prune).to.have.been.calledOnce, + expect(packagerMock.runScripts).to.have.been.calledOnce, + ])); }); it('should skip module copy if demanded by packager', () => { @@ -1055,22 +1055,22 @@ describe('packExternalModules', () => { packagerMock.mustCopyModules = false; module.compileStats = stats; return expect(module.packExternalModules()).to.be.fulfilled - .then(() => BbPromise.all([ - // The module package JSON and the composite one should have been stored - expect(writeFileSyncStub).to.have.been.calledTwice, - expect(writeFileSyncStub.firstCall.args[1]).to.equal(JSON.stringify(expectedCompositePackageJSON, null, 2)), - expect(writeFileSyncStub.secondCall.args[1]).to.equal(JSON.stringify(expectedPackageJSON, null, 2)), - // The modules should not have been copied - expect(fsExtraMock.copy).to.not.have.been.called, - // npm ls and npm prune should have been called - expect(packagerMock.getProdDependencies).to.have.been.calledOnce, - expect(packagerMock.install).to.have.been.calledOnce, - expect(packagerMock.prune).to.have.been.calledOnce, - expect(packagerMock.runScripts).to.have.been.calledOnce, - ])) - .finally(() => { - packagerMock.mustCopyModules = true; - }); + .then(() => BbPromise.all([ + // The module package JSON and the composite one should have been stored + expect(writeFileSyncStub).to.have.been.calledTwice, + expect(writeFileSyncStub.firstCall.args[1]).to.equal(JSON.stringify(expectedCompositePackageJSON, null, 2)), + expect(writeFileSyncStub.secondCall.args[1]).to.equal(JSON.stringify(expectedPackageJSON, null, 2)), + // The modules should not have been copied + expect(fsExtraMock.copy).to.not.have.been.called, + // npm ls and npm prune should have been called + expect(packagerMock.getProdDependencies).to.have.been.calledOnce, + expect(packagerMock.install).to.have.been.calledOnce, + expect(packagerMock.prune).to.have.been.calledOnce, + expect(packagerMock.runScripts).to.have.been.calledOnce, + ])) + .finally(() => { + packagerMock.mustCopyModules = true; + }); }); describe('peer dependencies', () => { @@ -1171,19 +1171,19 @@ describe('packExternalModules', () => { packagerMock.runScripts.returns(BbPromise.resolve()); module.compileStats = peerDepStats; return expect(module.packExternalModules()).to.be.fulfilled - .then(() => BbPromise.all([ - // The module package JSON and the composite one should have been stored - expect(writeFileSyncStub).to.have.been.calledTwice, - expect(writeFileSyncStub.firstCall.args[1]).to.equal(JSON.stringify(expectedCompositePackageJSON, null, 2)), - expect(writeFileSyncStub.secondCall.args[1]).to.equal(JSON.stringify(expectedPackageJSON, null, 2)), - // The modules should have been copied - expect(fsExtraMock.copy).to.have.been.calledOnce, - // npm ls and npm prune should have been called - expect(packagerMock.getProdDependencies).to.have.been.calledOnce, - expect(packagerMock.install).to.have.been.calledOnce, - expect(packagerMock.prune).to.have.been.calledOnce, - expect(packagerMock.runScripts).to.have.been.calledOnce, - ])); + .then(() => BbPromise.all([ + // The module package JSON and the composite one should have been stored + expect(writeFileSyncStub).to.have.been.calledTwice, + expect(writeFileSyncStub.firstCall.args[1]).to.equal(JSON.stringify(expectedCompositePackageJSON, null, 2)), + expect(writeFileSyncStub.secondCall.args[1]).to.equal(JSON.stringify(expectedPackageJSON, null, 2)), + // The modules should have been copied + expect(fsExtraMock.copy).to.have.been.calledOnce, + // npm ls and npm prune should have been called + expect(packagerMock.getProdDependencies).to.have.been.calledOnce, + expect(packagerMock.install).to.have.been.calledOnce, + expect(packagerMock.prune).to.have.been.calledOnce, + expect(packagerMock.runScripts).to.have.been.calledOnce, + ])); }); }); }); From 1abeaf6a2b6483d5b8605f058d2b8934303ef435 Mon Sep 17 00:00:00 2001 From: Emilien Escalle Date: Mon, 22 Apr 2019 19:46:07 +0200 Subject: [PATCH 3/7] Fix lint issues --- lib/packExternalModules.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/packExternalModules.js b/lib/packExternalModules.js index aad7a8f994..48b47cc570 100644 --- a/lib/packExternalModules.js +++ b/lib/packExternalModules.js @@ -158,7 +158,7 @@ function getExternalModules(stats) { } const externals = new Set(); - for (let chunk of stats.compilation.chunks) { + for (const chunk of stats.compilation.chunks) { if (!chunk.modulesIterable) { continue; } @@ -171,8 +171,8 @@ function getExternalModules(stats) { external: getExternalModuleName(module) }); } - }; - }; + } + } return Array.from(externals); } From ad2949b30d504d84d68bf659292efcdbbc6428cb Mon Sep 17 00:00:00 2001 From: "emilien.escalle" Date: Tue, 23 Apr 2019 12:00:16 +0200 Subject: [PATCH 4/7] Add ne tests --- tests/packExternalModules.test.js | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/tests/packExternalModules.test.js b/tests/packExternalModules.test.js index 5bc9d612d2..177bd98732 100644 --- a/tests/packExternalModules.test.js +++ b/tests/packExternalModules.test.js @@ -30,6 +30,12 @@ class ChunkMock { } } +class ChunkMockNoModulesIterable { + constructor(modules) { + this._modules = modules; + } +} + const packagerMockFactory = { create(sandbox) { const packagerMock = { @@ -152,7 +158,10 @@ describe('packExternalModules', () => { { identifier: _.constant('external "bluebird"') }, - ]) + ]), + new ChunkMockNoModulesIterable([ + + ]), ], compiler: { outputPath: '/my/Service/Path/.webpack/service' @@ -323,6 +332,22 @@ describe('packExternalModules', () => { ])); }); + it('should do nothing if webpackIncludeModules is empty', () => { + module.configuration = new Configuration(); + module.compileStats = { + stats: { + compileStats: {} + } + }; + + 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', @@ -1188,3 +1213,4 @@ describe('packExternalModules', () => { }); }); }); + From ece918b33584bea9d29cc398ecee0ac9e5960f0e Mon Sep 17 00:00:00 2001 From: "emilien.escalle" Date: Tue, 23 Apr 2019 12:06:50 +0200 Subject: [PATCH 5/7] Add new tests --- tests/packExternalModules.test.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/packExternalModules.test.js b/tests/packExternalModules.test.js index 177bd98732..f533dc0361 100644 --- a/tests/packExternalModules.test.js +++ b/tests/packExternalModules.test.js @@ -160,7 +160,7 @@ describe('packExternalModules', () => { }, ]), new ChunkMockNoModulesIterable([ - + ]), ], compiler: { @@ -336,7 +336,9 @@ describe('packExternalModules', () => { module.configuration = new Configuration(); module.compileStats = { stats: { - compileStats: {} + compilation: { + chunks: [] + } } }; From e9fc1a823b4ca474923aa4af5d899e30640d5f06 Mon Sep 17 00:00:00 2001 From: "emilien.escalle" Date: Tue, 23 Apr 2019 12:19:46 +0200 Subject: [PATCH 6/7] Improve code coverage --- lib/packExternalModules.js | 34 +++++++++++++++---------------- tests/packExternalModules.test.js | 18 ---------------- 2 files changed, 17 insertions(+), 35 deletions(-) diff --git a/lib/packExternalModules.js b/lib/packExternalModules.js index 48b47cc570..be6bf330d1 100644 --- a/lib/packExternalModules.js +++ b/lib/packExternalModules.js @@ -153,28 +153,28 @@ function findExternalOrigin(issuer) { } function getExternalModules(stats) { - if (!stats.compilation.chunks) { - return []; - } - - const externals = new Set(); - for (const chunk of stats.compilation.chunks) { - if (!chunk.modulesIterable) { - continue; - } + if (stats.compilation.chunks) { + const externals = new Set(); + for (const chunk of stats.compilation.chunks) { + if (!chunk.modulesIterable) { + continue; + } - // Explore each module within the chunk (built inputs): - for (const module of chunk.modulesIterable) { - if (isExternalModule(module)) { - externals.add({ - origin: _.get(findExternalOrigin(module.issuer), 'rawRequest'), - external: getExternalModuleName(module) - }); + // Explore each module within the chunk (built inputs): + for (const module of chunk.modulesIterable) { + if (isExternalModule(module)) { + externals.add({ + origin: _.get(findExternalOrigin(module.issuer), 'rawRequest'), + external: getExternalModuleName(module) + }); + } } } + + return Array.from(externals); } - return Array.from(externals); + return []; } module.exports = { diff --git a/tests/packExternalModules.test.js b/tests/packExternalModules.test.js index f533dc0361..0ed594f80f 100644 --- a/tests/packExternalModules.test.js +++ b/tests/packExternalModules.test.js @@ -332,24 +332,6 @@ describe('packExternalModules', () => { ])); }); - it('should do nothing if webpackIncludeModules is empty', () => { - module.configuration = new Configuration(); - module.compileStats = { - stats: { - compilation: { - chunks: [] - } - } - }; - - 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', From c82753b42d0924ee2470d746f30fb8296326d67b Mon Sep 17 00:00:00 2001 From: "emilien.escalle" Date: Wed, 24 Apr 2019 00:15:08 +0200 Subject: [PATCH 7/7] Improve code of getExternalModules function --- lib/packExternalModules.js | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/lib/packExternalModules.js b/lib/packExternalModules.js index be6bf330d1..0c7ab2f98d 100644 --- a/lib/packExternalModules.js +++ b/lib/packExternalModules.js @@ -153,28 +153,26 @@ function findExternalOrigin(issuer) { } function getExternalModules(stats) { - if (stats.compilation.chunks) { - const externals = new Set(); - for (const chunk of stats.compilation.chunks) { - if (!chunk.modulesIterable) { - continue; - } + if (!stats.compilation.chunks) { + return []; + } + const externals = new Set(); + for (const chunk of stats.compilation.chunks) { + if (!chunk.modulesIterable) { + continue; + } - // Explore each module within the chunk (built inputs): - for (const module of chunk.modulesIterable) { - if (isExternalModule(module)) { - externals.add({ - origin: _.get(findExternalOrigin(module.issuer), 'rawRequest'), - external: getExternalModuleName(module) - }); - } + // Explore each module within the chunk (built inputs): + for (const module of chunk.modulesIterable) { + if (isExternalModule(module)) { + externals.add({ + origin: _.get(findExternalOrigin(module.issuer), 'rawRequest'), + external: getExternalModuleName(module) + }); } } - - return Array.from(externals); } - - return []; + return Array.from(externals); } module.exports = {