diff --git a/.changeset/bright-pants-cry.md b/.changeset/bright-pants-cry.md new file mode 100644 index 0000000000..88d7356640 --- /dev/null +++ b/.changeset/bright-pants-cry.md @@ -0,0 +1,5 @@ +--- +'@backstage/cli': patch +--- + +Fixed an issue where the CLI would fail to function when there was a mix of workspace and non-workspace versions of the same package in `yarn.lock` when using Yarn 3. diff --git a/.changeset/forty-years-retire.md b/.changeset/forty-years-retire.md new file mode 100644 index 0000000000..e7b887a1e4 --- /dev/null +++ b/.changeset/forty-years-retire.md @@ -0,0 +1,5 @@ +--- +'@backstage/cli': patch +--- + +The frontend serve task now filters out allowed package duplicates during its package check, just like `versions:bump` and `versions:check`. diff --git a/.changeset/witty-eels-itch.md b/.changeset/witty-eels-itch.md new file mode 100644 index 0000000000..5a2da8b570 --- /dev/null +++ b/.changeset/witty-eels-itch.md @@ -0,0 +1,5 @@ +--- +'@backstage/cli': patch +--- + +Workspace ranges are no longer considered invalid by version commands. diff --git a/packages/cli/src/commands/start/startFrontend.ts b/packages/cli/src/commands/start/startFrontend.ts index f6923632c8..b7b81e6bff 100644 --- a/packages/cli/src/commands/start/startFrontend.ts +++ b/packages/cli/src/commands/start/startFrontend.ts @@ -21,7 +21,8 @@ import { serveBundle } from '../../lib/bundler'; import { loadCliConfig } from '../../lib/config'; import { paths } from '../../lib/paths'; import { Lockfile } from '../../lib/versioning'; -import { includedFilter } from '../versions/lint'; +import { forbiddenDuplicatesFilter, includedFilter } from '../versions/lint'; +import { PackageGraph } from '../../lib/monorepo'; interface StartAppOptions { verifyVersions?: boolean; @@ -36,10 +37,13 @@ export async function startFrontend(options: StartAppOptions) { const lockfile = await Lockfile.load(paths.resolveTargetRoot('yarn.lock')); const result = lockfile.analyze({ filter: includedFilter, + localPackages: PackageGraph.fromPackages( + await PackageGraph.listTargetPackages(), + ), }); - const problemPackages = [...result.newVersions, ...result.newRanges].map( - ({ name }) => name, - ); + const problemPackages = [...result.newVersions, ...result.newRanges] + .map(({ name }) => name) + .filter(forbiddenDuplicatesFilter); if (problemPackages.length > 1) { console.log( diff --git a/packages/cli/src/commands/versions/bump.ts b/packages/cli/src/commands/versions/bump.ts index 1905aded20..37eb858dcf 100644 --- a/packages/cli/src/commands/versions/bump.ts +++ b/packages/cli/src/commands/versions/bump.ts @@ -39,6 +39,7 @@ import { ReleaseManifest, } from '@backstage/release-manifests'; import 'global-agent/bootstrap'; +import { PackageGraph } from '../../lib/monorepo'; const DEP_TYPES = [ 'dependencies', @@ -305,8 +306,12 @@ export default async (opts: OptionValues) => { // Finally we make sure the new lockfile doesn't have any duplicates const dedupLockfile = await Lockfile.load(lockfilePath); + const result = dedupLockfile.analyze({ filter, + localPackages: PackageGraph.fromPackages( + await PackageGraph.listTargetPackages(), + ), }); if (result.newVersions.length > 0) { diff --git a/packages/cli/src/commands/versions/lint.ts b/packages/cli/src/commands/versions/lint.ts index 9939b2ba51..e6bc119b71 100644 --- a/packages/cli/src/commands/versions/lint.ts +++ b/packages/cli/src/commands/versions/lint.ts @@ -18,6 +18,7 @@ import { OptionValues } from 'commander'; import { Lockfile } from '../../lib/versioning'; import { paths } from '../../lib/paths'; import partition from 'lodash/partition'; +import { PackageGraph } from '../../lib/monorepo'; // Packages that we try to avoid duplicates for const INCLUDED = [/^@backstage\//]; @@ -55,6 +56,9 @@ export default async (cmd: OptionValues) => { const lockfile = await Lockfile.load(paths.resolveTargetRoot('yarn.lock')); const result = lockfile.analyze({ filter: includedFilter, + localPackages: PackageGraph.fromPackages( + await PackageGraph.listTargetPackages(), + ), }); logArray( diff --git a/packages/cli/src/lib/versioning/Lockfile.test.ts b/packages/cli/src/lib/versioning/Lockfile.test.ts index 4621b43372..e663ec4a93 100644 --- a/packages/cli/src/lib/versioning/Lockfile.test.ts +++ b/packages/cli/src/lib/versioning/Lockfile.test.ts @@ -16,6 +16,7 @@ import fs from 'fs-extra'; import mockFs from 'mock-fs'; +import { ExtendedPackage } from '../monorepo'; import { Lockfile } from './Lockfile'; const HEADER = `# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. @@ -91,7 +92,7 @@ describe('Lockfile', () => { }); const lockfile = await Lockfile.load('/yarn.lock'); - const result = lockfile.analyze(); + const result = lockfile.analyze({ localPackages: new Map() }); expect(result).toEqual({ invalidRanges: [], newRanges: [], @@ -120,7 +121,7 @@ describe('Lockfile', () => { }); const lockfile = await Lockfile.load('/yarn.lock'); - const result = lockfile.analyze(); + const result = lockfile.analyze({ localPackages: new Map() }); expect(result).toEqual({ invalidRanges: [], newRanges: [ @@ -192,6 +193,36 @@ b@^2: version: 2.0.1 `; +const mockANewLocal = `${newHeader} +a@^1: + version: 1.0.1 + dependencies: + b: ^2 + integrity: sha512-xyz + resolved: "https://my-registry/a-1.0.01.tgz#abc123" + +"b@2.0.x, b@^2.0.1": + version: 0.0.0-use.local + +b@^2: + version: 2.0.0 +`; + +const mockANewLocalDedup = `${newHeader} +a@^1: + version: 1.0.1 + dependencies: + b: ^2 + integrity: sha512-xyz + resolved: "https://my-registry/a-1.0.01.tgz#abc123" + +"b@2.0.x, b@^2.0.1": + version: 0.0.0-use.local + +b@^2: + version: 0.0.0-use.local +`; + describe('New Lockfile', () => { afterEach(() => { mockFs.restore(); @@ -218,7 +249,7 @@ describe('New Lockfile', () => { }); const lockfile = await Lockfile.load('/yarn.lock'); - const result = lockfile.analyze(); + const result = lockfile.analyze({ localPackages: new Map() }); expect(result).toEqual({ invalidRanges: [], newRanges: [], @@ -242,4 +273,46 @@ describe('New Lockfile', () => { mockANewDedup, ); }); + + it('should deduplicate and save mockANewLocal', async () => { + mockFs({ + '/yarn.lock': mockANewLocal, + }); + + const lockfile = await Lockfile.load('/yarn.lock'); + const result = lockfile.analyze({ + localPackages: new Map([ + [ + 'b', + { + packageJson: { version: '2.0.1' }, + } as ExtendedPackage, + ], + ]), + }); + expect(result).toEqual({ + invalidRanges: [], + newRanges: [], + newVersions: [ + { + name: 'b', + range: '^2', + oldVersion: '2.0.0', + newVersion: '0.0.0-use.local', + }, + ], + }); + + expect(lockfile.toString()).toBe(mockANewLocal); + lockfile.replaceVersions(result.newVersions); + expect(lockfile.toString()).toBe(mockANewLocalDedup); + + await expect(fs.readFile('/yarn.lock', 'utf8')).resolves.toBe( + mockANewLocal, + ); + await expect(lockfile.save()).resolves.toBeUndefined(); + await expect(fs.readFile('/yarn.lock', 'utf8')).resolves.toBe( + mockANewLocalDedup, + ); + }); }); diff --git a/packages/cli/src/lib/versioning/Lockfile.ts b/packages/cli/src/lib/versioning/Lockfile.ts index 016593ea8d..0559ca86a1 100644 --- a/packages/cli/src/lib/versioning/Lockfile.ts +++ b/packages/cli/src/lib/versioning/Lockfile.ts @@ -18,6 +18,7 @@ import fs from 'fs-extra'; import semver from 'semver'; import { parseSyml, stringifySyml } from '@yarnpkg/parsers'; import { stringify as legacyStringifyLockfile } from '@yarnpkg/lockfile'; +import { ExtendedPackage } from '../monorepo'; const ENTRY_PATTERN = /^((?:@[^/]+\/)?[^@/]+)@(.+)$/; @@ -164,8 +165,11 @@ export class Lockfile { } /** Analyzes the lockfile to identify possible actions and warnings for the entries */ - analyze(options?: { filter?: (name: string) => boolean }): AnalyzeResult { - const { filter } = options ?? {}; + analyze(options: { + filter?: (name: string) => boolean; + localPackages: Map; + }): AnalyzeResult { + const { filter, localPackages } = options; const result: AnalyzeResult = { invalidRanges: [], newVersions: [], @@ -178,7 +182,9 @@ export class Lockfile { } // Get rid of and signal any invalid ranges upfront - const invalid = allEntries.filter(e => !semver.validRange(e.range)); + const invalid = allEntries.filter( + e => !semver.validRange(e.range) && !e.range.startsWith('workspace:'), + ); result.invalidRanges.push( ...invalid.map(({ range }) => ({ name, range })), ); @@ -190,36 +196,55 @@ export class Lockfile { } // Find all versions currently in use - const versions = Array.from(new Set(entries.map(e => e.version))).sort( - (v1, v2) => semver.rcompare(v1, v2), - ); + const versions = Array.from(new Set(entries.map(e => e.version))) + .map(v => { + // Translate workspace:^ references to the actual version + if (v === '0.0.0-use.local') { + const local = localPackages.get(name); + if (!local) { + throw new Error(`No local package found for ${name}`); + } + if (!local.packageJson.version) { + throw new Error(`No version found for local package ${name}`); + } + return { + entryVersion: v, + actualVersion: local.packageJson.version, + }; + } + return { entryVersion: v, actualVersion: v }; + }) + .sort((v1, v2) => semver.rcompare(v1.actualVersion, v2.actualVersion)); // If we're not using at least 2 different versions we're done if (versions.length < 2) { continue; } + // TODO(Rugvip): Support bumping into workspace ranges too const acceptedVersions = new Set(); for (const { version, range } of entries) { // Finds the highest matching version from the the known versions // TODO(Rugvip): We may want to select the version that satisfies the most ranges rather than the highest one - const acceptedVersion = versions.find(v => semver.satisfies(v, range)); + const acceptedVersion = versions.find(v => + semver.satisfies(v.actualVersion, range), + ); if (!acceptedVersion) { throw new Error( `No existing version was accepted for range ${range}, searching through ${versions}, for package ${name}`, ); } - if (acceptedVersion !== version) { + if (acceptedVersion.entryVersion !== version) { result.newVersions.push({ name, range, - newVersion: acceptedVersion, + newVersion: acceptedVersion.entryVersion, oldVersion: version, }); } - acceptedVersions.add(acceptedVersion); + acceptedVersions.add(acceptedVersion.actualVersion); } // If all ranges were able to accept the same version, we're done