yarn-plugin: move Backstage version to parameters in bound descriptors

The previous version of this code transformed
the version range "backstage:^" to
"backstage:<version-from-backstage.json>" when
writing entries in yarn.lock. This does seem to be
something that the yarn plugin API supports, but
it turns out to cause issues in some third-party
tools which assume that version ranges will be
consistent between package.json and yarn.lock.

To fix this issue, we're now leveraging the
descriptor binding feature in Backstage (which I
didn't know about before) to append a descriptor
parameter containing the Backstage version. This
keeps the selector of the descriptors consistent
between package.json and yarn.lock entries, which
will hopefully mean that this plugin no longer
breaks other tools.

cf. https://github.com/snyk/nodejs-lockfile-parser/issues/256

Signed-off-by: MT Lewis <mtlewis@users.noreply.github.com>

packages/yarn-plugin/src/handlers/beforeWorkspacePacking.ts

@@ -16,7 +16,11 @@
 
 import { Descriptor, Workspace, structUtils } from '@yarnpkg/core';
 import { some } from 'lodash';
-import { getCurrentBackstageVersion, getPackageVersion } from '../util';
+import {
+  bindBackstageVersion,
+  getCurrentBackstageVersion,
+  getPackageVersion,
+} from '../util';
 import { PROTOCOL } from '../constants';
 
 const hasBackstageVersion = (range: string) =>
@@ -66,10 +70,7 @@ export const beforeWorkspacePacking = async (
       );
 
       rawManifest[finalDependencyType][ident] = `^${await getPackageVersion(
-        structUtils.makeDescriptor(
-          descriptor,
-          `${PROTOCOL}${backstageVersion}`,
-        ),
+        bindBackstageVersion(descriptor, backstageVersion),
       )}`;
     }
   }

packages/yarn-plugin/src/resolver/BackstageResolver.test.ts

