Fix issue with PNPM when reading shrinkwrap version for scoped peer dependencies (microsoft#488)

Nicholas Pape · web-flow · commit 11738d4d9d9c · 2018-01-22T12:56:38.000-08:00
* Re-write some logic to make it more straightforward
* Ensure that we properly handle cases where we have a scoped peer dependency
diff --git a/apps/rush-lib/src/cli/logic/pnpm/PnpmShrinkwrapFile.ts b/apps/rush-lib/src/cli/logic/pnpm/PnpmShrinkwrapFile.ts
@@ -59,6 +59,37 @@ interface IShrinkwrapYaml {
   specifiers: { [dependency: string]: string };
 }
 
+export function extractVersionFromPnpmVersionSpecifier(version: string): string | undefined {
+  let extractedVersion: string | undefined = undefined;
+
+  if (!version) {
+    return undefined;
+  }
+
+  const versionParts: string[] = version.split('/');
+
+  // it had no slashes, so we know it is a version like "0.0.5"
+  if (versionParts.length === 1) {
+    extractedVersion = version; // e.g. "0.0.5"
+  } else {
+    const isScoped: boolean = versionParts[1].indexOf('@') === 0;
+
+    // e.g. "/gulp-karma/0.0.5/karma@0.13.22"
+    // if it has 4 parts, then it should be unscoped
+    if (versionParts.length === 4 && !isScoped) {
+      extractedVersion = versionParts[2]; // e.g. "0.0.5"
+    }
+
+    // e.g. "/@ms/sp-client-utilities/3.1.1/foo@13.1.0"
+    // if it has 5 parts, it should be scoped
+    if (versionParts.length === 5 && isScoped) {
+      extractedVersion = versionParts[3]; // e.g. "3.1.1"
+    }
+  }
+
+  return extractedVersion;
+}
+
 export class PnpmShrinkwrapFile extends BaseShrinkwrapFile {
   private _shrinkwrapJson: IShrinkwrapYaml;
 
@@ -123,19 +154,23 @@ export class PnpmShrinkwrapFile extends BaseShrinkwrapFile {
     // version will be either:
     // A - the version (e.g. "0.0.5")
     // B - a peer dep version (e.g. "/gulp-karma/0.0.5/karma@0.13.22"
+    //                           or "/@ms/sp-client-utilities/3.1.1/foo@13.1.0"
     //                           or "/sinon-chai/2.8.0/chai@3.5.0+sinon@1.17.7")
 
     // check to see if this is the special style of specifiers
-    // e.g.:  "/gulp-karma/0.0.5/karma@0.13.22"
-    // split it by forward slashes, then grab the second group
+    // e.g.:  "/gulp-karma/0.0.5/karma@0.13.22" or
+    //     or "/@ms/sp-client-utilities/3.1.1/foo@13.1.0"
+    // split it by forward slashes, then grab the second group (or the 3rd, if the package name is scoped)
     // if the second group doesn't exist, return the version directly
     if (version) {
-      const versionParts: string[] = version.split('/');
-      if (versionParts.length !== 1 && versionParts.length !== 4) {
+      const extractedVersion: string | undefined = extractVersionFromPnpmVersionSpecifier(version);
+
+      if (!extractedVersion) {
         throw new Error(`Cannot parse pnpm shrinkwrap version specifier: `
           + `"${version}" for "${dependencyName}"`);
       }
-      return versionParts[2] || version;
+
+      return extractedVersion;
     } else {
       return undefined;
     }
diff --git a/apps/rush-lib/src/cli/logic/test/ShrinkwrapFile.test.ts b/apps/rush-lib/src/cli/logic/test/ShrinkwrapFile.test.ts
@@ -6,6 +6,7 @@ import * as path from 'path';
 
 import { BaseShrinkwrapFile } from '../base/BaseShrinkwrapFile';
 import { ShrinkwrapFileFactory } from '../ShrinkwrapFileFactory';
+import { extractVersionFromPnpmVersionSpecifier } from '../pnpm/PnpmShrinkwrapFile';
 
 describe('npm ShrinkwrapFile', () => {
   const filename: string = path.resolve(path.join(__dirname, './shrinkwrapFile/npm-shrinkwrap.json'));
@@ -30,9 +31,9 @@ describe('npm ShrinkwrapFile', () => {
 });
 
 describe('pnpm ShrinkwrapFile', () => {
-  const filename: string = path.resolve(path.join(
-    __dirname, '../../../../src/cli/logic/test/shrinkwrapFile/shrinkwrap.yaml'));
-  const shrinkwrapFile: BaseShrinkwrapFile = ShrinkwrapFileFactory.getShrinkwrapFile('pnpm', filename)!;
+const filename: string = path.resolve(path.join(
+  __dirname, '../../../../src/cli/logic/test/shrinkwrapFile/shrinkwrap.yaml'));
+const shrinkwrapFile: BaseShrinkwrapFile = ShrinkwrapFileFactory.getShrinkwrapFile('pnpm', filename)!;
 
   it('verifies root-level dependency', () => {
     assert.isTrue(shrinkwrapFile.hasCompatibleTopLevelDependency('jquery', '>=2.0.0 <3.0.0'));
@@ -51,3 +52,23 @@ describe('pnpm ShrinkwrapFile', () => {
     assert.deepEqual(tempProjectNames, ['@rush-temp/project1', '@rush-temp/project2']);
   });
 });
+
+describe('extractVersionFromPnpmVersionSpecifier', () => {
+  it('extracts a simple version with no slashes', () => {
+    assert.equal(extractVersionFromPnpmVersionSpecifier('0.0.5'), '0.0.5');
+  });
+  it('extracts an unscoped peer dep', () => {
+    assert.equal(extractVersionFromPnpmVersionSpecifier('/gulp-karma/0.0.5/karma@0.13.22'), '0.0.5');
+  });
+  it('extracts a scoped peer dep', () => {
+    assert.equal(extractVersionFromPnpmVersionSpecifier('/@ms/sp-client-utilities/3.1.1/foo@13.1.0'), '3.1.1');
+  });
+  it('handles bad cases', () => {
+    assert.equal(extractVersionFromPnpmVersionSpecifier('/foo/gulp-karma/0.0.5/karma@0.13.22'), undefined);
+    assert.equal(extractVersionFromPnpmVersionSpecifier('/@ms/3.1.1/foo@13.1.0'), undefined);
+    assert.equal(extractVersionFromPnpmVersionSpecifier(''), undefined);
+    assert.equal(extractVersionFromPnpmVersionSpecifier('/'), undefined);
+    assert.equal(extractVersionFromPnpmVersionSpecifier('//'), undefined);
+    assert.equal(extractVersionFromPnpmVersionSpecifier('/@/'), undefined);
+  });
+});
diff --git a/common/changes/@microsoft/rush/nickpape-fix-more-shrinkwrap-issue_2018-01-22-20-40.json b/common/changes/@microsoft/rush/nickpape-fix-more-shrinkwrap-issue_2018-01-22-20-40.json
@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "comment": "Fix an issue when parsing scoped peer dependencies in the pnpm shrinkwrap file",
+      "packageName": "@microsoft/rush",
+      "type": "none"
+    }
+  ],
+  "packageName": "@microsoft/rush",
+  "email": "nickpape-msft@users.noreply.github.com"
+}
