Merge pull request #281 from serverless-heaven/fix-npm-file-references

HyperBrain · web-flow · commit 2728d894ea18 · 2017-12-21T00:14:04.000+01:00
Fix/Support npm file references
diff --git a/README.md b/README.md
@@ -246,6 +246,13 @@ custom:
 If you specify a module in both arrays, `forceInclude` and `forceExclude`, the
 exclude wins and the module will not be packaged.
 
+#### Local modules
+
+You can use `file:` version references in your `package.json` to use a node module
+from a local folder (e.g. `"mymodule": "file:../../myOtherProject/mymodule"`).
+With that you can do test deployments from the local machine with different
+module versions or modules before they are published officially.
+
 #### Examples
 
 You can find an example setups in the [`examples`][link-examples] folder.
diff --git a/lib/packExternalModules.js b/lib/packExternalModules.js
@@ -7,18 +7,41 @@ const childProcess = require('child_process');
 const fse = require('fs-extra');
 const isBuiltinModule = require('is-builtin-module');
 
+function rebaseFileReferences(pathToPackageRoot, moduleVersion) {
+  if (/^file:[^/]{2}/.test(moduleVersion)) {
+    const filePath = _.replace(moduleVersion, /^file:/, '');
+    return _.replace(`file:${pathToPackageRoot}/${filePath}`, /\\/g, '/');
+  }
+
+  return moduleVersion;
+}
+
+function rebasePackageLock(pathToPackageRoot, module) {
+  if (module.version) {
+    module.version = rebaseFileReferences(pathToPackageRoot, module.version);
+  }
+
+  if (module.dependencies) {
+    _.forIn(module.dependencies, moduleDependency => {
+      rebasePackageLock(pathToPackageRoot, moduleDependency);
+    });
+  }
+}
+
 /**
  * Add the given modules to a package json's dependencies.
  */
