Repository: opencomponents/oc
Pull request #1181 (2021-02-26 17:45:02)

Handle scoped packages when removing version
Please provide enough information so that others can review your pull request:

Fix for https://github.com/opencomponents/oc/issues/1178 (Scoped packages being incorrectly handled when removing the version)

When stripping the version from a package string, we first separate the base name and directory. This is done so we can separate the scope (if present) from the package name, before using `split` to remove the package's version. Once removed, we return the full package with `path.join`

I've moved this code into a `split-version` utility, which is now called by `link-missing-dependencies` and `npm-utils`.

**Test plan (required)**

Unit tests have been added for `split-version.js` to cover the following cases:

* Passing a non-scoped package, both with and without a version
* Passing a scoped package, both with and without a version


Patch:
diff --git a/src/cli/domain/handle-dependencies/link-missing-dependencies.js b/src/cli/domain/handle-dependencies/link-missing-dependencies.js
index dc55949c..db0cb048 100644
--- a/src/cli/domain/handle-dependencies/link-missing-dependencies.js
+++ b/src/cli/domain/handle-dependencies/link-missing-dependencies.js
@@ -6,8 +6,7 @@ const path = require('path');
 const fs = require('fs-extra');
 const getMissingDependencies = require('./get-missing-dependencies');
 const strings = require('../../../resources/index');
-
-const getModuleName = dependency => dependency.split('@')[0];
+const stripVersion = require('../../../utils/strip-version');
 
 module.exports = (options, callback) => {
   const { componentPath, dependencies, logger } = options;
@@ -29,7 +28,7 @@ module.exports = (options, callback) => {
   const symLinkType = 'dir';
   let symLinkError = false;
   _.each(missingDependencies, dependency => {
-    const moduleName = getModuleName(dependency);
+    const moduleName = stripVersion(dependency);
     const pathToComponentModule = path.resolve(
       componentPath,
       'node_modules',
diff --git a/src/utils/npm-utils.js b/src/utils/npm-utils.js
index 0068fd16..13b94185 100644
--- a/src/utils/npm-utils.js
+++ b/src/utils/npm-utils.js
@@ -2,6 +2,7 @@
 
 const path = require('path');
 const spawn = require('cross-spawn');
+const stripVersion = require('./strip-version');
 
 const buildInstallCommand = options => {
   const args = ['install', '--prefix', options.installPath];
@@ -26,10 +27,8 @@ const executeCommand = (options, callback) => {
   cmd.on('close', code => callback(code !== 0 ? code : null));
 };
 
-const moduleName = dependency => dependency.split('@')[0];
-
 const getFullPath = ({ installPath, dependency }) =>
-  path.join(installPath, 'node_modules', moduleName(dependency));
+  path.join(installPath, 'node_modules', stripVersion(dependency));
 
 module.exports = {
   init: (options, callback) => {
diff --git a/src/utils/strip-version.js b/src/utils/strip-version.js
new file mode 100644
index 00000000..8b6b2d08
--- /dev/null
+++ b/src/utils/strip-version.js
@@ -0,0 +1,6 @@
+const path = require('path');
+
+module.exports = dependency => {
+  const parts = path.parse(dependency);
+  return path.join(parts.dir, parts.base.split('@')[0]);
+};


Test patch:
diff --git a/test/unit/utils-strip-version.js b/test/unit/utils-strip-version.js
new file mode 100644
index 00000000..48e02ead
--- /dev/null
+++ b/test/unit/utils-strip-version.js
@@ -0,0 +1,47 @@
+'use strict';
+
+const expect = require('chai').expect;
+
+describe('utils : stripVersion', () => {
+  const stripVersion = require('../../src/utils/strip-version');
+
+  describe('when a non scoped dependency is provided', () => {
+    const dependency = '/path/to/dependency';
+
+    describe('when a version is included', () => {
+      const name = stripVersion(dependency + '@1.0.0');
+
+      it('should return the dependency without the version', () => {
+        expect(name).to.equal('/path/to/dependency');
+      });
+    });
+
+    describe('when a version is not included', () => {
+      const name = stripVersion(dependency);
+
+      it('should return the unmodified dependency', () => {
+        expect(name).to.equal('/path/to/dependency');
+      });
+    });
+  });
+
+  describe('when a scoped dependency is provided', () => {
+    const dependency = '/path/to/@the-scoped/package';
+
+    describe('when a version is included', () => {
+      const name = stripVersion(dependency + '@1.2.3');
+
+      it('should return the dependency without the version', () => {
+        expect(name).to.equal('/path/to/@the-scoped/package');
+      });
+    });
+
+    describe('when a version is not included', () => {
+      const name = stripVersion(dependency);
+
+      it('should return the unmodified dependency', () => {
+        expect(name).to.equal('/path/to/@the-scoped/package');
+      });
+    });
+  });
+});