diff --git a/.changeset/upgrade-catalyst-dependency-migration.md b/.changeset/upgrade-catalyst-dependency-migration.md new file mode 100644 index 000000000..d01a85482 --- /dev/null +++ b/.changeset/upgrade-catalyst-dependency-migration.md @@ -0,0 +1,11 @@ +--- +"@bigcommerce/catalyst": minor +--- + +`catalyst upgrade` now keeps your `@bigcommerce/catalyst*` dependencies up to date. + +Until now these versions never moved during an upgrade, so a project stayed pinned to whatever it was created with. The upgrade now brings them to the versions that shipped with the release you're upgrading to, handled like any other change: applied automatically, or flagged as a conflict if you'd pinned one on purpose. + +If your project still references these packages with `workspace:^`, the upgrade offers to swap them for published versions so your package manager can keep them current from then on. Declining, running without a terminal, or using `--dry-run` changes nothing; `--yes` accepts. + +Two new reminders round it out: run an install when the upgrade touched your `package.json`, and update `@bigcommerce/catalyst` itself when a newer version is out. diff --git a/packages/catalyst/src/cli/commands/upgrade.action.spec.ts b/packages/catalyst/src/cli/commands/upgrade.action.spec.ts index 1b7c9e513..074d93de8 100644 --- a/packages/catalyst/src/cli/commands/upgrade.action.spec.ts +++ b/packages/catalyst/src/cli/commands/upgrade.action.spec.ts @@ -5,6 +5,7 @@ * Downloads use the shared CLI cache so subsequent runs are offline. */ +import { confirm } from '@inquirer/prompts'; import { execa } from 'execa'; import { cp, mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; @@ -16,6 +17,7 @@ vi.setConfig({ hookTimeout: 60_000 }); import { downloadCore, parseRef, upgrade } from './upgrade'; +vi.mock('@inquirer/prompts', () => ({ confirm: vi.fn() })); vi.mock('../lib/telemetry', () => ({ getTelemetry: () => ({ track: vi.fn() }) })); vi.mock('../lib/logger', () => ({ consola: { log: vi.fn(), success: vi.fn(), error: vi.fn(), warn: vi.fn(), info: vi.fn() }, @@ -34,6 +36,10 @@ const TIMEOUT = 120_000; const createdDirs: string[] = []; let originalCwd: string; +// vitest.setup.ts pins stdin to non-interactive; opt back in for the branches +// that are TTY-gated, and reset in afterEach. +const setTty = (value: boolean) => + Object.defineProperty(process.stdin, 'isTTY', { value, configurable: true }); // JSON.parse returns `any`; centralise the unsafe-return suppression here. // eslint-disable-next-line @typescript-eslint/no-unsafe-return @@ -53,6 +59,8 @@ beforeEach(() => { afterEach(async () => { process.chdir(originalCwd); + setTty(false); + vi.mocked(confirm).mockReset(); vi.restoreAllMocks(); await Promise.all( createdDirs @@ -341,3 +349,64 @@ test( }, TIMEOUT, ); + +// ── workspace-protocol dependency migration ────────────────────────────────── +// A project seeded straight from the tarball still carries `workspace:^`, which +// is exactly the monorepo-structure case: `catalyst create` never rewrote it. + +const CLIENT = '@bigcommerce/catalyst-client'; + +const readDeps = async (projectDir: string): Promise> => { + const pkg = parseJson(await readFile(join(projectDir, 'package.json'), 'utf-8')); + + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions + return pkg.dependencies as Record; +}; + +// The gate is `stampedPkg && (--yes || (isTTY && confirm))`; these four rows walk it. +test.each([ + ['accepting the prompt migrates', [TARGET_VERSION], true, true, '^1.0.2'], + ['declining the prompt keeps workspace:', [TARGET_VERSION], true, false, 'workspace:^'], + ['--yes migrates without prompting', [TARGET_VERSION, '--yes'], true, undefined, '^1.0.2'], + ['a non-interactive run keeps workspace:', [TARGET_VERSION], false, undefined, 'workspace:^'], +] as const)( + '%s', + async (_name, argv, tty, confirmValue, expected) => { + const root = await mkTmp(); + const projectDir = await setup163Project(root); + + expect((await readDeps(projectDir))[CLIENT]).toBe('workspace:^'); + + setTty(tty); + + if (confirmValue !== undefined) vi.mocked(confirm).mockResolvedValue(confirmValue); + + process.chdir(projectDir); + await upgrade.parseAsync([...argv], { from: 'user' }); + + expect((await readDeps(projectDir))[CLIENT]).toBe(expected); + expect(confirm).toHaveBeenCalledTimes(confirmValue === undefined ? 0 : 1); + }, + TIMEOUT, +); + +test( + '--dry-run reports the workspace: references without rewriting them', + async () => { + const root = await mkTmp(); + const projectDir = await setup163Project(root); + + setTty(true); + + process.chdir(projectDir); + await upgrade.parseAsync([TARGET_VERSION, '--dry-run'], { from: 'user' }); + + expect(confirm).not.toHaveBeenCalled(); + expect((await readDeps(projectDir))[CLIENT]).toBe('workspace:^'); + + const status = (await execa('git', ['status', '--porcelain'], { cwd: projectDir })).stdout; + + expect(status.trim()).toBe(''); + }, + TIMEOUT, +); diff --git a/packages/catalyst/src/cli/commands/upgrade.integration.spec.ts b/packages/catalyst/src/cli/commands/upgrade.integration.spec.ts index 9bc7ebbd0..89a0bc111 100644 --- a/packages/catalyst/src/cli/commands/upgrade.integration.spec.ts +++ b/packages/catalyst/src/cli/commands/upgrade.integration.spec.ts @@ -19,9 +19,13 @@ import { downloadCore, mergeCorePerFile, mergeCoreTree, + normalizeWorkspaceDeps, resolveProject, + rewriteWorkspaceSpecifier, } from './upgrade'; +const CLIENT = '@bigcommerce/catalyst-client'; + const SUPPORTS_TREE = (() => { try { const match = /(\d+)\.(\d+)/.exec(execSync('git --version').toString()); @@ -98,6 +102,66 @@ describe.each(engines)('integration (engine: %s)', (engine) => { ? mergeCoreTree(baseDir, theirsDir, oursDir) : mergeCorePerFile(baseDir, theirsDir, oursDir, emptyFile); + test( + 'bumps a Catalyst dependency across the real 1.6.3 → 1.7.0 tags', + async () => { + const root = await mkTmp(); + const baseDir = join(root, 'base'); + const theirsDir = join(root, 'theirs'); + const [baseVersions, targetVersions] = await Promise.all([ + downloadCore(REPO, BASE_REF, baseDir), + downloadCore(REPO, TARGET_REF, theirsDir), + ]); + + // The published client really did move between these two tags — the whole + // reason the dependency needs reconciling at all. + expect(baseVersions[CLIENT]).toBe('1.0.1'); + expect(targetVersions[CLIENT]).toBe('1.0.2'); + + // Both tags still carry the workspace specifier in their committed tree, + // which is why an unassisted merge leaves the merchant's version frozen. + expect(await readFile(join(baseDir, 'package.json'), 'utf-8')).toContain( + `"${CLIENT}": "workspace:^"`, + ); + + // A flat project: `catalyst create` resolved the specifier at scaffold + // time, so the merchant holds a real range. + const oursDir = join(root, 'project'); + + await cp(baseDir, oursDir, { recursive: true }); + + const pkgPath = join(oursDir, 'package.json'); + + await writeFile( + pkgPath, + rewriteWorkspaceSpecifier(await readFile(pkgPath, 'utf-8'), CLIENT, '1.0.1'), + ); + await initGitProject(oursDir); + + const { workspace, bumped } = await normalizeWorkspaceDeps( + await readFile(pkgPath, 'utf-8'), + baseDir, + theirsDir, + baseVersions, + targetVersions, + ); + + expect(bumped).toContain(CLIENT); + // The project isn't on the workspace protocol, so there's nothing to migrate. + expect(workspace.map((finding) => finding.name)).not.toContain(CLIENT); + + const emptyFile = join(root, '.empty'); + + await writeFile(emptyFile, ''); + + const result = await runMerge(baseDir, theirsDir, oursDir, emptyFile); + + expect(result.conflicted).not.toContain('package.json'); + expect(await readFile(pkgPath, 'utf-8')).toContain(`"${CLIENT}": "^1.0.2"`); + }, + TIMEOUT, + ); + test( 'clean project upgrades without conflicts and all changes staged', async () => { diff --git a/packages/catalyst/src/cli/commands/upgrade.spec.ts b/packages/catalyst/src/cli/commands/upgrade.spec.ts index f2b81f347..dd1c70573 100644 --- a/packages/catalyst/src/cli/commands/upgrade.spec.ts +++ b/packages/catalyst/src/cli/commands/upgrade.spec.ts @@ -1,11 +1,15 @@ import { Command } from '@commander-js/extra-typings'; import { execa } from 'execa'; +import { http, HttpResponse } from 'msw'; import { execSync } from 'node:child_process'; import { access, mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { dirname, join } from 'node:path'; import { afterEach, describe, expect, test, vi } from 'vitest'; +import { server } from '../../../tests/mocks/node'; +import { detectLockfileManager } from '../lib/detect-package-manager'; + // The tree engine runs many git subprocesses sequentially on Windows CI; give // every test in this file enough headroom (the fast ones finish in < 1 s). vi.setConfig({ testTimeout: 30_000 }); @@ -13,12 +17,16 @@ vi.setConfig({ testTimeout: 30_000 }); import { applyIndexState, computeBaseSimilarity, + findStaleCli, mergeCorePerFile, mergeCoreTree, + migrateWorkspaceDeps, + normalizeWorkspaceDeps, parseRef, resolveBaseRef, resolveProject, resolveStrategy, + rewriteWorkspaceSpecifier, upgrade, } from './upgrade'; @@ -526,3 +534,257 @@ describe.skipIf(!SUPPORTS_TREE)('mergeCoreTree rename fidelity', () => { expect(await readFile(join(oursDir, 'new.ts'), 'utf-8')).toBe('a\nb\nc\nd\ne-merchant\nf\n'); }); }); + +// ── Catalyst dependency reconciliation ──────────────────────────────────────── + +const CLIENT = '@bigcommerce/catalyst-client'; +const ESLINT_CONFIG = '@bigcommerce/eslint-config-catalyst'; + +// Mirrors the shape of a real core/package.json closely enough for the textual +// rewrite: 2-space indent, deps split across two fields. +const corePkg = (client: string, eslintConfig: string): string => + `${JSON.stringify( + { + name: '@bigcommerce/catalyst-core', + version: '1.6.3', + dependencies: { [CLIENT]: client, next: '^15.5.0' }, + devDependencies: { [ESLINT_CONFIG]: eslintConfig }, + }, + null, + 2, + )}\n`; + +describe('rewriteWorkspaceSpecifier', () => { + test('swaps a workspace specifier for a caret range, leaving the rest byte-identical', () => { + const raw = corePkg('workspace:^', 'workspace:^'); + const rewritten = rewriteWorkspaceSpecifier(raw, CLIENT, '1.0.2'); + + expect(rewritten).toContain(`"${CLIENT}": "^1.0.2"`); + // Only that one value moved. + expect(rewritten).toBe(raw.replace('"workspace:^",', '"^1.0.2",')); + }); + + test('leaves a dependency that is already a real range alone', () => { + const raw = corePkg('^1.0.1', 'workspace:^'); + + expect(rewriteWorkspaceSpecifier(raw, CLIENT, '1.0.2')).toBe(raw); + }); + + test('is a no-op for a package that is not present', () => { + const raw = corePkg('workspace:^', 'workspace:^'); + + expect(rewriteWorkspaceSpecifier(raw, '@bigcommerce/nope', '9.9.9')).toBe(raw); + }); +}); + +describe('normalizeWorkspaceDeps', () => { + // Writes base/theirs trees carrying `workspace:^` (what every tag actually + // ships) and returns their dirs. + async function trees(root: string): Promise<{ baseDir: string; theirsDir: string }> { + const baseDir = join(root, 'base'); + const theirsDir = join(root, 'theirs'); + + await Promise.all([ + write(join(baseDir, 'package.json'), corePkg('workspace:^', 'workspace:^')), + write(join(theirsDir, 'package.json'), corePkg('workspace:^', 'workspace:^')), + ]); + + return { baseDir, theirsDir }; + } + + test('rewrites both sides to each tag version when the project holds a real range', async () => { + const root = await mkTmp(); + const { baseDir, theirsDir } = await trees(root); + + const result = await normalizeWorkspaceDeps( + corePkg('^1.0.1', '^1.0.0'), + baseDir, + theirsDir, + { [CLIENT]: '1.0.1', [ESLINT_CONFIG]: '1.0.0' }, + { [CLIENT]: '1.0.2', [ESLINT_CONFIG]: '1.0.0' }, + ); + + expect(await readFile(join(baseDir, 'package.json'), 'utf-8')).toContain( + `"${CLIENT}": "^1.0.1"`, + ); + expect(await readFile(join(theirsDir, 'package.json'), 'utf-8')).toContain( + `"${CLIENT}": "^1.0.2"`, + ); + expect(result.workspace).toEqual([]); + // eslint-config-catalyst didn't move between the two tags. + expect(result.bumped).toEqual([CLIENT]); + }); + + test('leaves both sides untouched for a dependency the project still holds as workspace:', async () => { + const root = await mkTmp(); + const { baseDir, theirsDir } = await trees(root); + const before = await readFile(join(baseDir, 'package.json'), 'utf-8'); + + const result = await normalizeWorkspaceDeps( + corePkg('workspace:^', 'workspace:^'), + baseDir, + theirsDir, + { [CLIENT]: '1.0.1', [ESLINT_CONFIG]: '1.0.0' }, + { [CLIENT]: '1.0.2', [ESLINT_CONFIG]: '1.0.0' }, + ); + + // Normalizing one side only would manufacture a conflict on a dependency + // that works fine as-is, so neither side moves. + expect(await readFile(join(baseDir, 'package.json'), 'utf-8')).toBe(before); + expect(await readFile(join(theirsDir, 'package.json'), 'utf-8')).toBe(before); + expect(result.bumped).toEqual([]); + expect(result.workspace).toEqual([ + { name: CLIENT, ours: 'workspace:^', version: '1.0.2' }, + { name: ESLINT_CONFIG, ours: 'workspace:^', version: '1.0.0' }, + ]); + }); + + test('leaves a side alone when that tag never published the package', async () => { + const root = await mkTmp(); + const { baseDir, theirsDir } = await trees(root); + + const result = await normalizeWorkspaceDeps( + corePkg('^1.0.1', '^1.0.0'), + baseDir, + theirsDir, + {}, // the package didn't exist at the base ref + { [CLIENT]: '1.0.2' }, + ); + + expect(await readFile(join(baseDir, 'package.json'), 'utf-8')).toContain( + `"${CLIENT}": "workspace:^"`, + ); + expect(await readFile(join(theirsDir, 'package.json'), 'utf-8')).toContain( + `"${CLIENT}": "^1.0.2"`, + ); + // Nothing to compare against on the base side, so no bump is claimed. + expect(result.bumped).toEqual([]); + }); + + test('returns nothing when a downloaded package.json is missing', async () => { + const root = await mkTmp(); + + await expect( + normalizeWorkspaceDeps( + corePkg('^1.0.1', '^1.0.0'), + join(root, 'nope'), + join(root, 'gone'), + {}, + {}, + ), + ).resolves.toEqual({ workspace: [], bumped: [] }); + }); +}); + +describe('migrateWorkspaceDeps', () => { + test('applies every finding to the project package.json text', () => { + const migrated = migrateWorkspaceDeps(corePkg('workspace:^', 'workspace:*'), [ + { name: CLIENT, ours: 'workspace:^', version: '1.0.2' }, + { name: ESLINT_CONFIG, ours: 'workspace:*', version: '1.0.0' }, + ]); + + expect(migrated).toContain(`"${CLIENT}": "^1.0.2"`); + expect(migrated).toContain(`"${ESLINT_CONFIG}": "^1.0.0"`); + expect(migrated).not.toContain('workspace:'); + }); +}); + +describe('findStaleCli', () => { + const withRegistryVersion = (version: string) => + server.use( + http.get('https://registry.npmjs.org/:scope/:name/latest', () => + HttpResponse.json({ name: '@bigcommerce/catalyst', version }), + ), + ); + + const projectWith = (cliRange: string) => + JSON.stringify({ devDependencies: { '@bigcommerce/catalyst': cliRange } }); + + test('reports the gap when the pinned CLI is behind the published one', async () => { + withRegistryVersion('1.2.0'); + + await expect(findStaleCli(projectWith('1.1.0'))).resolves.toEqual({ + current: '1.1.0', + latest: '1.2.0', + }); + }); + + test('stays quiet when the project is already on the published version', async () => { + withRegistryVersion('1.2.0'); + + await expect(findStaleCli(projectWith('1.2.0'))).resolves.toBeNull(); + }); + + test('stays quiet when the range already admits the published version', async () => { + withRegistryVersion('1.2.5'); + + // `^1.2.0` picks up 1.2.5 on the next install, so there is nothing to say. + await expect(findStaleCli(projectWith('^1.2.0'))).resolves.toBeNull(); + }); + + test('reports a range that cannot reach the published version', async () => { + withRegistryVersion('2.0.0'); + + await expect(findStaleCli(projectWith('^1.1.0'))).resolves.toEqual({ + current: '1.1.0', + latest: '2.0.0', + }); + }); + + test('stays quiet when the registry returns a non-semver version', async () => { + withRegistryVersion('not-a-version'); + + await expect(findStaleCli(projectWith('1.1.0'))).resolves.toBeNull(); + }); + + test('skips the registry entirely when the project has no CLI dependency', async () => { + await expect( + findStaleCli(JSON.stringify({ dependencies: { next: '^15.5.0' } })), + ).resolves.toBeNull(); + }); + + test('ignores an unparseable range rather than throwing', async () => { + await expect(findStaleCli(projectWith('catalyst.tgz'))).resolves.toBeNull(); + }); + + test('stays quiet when the registry is unreachable', async () => { + // The default handler 404s. + await expect(findStaleCli(projectWith('1.1.0'))).resolves.toBeNull(); + }); + + test('stays quiet when package.json still has conflict markers', async () => { + await expect( + findStaleCli('<<<<<<< ours\n{}\n=======\n{}\n>>>>>>> theirs\n'), + ).resolves.toBeNull(); + }); +}); + +describe('detectLockfileManager', () => { + test.each([ + ['pnpm-lock.yaml', 'pnpm'], + ['yarn.lock', 'yarn'], + ['bun.lock', 'bun'], + ['package-lock.json', 'npm'], + ])('maps %s to %s', async (lockfile, manager) => { + const root = await mkTmp(); + + await write(join(root, lockfile), ''); + + expect(await detectLockfileManager(root)).toBe(manager); + }); + + test('prefers pnpm when several lockfiles are present', async () => { + const root = await mkTmp(); + + await Promise.all([ + write(join(root, 'package-lock.json'), ''), + write(join(root, 'pnpm-lock.yaml'), ''), + ]); + + expect(await detectLockfileManager(root)).toBe('pnpm'); + }); + + test('returns null when there is no lockfile, so the caller can keep looking', async () => { + expect(await detectLockfileManager(await mkTmp())).toBeNull(); + }); +}); diff --git a/packages/catalyst/src/cli/commands/upgrade.ts b/packages/catalyst/src/cli/commands/upgrade.ts index 8f0b8ea94..4908cafcf 100644 --- a/packages/catalyst/src/cli/commands/upgrade.ts +++ b/packages/catalyst/src/cli/commands/upgrade.ts @@ -15,9 +15,11 @@ import { } from 'node:fs/promises'; import { homedir, tmpdir } from 'node:os'; import { dirname, join, relative } from 'node:path'; +import { minVersion, satisfies, lt as semverLt, validRange, valid as validSemver } from 'semver'; import yoctoSpinner from 'yocto-spinner'; import { z } from 'zod'; +import { detectLockfileManager, PackageManager } from '../lib/detect-package-manager'; import { consola } from '../lib/logger'; import { getTelemetry } from '../lib/telemetry'; @@ -27,6 +29,26 @@ const CorePackageJson = z.object({ catalyst: z.object({ version: z.string(), ref: z.string() }).optional(), }); +const DEP_FIELDS = [ + 'dependencies', + 'devDependencies', + 'optionalDependencies', + 'peerDependencies', +] as const; + +const DepsPackageJson = z.looseObject({ + dependencies: z.record(z.string(), z.string()).optional(), + devDependencies: z.record(z.string(), z.string()).optional(), + optionalDependencies: z.record(z.string(), z.string()).optional(), + peerDependencies: z.record(z.string(), z.string()).optional(), +}); + +const WorkspacePackageJson = z.object({ name: z.string(), version: z.string() }); + +// The CLI itself. `catalyst create` pins it exactly in devDependencies and it +// doesn't exist in the upstream tree, so no merge will ever move it. +const CATALYST_CLI_PACKAGE = '@bigcommerce/catalyst'; + // Catalyst versions are git tags on the upstream monorepo (e.g. // "@bigcommerce/catalyst-core@1.7.0"), NOT npm packages. GitHub serves a // tarball for any tag via the repos///tarball/ endpoint. @@ -125,6 +147,165 @@ async function readResolvedVersion(coreDir: string): Promise { return pkg.catalyst?.version ?? pkg.version; } +// ── Catalyst dependency reconciliation ──────────────────────────────────────── +// Every tag's committed core/package.json still carries `workspace:` specifiers +// for the Catalyst packages: core is `private`, so it is never published and +// pnpm's publish-time rewrite never runs on the tagged tree. Base and theirs +// therefore agree on those lines, the merge correctly leaves ours alone, and the +// merchant's Catalyst dependency versions never move. `catalyst create` resolves +// them at scaffold time instead, which is why flat projects hold a real range +// and monorepo-structure projects still hold `workspace:^`. + +// Package name → the version that tag published. Indexing can miss: a package +// that didn't exist yet at an older ref simply isn't in that tag's manifest. +export type WorkspaceVersions = Record; + +// A Catalyst dependency the merchant still references through the workspace +// protocol, alongside the published version it would migrate to. +export interface WorkspaceDepFinding { + name: string; + ours: string; + version: string; +} + +const readDepSpecifiers = (raw: string): Record => { + let parsed: z.infer; + + try { + parsed = DepsPackageJson.parse(JSON.parse(raw)); + } catch { + return {}; // conflict markers or hand-broken JSON — nothing to reconcile + } + + return Object.fromEntries(DEP_FIELDS.flatMap((field) => Object.entries(parsed[field] ?? {}))); +}; + +// Scoped package names carry "@" and "/", and some carry "."; escape before +// embedding in a pattern. +const escapeRegExp = (value: string) => value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + +// Swap a dependency's `workspace:` specifier for `^` in raw +// package.json text. Deliberately textual rather than a JSON round-trip: parsing +// and re-stringifying would reformat the whole file, desyncing the downloaded +// trees from the merchant's copy and turning a one-line dependency bump into a +// whole-file conflict. It also keeps working on a file that still has conflict +// markers in other sections. +export const rewriteWorkspaceSpecifier = (raw: string, name: string, version: string): string => + raw.replace( + new RegExp(`("${escapeRegExp(name)}"\\s*:\\s*)"workspace:[^"]*"`, 'g'), + (_match, key: string) => `${key}"^${version}"`, + ); + +// Rewrites the downloaded base/theirs package.json so each side names the +// version its own tag actually shipped. The normal 3-way merge then carries the +// bump for free — or conflicts, if the merchant pinned deliberately. +// +// A dependency the merchant still holds as `workspace:` is skipped on both +// sides: normalizing there would manufacture a conflict on something that works +// fine as-is. Those come back as findings for the migration prompt instead. +export async function normalizeWorkspaceDeps( + ourPkgRaw: string, + baseDir: string, + theirsDir: string, + baseVersions: WorkspaceVersions, + targetVersions: WorkspaceVersions, +): Promise<{ workspace: WorkspaceDepFinding[]; bumped: string[] }> { + const basePath = join(baseDir, 'package.json'); + const theirsPath = join(theirsDir, 'package.json'); + const [originalBase, originalTheirs] = await Promise.all([ + readFile(basePath, 'utf-8').catch(() => null), + readFile(theirsPath, 'utf-8').catch(() => null), + ]); + + if (originalBase === null || originalTheirs === null) return { workspace: [], bumped: [] }; + + const ourSpecifiers = readDepSpecifiers(ourPkgRaw); + const names = new Set( + [ + ...Object.entries(readDepSpecifiers(originalBase)), + ...Object.entries(readDepSpecifiers(originalTheirs)), + ] + .filter(([, range]) => range?.startsWith('workspace:')) + .map(([name]) => name), + ); + + const decisions = [...names].map((name) => ({ + name, + ours: ourSpecifiers[name], + baseVersion: baseVersions[name], + targetVersion: targetVersions[name], + })); + + const workspace = decisions.flatMap(({ name, ours, targetVersion }) => + ours?.startsWith('workspace:') && targetVersion !== undefined + ? [{ name, ours, version: targetVersion }] + : [], + ); + + // Everything else gets both sides pinned to the version its own tag shipped. + // A version is missing when the package didn't exist at that ref yet; leave + // that side untouched so the merge sees no change rather than a bogus one. + const normalizable = decisions.filter(({ ours }) => !ours?.startsWith('workspace:')); + + const baseRaw = normalizable.reduce( + (acc, { name, baseVersion }) => + baseVersion === undefined ? acc : rewriteWorkspaceSpecifier(acc, name, baseVersion), + originalBase, + ); + const theirsRaw = normalizable.reduce( + (acc, { name, targetVersion }) => + targetVersion === undefined ? acc : rewriteWorkspaceSpecifier(acc, name, targetVersion), + originalTheirs, + ); + const bumped = normalizable + .filter( + ({ baseVersion, targetVersion }) => + baseVersion !== undefined && targetVersion !== undefined && baseVersion !== targetVersion, + ) + .map(({ name }) => name); + + await Promise.all([ + baseRaw === originalBase ? null : writeFile(basePath, baseRaw), + theirsRaw === originalTheirs ? null : writeFile(theirsPath, theirsRaw), + ]); + + return { workspace, bumped }; +} + +// Applies the findings to the merchant's own package.json text. +export const migrateWorkspaceDeps = (raw: string, findings: WorkspaceDepFinding[]): string => + findings.reduce((acc, { name, version }) => rewriteWorkspaceSpecifier(acc, name, version), raw); + +// The CLI that performs upgrades can't upgrade itself through the merge, so +// check npm directly. Best-effort: a registry hiccup must never fail an upgrade. +export async function findStaleCli( + pkgRaw: string, +): Promise<{ current: string; latest: string } | null> { + const range = readDepSpecifiers(pkgRaw)[CATALYST_CLI_PACKAGE]; + + // A merchant may point the dep at a tarball, git URL, or `link:` path, none of + // which minVersion() can parse (it throws rather than returning null). + if (range === undefined || !validRange(range)) return null; + + const body: unknown = await fetch(`https://registry.npmjs.org/${CATALYST_CLI_PACKAGE}/latest`, { + // A hanging registry would otherwise stall the CLI indefinitely. + signal: AbortSignal.timeout(5000), + }) + .then((res) => (res.ok ? res.json() : null)) + .catch(() => null); + const latest = z.object({ version: z.string() }).safeParse(body).data?.version; + + // The registry promises a string, not a semver one, and an unparseable version + // would throw straight out of satisfies(). A range that already admits the + // published version needs no advisory either — the next install picks it up, + // which leaves this for the exact pin `catalyst create` writes. + if (latest === undefined || !validSemver(latest) || satisfies(latest, range)) return null; + + const current = minVersion(range)?.version; + + return current !== undefined && semverLt(current, latest) ? { current, latest } : null; +} + // ── per-file 3-way merge engine ─────────────────────────────────────────────── type MergeOutcome = 'applied' | 'added' | 'deleted' | 'conflicted'; @@ -620,31 +801,79 @@ export async function resolveProject(cwd: string): Promise { } // ── tarball download (source side stays the monorepo) ───────────────────────── +// Harvests the versions the monorepo's publishable packages carried at this ref. +// This is the only place they're knowable: the tag's core/package.json names them +// as `workspace:`, and they version independently of core (no changesets +// `linked`/`fixed` group), so an npm `latest` lookup would answer for today +// rather than for the ref being downloaded. +const readWorkspaceVersions = async (repoDir: string): Promise => { + const packagesDir = join(repoDir, 'packages'); + const entries = await readdir(packagesDir, { withFileTypes: true }).catch(() => []); + + const manifests = await Promise.all( + entries + .filter((entry) => entry.isDirectory()) + .map(async (entry) => { + const raw = await readFile(join(packagesDir, entry.name, 'package.json'), 'utf-8').catch( + () => null, + ); + + if (raw === null) return null; + + try { + return WorkspacePackageJson.parse(JSON.parse(raw)); + } catch { + return null; + } + }), + ); + + return Object.fromEntries( + manifests.filter((pkg) => pkg !== null).map(({ name, version }) => [name, version]), + ); +}; + +const readCachedVersions = async (path: string): Promise => { + const raw = await readFile(path, 'utf-8').catch(() => null); + + if (raw === null) return null; + + try { + return z.record(z.string(), z.string()).parse(JSON.parse(raw)); + } catch { + return null; + } +}; + export async function downloadCore( repository: string, ref: string, destDir: string, token?: string, -): Promise { +): Promise { // Never cache moving tags (latest/canary/alpha) — they'd go stale silently. const cacheable = !MOVING_TAGS.has(parseRef(ref).version); // Use a separator before the ref so that repository strings differing only // by characters that sanitize to '_' (e.g. '/' vs '_') produce distinct keys. const cacheKey = `${repository.replace(/[^a-zA-Z0-9._-]/g, '_')}__${ref.replace(/[^a-zA-Z0-9._-]/g, '_')}`; const cachePath = join(CACHE_DIR, cacheKey); + const versionsPath = `${cachePath}.versions.json`; // Guard against a partially-written cache entry: two concurrent downloadCore // calls for the same ref can race — the first creates cachePath mid-copy, and // the second sees it as present but gets an incomplete directory. Validate with - // a package.json sentinel (every extracted core/ tree must have one). - if ( - cacheable && - (await pathExists(cachePath)) && - (await pathExists(join(cachePath, 'package.json'))) - ) { + // a package.json sentinel (every extracted core/ tree must have one) plus the + // versions sidecar, which also makes entries written before the sidecar existed + // fall through to a fresh download rather than answering with no versions. + const cachedVersions = + cacheable && (await pathExists(join(cachePath, 'package.json'))) + ? await readCachedVersions(versionsPath) + : null; + + if (cachedVersions) { await cp(cachePath, destDir, { recursive: true }); - return; + return cachedVersions; } // encodeURIComponent turns "@bigcommerce/catalyst-core@1.7.0" into @@ -681,16 +910,23 @@ export async function downloadCore( const coreDir = join(rawDir, 'core'); const sourceDir = (await pathExists(coreDir)) ? coreDir : rawDir; + // Read the sibling packages before rawDir is torn down — destDir keeps core/ only. + const workspaceVersions = await readWorkspaceVersions(rawDir); + await rename(sourceDir, destDir); await rm(tarballPath, { force: true }); await rm(rawDir, { recursive: true, force: true }); if (cacheable) { await mkdir(CACHE_DIR, { recursive: true }); - await cp(destDir, cachePath, { recursive: true }).catch(() => { - /* best-effort cache; ignore failures */ - }); + await cp(destDir, cachePath, { recursive: true }) + .then(() => writeFile(versionsPath, JSON.stringify(workspaceVersions))) + .catch(() => { + /* best-effort cache; ignore failures */ + }); } + + return workspaceVersions; } // ── base-ref resolution / auto-detect ───────────────────────────────────────── @@ -727,7 +963,36 @@ export async function resolveBaseRef( } // ── summary output ──────────────────────────────────────────────────────────── -function printSummary(result: MergeResult, relDir: string, stampedPkg: boolean): void { +function printWorkspaceFindings(findings: WorkspaceDepFinding[]): void { + if (findings.length === 0) return; + + const pad = Math.max(...findings.map((finding) => finding.name.length)); + const rows = findings + .map((finding) => ` ${finding.name.padEnd(pad)} ${finding.ours} → ^${finding.version}`) + .join('\n'); + + consola.warn( + `Catalyst dependencies still using the workspace protocol (${findings.length}):\n${rows}\n\nThey resolve to your local packages/ copies, which \`catalyst upgrade\` does not update — so they never receive upgrades.`, + ); +} + +interface Summary { + result: MergeResult; + relDir: string; + stampedPkg: boolean; + migratedDeps: number; + packageManager: PackageManager; + staleCli: { current: string; latest: string } | null; +} + +function printSummary({ + result, + relDir, + stampedPkg, + migratedDeps, + packageManager, + staleCli, +}: Summary): void { // package.json is excluded from the conflict list when the stamp resolved it. const unresolved = result.conflicted.filter((f) => !(stampedPkg && f === 'package.json')); @@ -741,15 +1006,38 @@ function printSummary(result: MergeResult, relDir: string, stampedPkg: boolean): consola.log(`\n${parts.join(', ')}`); + // Any package.json change can move dependencies, and the upgrade never installs, + // so the lockfile is left behind either way. + const pkgConflicted = unresolved.includes('package.json'); + const depsChanged = + migratedDeps > 0 || + [...result.applied, ...result.added, ...result.conflicted].includes('package.json'); + + // Stay quiet while package.json itself is unresolved: an install against a + // manifest full of conflict markers just fails. It joins the resolution steps + // below instead, so it runs in the right order. + if (depsChanged && !pkgConflicted) { + consola.info( + `Dependencies changed — run \`${packageManager} install\` to update your lockfile.`, + ); + } + if (unresolved.length) { const files = unresolved.map((f) => ` ${f}`).join('\n'); + const install = depsChanged && pkgConflicted ? ` && ${packageManager} install` : ''; consola.warn( - `\nStaged the clean changes. ${unresolved.length} file(s) need conflict resolution (the <<>> markers):\n${files}\n\nResolve them, then: git add ${relDir} && git commit`, + `\nStaged the clean changes. ${unresolved.length} file(s) need conflict resolution (the <<>> markers):\n${files}\n\nResolve them, then: git add ${relDir} && git commit${install}`, ); } else { consola.success('Staged all changes — review with `git diff --cached`, then commit.'); } + + if (staleCli) { + consola.info( + `Your Catalyst CLI is behind (${staleCli.current} → ${staleCli.latest}). Update it with \`${packageManager} add -D ${CATALYST_CLI_PACKAGE}@${staleCli.latest}\`.`, + ); + } } export const upgrade = new Command('upgrade') @@ -893,10 +1181,20 @@ to raise the GitHub API rate limit.`, await writeFile(emptyFile, ''); + // Start the registry check now so its round-trip overlaps the tarball + // downloads rather than adding dead air after the merge. Reading the + // pre-merge manifest is also more robust: the CLI dependency isn't in + // either upstream tree, so nothing below can move it, and this way the + // advisory survives a merge that leaves conflict markers in package.json. + const staleCliCheck = findStaleCli(project.rawContent).catch(() => null); + const downloadSpinner = yoctoSpinner().start(`Downloading ${baseRef} and ${upstreamRef}...`); + let baseVersions: WorkspaceVersions; + let targetVersions: WorkspaceVersions; + try { - await Promise.all([ + [baseVersions, targetVersions] = await Promise.all([ downloadCore(options.repository, baseRef, baseDir, token), downloadCore(options.repository, upstreamRef, theirsDir, token), ]); @@ -931,6 +1229,18 @@ to raise the GitHub API rate limit.`, ? `${basePackage}@${resolvedVersion}` : upstreamRef; + // ── 3c. Teach the merge about Catalyst dependency versions ────────── + // Runs before the dry run so the preview shows the dependency change too. + // rawContent predates the 3b backfill, which only touches the `catalyst` + // field — dependency specifiers are identical either way. + const { workspace: workspaceDeps, bumped } = await normalizeWorkspaceDeps( + project.rawContent, + baseDir, + theirsDir, + baseVersions, + targetVersions, + ); + // ── 4. Dry run: show the unified diff and stop ────────────────────── if (options.dryRun) { const diffSpinner = yoctoSpinner().start('Generating diff...'); @@ -957,6 +1267,8 @@ to raise the GitHub API rate limit.`, consola.log('\nDiff preview (--dry-run, not applied):\n'); consola.log(diff.stdout); + printWorkspaceFindings(workspaceDeps); + return; } @@ -988,14 +1300,17 @@ to raise the GitHub API rate limit.`, // ── 6. Stamp catalyst.ref (skip if package.json itself conflicted) ── const patchedRaw = await readFile(pkgPath, 'utf-8'); - let stampedPkg = false; + // Non-null once the stamp has written clean JSON, and then it is exactly + // what sits on disk. Stays null when package.json came out of the merge + // carrying conflict markers. + let stampedText: string | null = null; try { const patchedPkg = z.record(z.string(), z.unknown()).parse(JSON.parse(patchedRaw)); patchedPkg.catalyst = { version: resolvedVersion, ref: newRef }; - await writeFile(pkgPath, `${JSON.stringify(patchedPkg, null, 2)}\n`); - stampedPkg = true; + stampedText = `${JSON.stringify(patchedPkg, null, 2)}\n`; + await writeFile(pkgPath, stampedText); consola.success(`catalyst.ref updated → ${newRef}`); } catch { // package.json has conflict markers (scripts, deps, etc.). The catalyst @@ -1020,7 +1335,7 @@ to raise the GitHub API rate limit.`, if (updated !== patchedRaw) { await writeFile(pkgPath, updated); - // stampedPkg stays false — the file still has conflict markers in other + // stampedText stays null — the file still has conflict markers in other // sections, so it must stay as a UU unmerged entry so the editor shows // the merge UI. Only the catalyst field was resolved in place. consola.success(`catalyst.ref updated → ${newRef}`); @@ -1034,11 +1349,44 @@ to raise the GitHub API rate limit.`, } } + // ── 6b. Offer to migrate workspace-protocol Catalyst dependencies ──── + printWorkspaceFindings(workspaceDeps); + + let migratedDeps = 0; + + if (workspaceDeps.length) { + // Rewriting a package.json that still holds conflict markers could land + // inside a hunk the merchant hasn't reviewed, so the swap is only ever + // applied to a file the stamp already parsed cleanly. Past that, --yes + // means yes (as it does for the inferred base ref), and a non-interactive + // run falls through to printing the versions. + const accepted = + stampedText !== null && + (options.yes === true || + (Boolean(process.stdin.isTTY) && + (await confirm({ + message: 'Replace them with the published versions?', + default: true, + }).catch(() => false)))); + + if (accepted && stampedText !== null) { + await writeFile(pkgPath, migrateWorkspaceDeps(stampedText, workspaceDeps)); + migratedDeps = workspaceDeps.length; + consola.success( + `Migrated ${migratedDeps} dependenc${migratedDeps === 1 ? 'y' : 'ies'} to published versions.`, + ); + } else { + consola.info( + "Keeping the workspace references. Switching abandons any local packages/ copy you've customized; otherwise apply the versions above in package.json and reinstall.", + ); + } + } + // ── 7. Stage the clean changes; mark conflicts as real unmerged entries ─ // Staging is a convenience; the merge already landed on disk, so never let // a git quirk here fail the whole upgrade. try { - await applyIndexState(gitRoot, relDir, baseDir, theirsDir, result, stampedPkg); + await applyIndexState(gitRoot, relDir, baseDir, theirsDir, result, stampedText !== null); } catch (err) { consola.warn( `Couldn't auto-stage the changes (${err instanceof Error ? err.message : String(err)}). Your files are merged on disk — run \`git add ${relDir}\` yourself.`, @@ -1049,6 +1397,12 @@ to raise the GitHub API rate limit.`, .then((r) => r.stdout.trim()) .catch(() => 'unknown'); + // The lockfile lives at the repo root for both layouts and is committed by + // every real project, so it answers this on its own. Catalyst scaffolds with + // pnpm, which makes it the least-surprising answer when there isn't one. + const packageManager = (await detectLockfileManager(gitRoot)) ?? 'pnpm'; + const staleCli = await staleCliCheck; + await getTelemetry().track('upgrade', { strategy, gitVersion, @@ -1058,9 +1412,20 @@ to raise the GitHub API rate limit.`, deleted: result.deleted.length, conflicts: result.conflicted.length, hasConflicts: result.conflicted.length > 0, + workspaceDepsFound: workspaceDeps.length, + workspaceDepsMigrated: migratedDeps, + catalystDepsBumped: bumped.length, + cliOutdated: staleCli !== null, }); - printSummary(result, relDir, stampedPkg); + printSummary({ + result, + relDir, + stampedPkg: stampedText !== null, + migratedDeps, + packageManager, + staleCli, + }); } finally { await rm(tmpDir, { recursive: true, force: true, maxRetries: 10, retryDelay: 500 }); } diff --git a/packages/catalyst/src/cli/lib/detect-package-manager.ts b/packages/catalyst/src/cli/lib/detect-package-manager.ts index 6e4e681dd..c9bad15f4 100644 --- a/packages/catalyst/src/cli/lib/detect-package-manager.ts +++ b/packages/catalyst/src/cli/lib/detect-package-manager.ts @@ -1,7 +1,36 @@ +import { access } from 'node:fs/promises'; +import { join } from 'node:path'; import { detectPackageManager as detectFromDir } from 'nypm'; export type PackageManager = 'npm' | 'pnpm' | 'yarn' | 'bun'; +// Lockfile -> package manager, in nypm's own check order. +const PROJECT_LOCKFILES = [ + ['pnpm-lock.yaml', 'pnpm'], + ['yarn.lock', 'yarn'], + ['bun.lock', 'bun'], + ['bun.lockb', 'bun'], + ['package-lock.json', 'npm'], +] as const satisfies ReadonlyArray; + +// Detect a project's package manager from its lockfile alone, returning null +// when the directory has none so the caller can decide its own last resort. +// Unlike detectProjectPackageManager() this never throws — nypm parses +// package.json to read `packageManager`, which blows up on a manifest left full +// of merge conflict markers — and it can express "I don't know", which nypm +// can't, since it falls back to npm internally. +export const detectLockfileManager = async (dir: string): Promise => { + const found = await Promise.all( + PROJECT_LOCKFILES.map(async ([file, manager]) => + access(join(dir, file)) + .then(() => manager) + .catch(() => null), + ), + ); + + return found.find((manager) => manager !== null) ?? null; +}; + // Detect the package manager that INVOKED the CLI (via npx / pnpm dlx / yarn dlx // / bunx) by parsing the leading token of npm_config_user_agent, which every // manager sets when it spawns a child process. This is deliberately not diff --git a/packages/catalyst/src/cli/lib/rewrite-core-package.ts b/packages/catalyst/src/cli/lib/rewrite-core-package.ts index acf32a4d3..4fce363e4 100644 --- a/packages/catalyst/src/cli/lib/rewrite-core-package.ts +++ b/packages/catalyst/src/cli/lib/rewrite-core-package.ts @@ -45,9 +45,15 @@ const registryVersionSchema = z.object({ version: z.string() }); // Resolve the published npm version for a workspace dependency. The extracted // tag tree still carries `workspace:^` (core is private, so changesets' publish- // time rewrite never touches the committed tree) — so we resolve the real -// version at extraction time from the npm registry. The two workspace deps -// (`@bigcommerce/catalyst-client`, `@bigcommerce/eslint-config-catalyst`) are -// published in lockstep with core, so `latest` is the compatible version. +// version at extraction time from the npm registry. +// +// NOTE: `latest` answers for today, not for `ref`. These packages version +// independently of core (the changesets config has no `linked`/`fixed` group), +// so scaffolding from an older `--gh-ref` can splice in a newer version than +// that tag shipped. Harmless while they stay in step, wrong at the first major +// bump. `catalyst upgrade` reads the versions out of the tag's own +// `packages/*/package.json`; this path can't yet, because `extractCatalyst` +// filters the tarball down to `core/`. Tracked as a follow-up. const resolvePublishedVersion = async (pkgName: string): Promise => { const response = await fetch(`https://registry.npmjs.org/${pkgName}/latest`); diff --git a/packages/catalyst/tests/mocks/handlers.ts b/packages/catalyst/tests/mocks/handlers.ts index 3a7b91a49..5182d57fb 100644 --- a/packages/catalyst/tests/mocks/handlers.ts +++ b/packages/catalyst/tests/mocks/handlers.ts @@ -309,4 +309,10 @@ export const handlers = [ data: { id: 1, url: 'https://example.com', channel_id: 1 }, }), ), + + // Default handler for the npm registry — 404 so the stale-CLI check stays + // silent by default. Tests that assert on it override with a version payload. + http.get('https://registry.npmjs.org/:scope/:name/latest', () => + HttpResponse.json({ error: 'Not found' }, { status: 404 }), + ), ];