-function addModulesToPackageJson(externalModules, packageJson) {
+function addModulesToPackageJson(externalModules, packageJson, pathToPackageRoot) {
   _.forEach(externalModules, externalModule => {
     const splitModule = _.split(externalModule, '@');
     // If we have a scoped module we have to re-add the @
     if (_.startsWith(externalModule, '@')) {
       splitModule.splice(0, 1);
       splitModule[0] = '@' + splitModule[0];
     }
-    const moduleVersion = _.join(_.tail(splitModule), '@');
+    let moduleVersion = _.join(_.tail(splitModule), '@');
+    // We have to rebase file references to the target package.json
+    moduleVersion = rebaseFileReferences(pathToPackageRoot, moduleVersion);
     packageJson.dependencies = packageJson.dependencies || {};
     packageJson.dependencies[_.first(splitModule)] = moduleVersion;
   });
@@ -247,7 +270,8 @@ module.exports = {
         description: `Packaged externals for ${this.serverless.service.service}`,
         private: true
       };
-      addModulesToPackageJson(compositeModules, compositePackage);
+      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
@@ -256,8 +280,21 @@ module.exports = {
       .then(exists => {
         if (exists) {
           this.serverless.cli.log('Package lock found - Using locked versions');
-          return BbPromise.fromCallback(cb => fse.copy(packageLockPath, path.join(compositeModulePath, 'package-lock.json'), cb))
-          .catch(err => this.serverless.cli.log(`Warning: Could not copy lock file: ${err.message}`));
+          try {
+            const packageLockJson = this.serverless.utils.readFileSync(packageLockPath);
+            /**
+             * We should not be modifying 'package-lock.json'
+             * because this file should be treat as internal to npm.
+             * 
+             * Rebase package-lock is a temporary workaround and must be
+             * removed as soon as https://github.com/npm/npm/issues/19183 gets fixed.
+             */
+            rebasePackageLock(relPath, packageLockJson);
+
+            this.serverless.utils.writeFileSync(path.join(compositeModulePath, 'package-lock.json'), JSON.stringify(packageLockJson, null, 2));
+          } catch(err) {
+            this.serverless.cli.log(`Warning: Could not read lock file: ${err.message}`);
+          }
         }
         return BbPromise.resolve();
       })
@@ -288,7 +325,8 @@ module.exports = {
             _.map(packageForceIncludes, whitelistedPackage => ({ external: whitelistedPackage }))
           ), packagePath, dependencyGraph);
         removeExcludedModules.call(this, prodModules, packageForceExcludes);
-        addModulesToPackageJson(prodModules, modulePackage);
+        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
diff --git a/tests/mocks/packageLocalRef.mock.json b/tests/mocks/packageLocalRef.mock.json
@@ -0,0 +1,33 @@
+{
+  "dependencies": {
+    "archiver": "^2.0.0",
+    "bluebird": "^3.4.0",
+    "fs-extra": "^0.26.7",
+    "glob": "^7.1.2",
+    "localmodule": "file:../../mymodule",
+    "lodash": "^4.17.4",
+    "npm-programmatic": "0.0.5",
+    "uuid": "^5.4.1",
+    "ts-node": "^3.2.0",
+    "@scoped/vendor": "1.0.0",
+    "pg": "^4.3.5"
+  },
+  "devDependencies": {
+    "babel-eslint": "^7.2.3",
+    "chai": "^4.1.0",
+    "chai-as-promised": "^7.1.1",
+    "eslint": "^4.3.0",
+    "eslint-plugin-import": "^2.7.0",
+    "eslint-plugin-lodash": "^2.4.4",
+    "eslint-plugin-promise": "^3.5.0",
+    "istanbul": "^0.4.5",
+    "mocha": "^3.4.2",
+    "mockery": "^2.1.0",
+    "serverless": "^1.17.0",
+    "sinon": "^2.3.8",
+    "sinon-chai": "^2.12.0"
+  },
+  "peerDependencies": {
+    "webpack": "*"
+  }
+}
diff --git a/tests/packExternalModules.test.js b/tests/packExternalModules.test.js
@@ -12,6 +12,7 @@ const Serverless = require('serverless');
 const childProcessMockFactory = require('./mocks/child_process.mock');
 const fsExtraMockFactory = require('./mocks/fs-extra.mock');
 const packageMock = require('./mocks/package.mock.json');
+const packageLocalRefMock = require('./mocks/packageLocalRef.mock.json');
 
 chai.use(require('chai-as-promised'));
 chai.use(require('sinon-chai'));
@@ -29,6 +30,7 @@ describe('packExternalModules', () => {
   let fsExtraMock;
   // Serverless stubs
   let writeFileSyncStub;
+  let readFileSyncStub;
 
   before(() => {
     sandbox = sinon.sandbox.create();
@@ -59,6 +61,7 @@ describe('packExternalModules', () => {
     _.set(serverless, 'service.service', 'test-service');
 
     writeFileSyncStub = sandbox.stub(serverless.utils, 'writeFileSync');
+    readFileSyncStub = sandbox.stub(serverless.utils, 'readFileSync');
     _.set(serverless, 'service.custom.webpackIncludeModules', true);
 
     module = _.assign({
@@ -72,6 +75,7 @@ describe('packExternalModules', () => {
   afterEach(() => {
     // Reset all counters and restore all stubbed functions
     writeFileSyncStub.reset();
+    readFileSyncStub.reset();
     childProcessMock.exec.reset();
     fsExtraMock.pathExists.reset();
     fsExtraMock.copy.reset();
@@ -151,6 +155,50 @@ describe('packExternalModules', () => {
         }
       ]
     };
+    const statsWithFileRef = {
+      stats: [
+        {
+          compilation: {
+            chunks: [
+              {
+                modules: [
+                  {
+                    identifier: _.constant('"crypto"')
+                  },
+                  {
+                    identifier: _.constant('"uuid/v4"')
+                  },
+                  {
+                    identifier: _.constant('external "eslint"')
+                  },
+                  {
+                    identifier: _.constant('"mockery"')
+                  },
+                  {
+                    identifier: _.constant('"@scoped/vendor/module1"')
+                  },
+                  {
+                    identifier: _.constant('external "@scoped/vendor/module2"')
+                  },
+                  {
+                    identifier: _.constant('external "uuid/v4"')
+                  },
+                  {
+                    identifier: _.constant('external "localmodule"')
+                  },
+                  {
+                    identifier: _.constant('external "bluebird"')
+                  },
+                ]
+              }
+            ],
+            compiler: {
+              outputPath: '/my/Service/Path/.webpack/service'
+            }
+          }
+        }
+      ]
+    };
 
     it('should do nothing if webpackIncludeModules is not set', () => {
       _.unset(serverless, 'service.custom.webpackIncludeModules');
@@ -212,6 +260,102 @@ describe('packExternalModules', () => {
       ]));
     });
 
+    it('should rebase file references', () => {
+      const expectedLocalModule = 'file:../../locals/../../mymodule';
+      const expectedCompositePackageJSON = {
+        name: 'test-service',
+        version: '1.0.0',
+        description: 'Packaged externals for test-service',
+        private: true,
+        dependencies: {
+          '@scoped/vendor': '1.0.0',
+          uuid: '^5.4.1',
+          localmodule: 'file:../../locals/../../mymodule',
+          bluebird: '^3.4.0'
+        }
+      };
+      const expectedPackageJSON = {
+        dependencies: {
+          '@scoped/vendor': '1.0.0',
+          uuid: '^5.4.1',
+          localmodule: expectedLocalModule,
+          bluebird: '^3.4.0'
+        }
+      };
+
+      const fakePackageLockJSON = {
+        name: 'test-service',
+        version: '1.0.0',
+        description: 'Packaged externals for test-service',
+        private: true,
+        dependencies: {
+          '@scoped/vendor': '1.0.0',
+          uuid: {
+            version: '^5.4.1'
+          },
+          bluebird: {
+            version: '^3.4.0'
+          },
+          localmodule: {
+            version: 'file:../../mymodule'
+          }
+        }
+      };
+      const expectedPackageLockJSON = {
+        name: 'test-service',
+        version: '1.0.0',
+        description: 'Packaged externals for test-service',
+        private: true,
+        dependencies: {
+          '@scoped/vendor': '1.0.0',
+          uuid: {
+            version: '^5.4.1'
+          },
+          bluebird: {
+            version: '^3.4.0'
+          },
+          localmodule: {
+            version: expectedLocalModule
+          }
+        }
+      };
+
+      _.set(serverless, 'service.custom.webpackIncludeModules.packagePath', path.join('locals', 'package.json'));
+      module.webpackOutputPath = 'outputPath';
+      readFileSyncStub.returns(fakePackageLockJSON);
+      fsExtraMock.pathExists.yields(null, true);
+      fsExtraMock.copy.yields();
+      childProcessMock.exec.onFirstCall().yields(null, '{}', '');
+      childProcessMock.exec.onSecondCall().yields(null, '', '');
+      childProcessMock.exec.onThirdCall().yields();
+      module.compileStats = statsWithFileRef;
+
+      sandbox.stub(process, 'cwd').returns(path.join('/my/Service/Path'));
+      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.secondCall.args[1]).to.equal(JSON.stringify(expectedPackageLockJSON, null, 2)),
+        expect(writeFileSyncStub.thirdCall.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(childProcessMock.exec).to.have.been.calledThrice,
+        expect(childProcessMock.exec.firstCall).to.have.been.calledWith(
+          'npm ls -prod -json -depth=1'
+        ),
+        expect(childProcessMock.exec.secondCall).to.have.been.calledWith(
+          'npm install'
+        ),
+        expect(childProcessMock.exec.thirdCall).to.have.been.calledWith(
+          'npm prune'
+        )
+      ]));
+    });
+
     it('should skip module copy for Google provider', () => {
       const expectedCompositePackageJSON = {
         name: 'test-service',
@@ -601,10 +745,7 @@ describe('packExternalModules', () => {
         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.calledTwice,
-        expect(fsExtraMock.copy.firstCall).to.have.been.calledWith(
-          sinon.match(/package-lock.json$/)
-        ),
+        expect(fsExtraMock.copy).to.have.been.calledOnce,
         // npm ls and npm prune should have been called
         expect(childProcessMock.exec).to.have.been.calledThrice,
         expect(childProcessMock.exec.firstCall).to.have.been.calledWith(
@@ -640,9 +781,9 @@ describe('packExternalModules', () => {
       };
 
       module.webpackOutputPath = 'outputPath';
+      readFileSyncStub.throws(new Error('Failed to read package-lock.json'));
       fsExtraMock.pathExists.yields(null, true);
-      fsExtraMock.copy.onFirstCall().yields(new Error('Failed to read package-lock.json'));
-      fsExtraMock.copy.onSecondCall().yields();
+      fsExtraMock.copy.onFirstCall().yields();
       childProcessMock.exec.onFirstCall().yields(null, '{}', '');
       childProcessMock.exec.onSecondCall().yields(null, '', '');
       childProcessMock.exec.onThirdCall().yields();
@@ -654,10 +795,7 @@ describe('packExternalModules', () => {
         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.calledTwice,
-        expect(fsExtraMock.copy.firstCall).to.have.been.calledWith(
-          sinon.match(/package-lock.json$/)
-        ),
+        expect(fsExtraMock.copy).to.have.been.calledOnce,
         // npm ls and npm prune should have been called
         expect(childProcessMock.exec).to.have.been.calledThrice,
         expect(childProcessMock.exec.firstCall).to.have.been.calledWith(
