From 901748188a8997a7ed4c0d6f60b8fbaceb29fe94 Mon Sep 17 00:00:00 2001 From: MT Lewis Date: Tue, 19 Nov 2024 20:19:13 +0000 Subject: [PATCH] yarn-plugin: move Backstage version to parameters in bound descriptors The previous version of this code transformed the version range "backstage:^" to "backstage:" 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 --- .../src/handlers/beforeWorkspacePacking.ts | 11 ++-- .../src/resolver/BackstageResolver.test.ts | 57 +++++++++++-------- .../src/resolver/BackstageResolver.ts | 29 ++-------- packages/yarn-plugin/src/util.ts | 31 ++++++++-- 4 files changed, 71 insertions(+), 57 deletions(-) diff --git a/packages/yarn-plugin/src/handlers/beforeWorkspacePacking.ts b/packages/yarn-plugin/src/handlers/beforeWorkspacePacking.ts index 41014a55a6..3221400f48 100644 --- a/packages/yarn-plugin/src/handlers/beforeWorkspacePacking.ts +++ b/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), )}`; } } diff --git a/packages/yarn-plugin/src/resolver/BackstageResolver.test.ts b/packages/yarn-plugin/src/resolver/BackstageResolver.test.ts index d7d9126923..f6369b6cdf 100644 --- a/packages/yarn-plugin/src/resolver/BackstageResolver.test.ts +++ b/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); }); }); }); diff --git a/packages/yarn-plugin/src/resolver/BackstageResolver.ts b/packages/yarn-plugin/src/resolver/BackstageResolver.ts index 7e794f5693..0e575cc687 100644 --- a/packages/yarn-plugin/src/resolver/BackstageResolver.ts +++ b/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 { - 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, diff --git a/packages/yarn-plugin/src/util.ts b/packages/yarn-plugin/src/util.ts index 0b613a477c..df7ed75e0f 100644 --- a/packages/yarn-plugin/src/util.ts +++ b/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(