@@ -96,7 +96,7 @@ describe('BackstageResolver', () => {
 
   describe('bindDescriptor', () => {
     describe('with range "backstage:^"', () => {
-      it('returns a descriptor basedwith a version range for the current Backstage version', () => {
+      it('returns a descriptor with a version range for the current Backstage version', () => {
         expect(
           backstageResolver.bindDescriptor(
             structUtils.makeDescriptor(
@@ -107,25 +107,25 @@ describe('BackstageResolver', () => {
         ).toEqual(
           structUtils.makeDescriptor(
             structUtils.makeIdent('backstage', 'core'),
-            'backstage:1.23.45',
+            'backstage:^::v=1.23.45',
           ),
         );
       });
     });
 
-    describe('with range "backstage:1.23.45"', () => {
+    describe('with range "backstage:^::v=1.23.45"', () => {
       it('returns the correct descriptor', () => {
         expect(
           backstageResolver.bindDescriptor(
             structUtils.makeDescriptor(
               structUtils.makeIdent('backstage', 'core'),
-              'backstage:1.23.45',
+              'backstage:^::v=1.23.45',
             ),
           ),
         ).toEqual(
           structUtils.makeDescriptor(
             structUtils.makeIdent('backstage', 'core'),
-            'backstage:1.23.45',
+            'backstage:^::v=1.23.45',
           ),
         );
       });
@@ -136,7 +136,7 @@ describe('BackstageResolver', () => {
     it('returns an npm: descriptor based on the manifest for the appropriate backstage version', async () => {
       const descriptor = structUtils.makeDescriptor(
         structUtils.makeIdent('backstage', 'core'),
-        'backstage:1.23.45',
+        'backstage:^::v=1.23.45',
       );
 
       await expect(
@@ -155,7 +155,7 @@ describe('BackstageResolver', () => {
       ).rejects.toThrow(/unsupported version protocol/i);
     });
 
-    it('rejects backstage: ranges with a ^ shorthand version', async () => {
+    it('rejects backstage: ranges missing a version parameter', async () => {
       await expect(
         backstageResolver.getCandidates(
           structUtils.makeDescriptor(
@@ -163,35 +163,42 @@ describe('BackstageResolver', () => {
             'backstage:^',
           ),
         ),
-      ).rejects.toThrow(/invalid backstage version/i);
+      ).rejects.toThrow(/missing Backstage version/i);
     });
 
-    it('rejects backstage: ranges with a * shorthand version', async () => {
+    it('rejects backstage: ranges with multiple version parameters', async () => {
       await expect(
         backstageResolver.getCandidates(
           structUtils.makeDescriptor(
             structUtils.makeIdent('backstage', 'core'),
-            'backstage:*',
+            'backstage:^::v=1&v=2',
           ),
         ),
-      ).rejects.toThrow(/invalid backstage version/i);
+      ).rejects.toThrow(/multiple Backstage versions/i);
     });
 
-    it('rejects backstage: ranges with an invalid version specified', async () => {
-      await expect(
-        backstageResolver.getCandidates(
-          structUtils.makeDescriptor(
-            structUtils.makeIdent('backstage', 'core'),
-            'backstage:latest',
+    it.each`
+      selector
+      ${'*'}
+      ${'latest'}
+    `(
+      'rejects backstage: ranges with invalid selector "$selector"',
+      async ({ selector }) => {
+        await expect(
+          backstageResolver.getCandidates(
+            structUtils.makeDescriptor(
+              structUtils.makeIdent('backstage', 'core'),
+              `backstage:${selector}`,
+            ),
           ),
-        ),
-      ).rejects.toThrow(/invalid backstage version/i);
-    });
+        ).rejects.toThrow(/unexpected version selector/i);
+      },
+    );
 
     it('memoizes manifest retrieval', async () => {
       const descriptor1 = structUtils.makeDescriptor(
         structUtils.makeIdent('backstage', 'core'),
-        'backstage:1.23.45',
+        'backstage:^::v=1.23.45',
       );
 
       for (let i = 0; i < 5; i++) {
@@ -202,7 +209,7 @@ describe('BackstageResolver', () => {
 
       const descriptor2 = structUtils.makeDescriptor(
         structUtils.makeIdent('backstage', 'core'),
-        'backstage:6.78.90',
+        'backstage:^::v=6.78.90',
       );
 
       for (let i = 0; i < 5; i++) {
@@ -219,7 +226,7 @@ describe('BackstageResolver', () => {
         backstageResolver.getSatisfying(
           structUtils.makeDescriptor(
             structUtils.makeIdent('backstage', 'core'),
-            'backstage:1.23.45',
+            'backstage:^::v=1.23.45',
           ),
           {},
           [
@@ -253,7 +260,7 @@ describe('BackstageResolver', () => {
         backstageResolver.getSatisfying(
           structUtils.makeDescriptor(
             structUtils.makeIdent('backstage', 'core'),
-            'backstage:1.23.45',
+            'backstage:^::v=1.23.45',
           ),
           {},
           [
@@ -292,7 +299,7 @@ describe('BackstageResolver', () => {
           {},
           [],
         ),
-      ).rejects.toThrow(/unexpected npm: range/i);
+      ).rejects.toThrow(/unsupported version protocol/i);
     });
   });
 });

packages/yarn-plugin/src/resolver/BackstageResolver.ts

@@ -21,9 +21,12 @@ import {
   Package,
   Resolver,
 } from '@yarnpkg/core';
-import semver from 'semver';
 import { PROTOCOL } from '../constants';
-import { getCurrentBackstageVersion, getPackageVersion } from '../util';
+import {
+  bindBackstageVersion,
+  getCurrentBackstageVersion,
+  getPackageVersion,
+} from '../util';
 
 export class BackstageResolver implements Resolver {
   static protocol = PROTOCOL;
@@ -45,10 +48,7 @@ export class BackstageResolver implements Resolver {
    */
   bindDescriptor(descriptor: Descriptor): Descriptor {
     if (descriptor.range === 'backstage:^') {
-      return structUtils.makeDescriptor(
-        descriptor,
-        `${PROTOCOL}${getCurrentBackstageVersion()}`,
-      );
+      return bindBackstageVersion(descriptor, getCurrentBackstageVersion());
     }
 
     return descriptor;
@@ -61,23 +61,6 @@ export class BackstageResolver implements Resolver {
    * backstage release.
    */
   async getCandidates(descriptor: Descriptor): Promise<Locator[]> {
-    const range = structUtils.parseRange(descriptor.range);
-    if (range.protocol !== BackstageResolver.protocol) {
-      throw new Error(
-        `Unsupported version protocol in version range "${
-          descriptor.range
-        }" for package ${structUtils.stringifyIdent(descriptor)}`,
-      );
-    }
-
-    if (!semver.valid(range.selector)) {
-      throw new Error(
-        `Invalid Backstage version string when resolving version for ${structUtils.stringifyIdent(
-          descriptor,
-        )}`,
-      );
-    }
-
     return [
       structUtils.makeLocator(
         descriptor,

packages/yarn-plugin/src/util.ts

@@ -43,20 +43,43 @@ export const getCurrentBackstageVersion = () => {
   return backstageVersion;
 };
 
+export const bindBackstageVersion = (
+  descriptor: Descriptor,
+  backstageVersion: string,
+) => {
+  return structUtils.bindDescriptor(descriptor, { v: backstageVersion });
+};
+
 export const getPackageVersion = async (descriptor: Descriptor) => {
   const ident = structUtils.stringifyIdent(descriptor);
   const range = structUtils.parseRange(descriptor.range);
 
   if (range.protocol !== PROTOCOL) {
-    throw new Error(`Unexpected ${range.protocol} range when packing`);
+    throw new Error(
+      `Unsupported version protocol in version range "${descriptor.range}" for package ${ident}`,
+    );
   }
 
-  if (!semverValid(range.selector)) {
-    throw new Error(`Missing backstage version in range ${descriptor.range}`);
+  if (range.selector !== '^') {
+    throw new Error(
+      `Unexpected version selector "${range.selector}" for package ${ident}`,
+    );
+  }
+
+  if (!range.params?.v) {
+    throw new Error(
+      `Missing Backstage version parameter in range "${descriptor.range}" for package ${ident}`,
+    );
+  }
+
+  if (Array.isArray(range.params.v)) {
+    throw new Error(
+      `Multiple Backstage versions specified in range "${descriptor.range}" for package ${ident}`,
+    );
   }
 
   const manifest = await getManifestByVersion({
-    version: range.selector,
+    version: range.params.v,
   });
 
   const manifestEntry = manifest.packages.find(