From 90a98220563227e88b020edb1c097c13281d8607 Mon Sep 17 00:00:00 2001 From: Jacek Date: Tue, 17 Feb 2026 21:23:00 -0600 Subject: [PATCH] fix(upgrade): improve CLI usability for monorepos and CI environments - Traverse parent directories for lockfiles and packageManager field in package.json - Add -w flag for pnpm install/remove at workspace roots - Resolve catalog: protocol versions from pnpm-workspace.yaml - Show actionable example commands in non-interactive error messages - Pass verbose: 0 to jscodeshift for error-level file path logging --- .../nextjs-catalog-resolved/package.json | 9 ++ .../nextjs-catalog-resolved/pnpm-lock.yaml | 4 + .../pnpm-workspace.yaml | 7 + .../nextjs-catalog-resolved/src/app.tsx | 9 ++ .../src/__tests__/integration/cli.test.js | 9 ++ .../__tests__/integration/detect-sdk.test.js | 135 +++++++++++++++++- packages/upgrade/src/cli.js | 12 +- packages/upgrade/src/codemods/index.js | 2 + packages/upgrade/src/render.js | 10 +- packages/upgrade/src/runner.js | 6 +- packages/upgrade/src/util/detect-sdk.js | 35 +++++ packages/upgrade/src/util/package-manager.js | 60 +++++--- 12 files changed, 268 insertions(+), 30 deletions(-) create mode 100644 packages/upgrade/src/__tests__/fixtures/nextjs-catalog-resolved/package.json create mode 100644 packages/upgrade/src/__tests__/fixtures/nextjs-catalog-resolved/pnpm-lock.yaml create mode 100644 packages/upgrade/src/__tests__/fixtures/nextjs-catalog-resolved/pnpm-workspace.yaml create mode 100644 packages/upgrade/src/__tests__/fixtures/nextjs-catalog-resolved/src/app.tsx diff --git a/packages/upgrade/src/__tests__/fixtures/nextjs-catalog-resolved/package.json b/packages/upgrade/src/__tests__/fixtures/nextjs-catalog-resolved/package.json new file mode 100644 index 00000000000..653e8d6d93b --- /dev/null +++ b/packages/upgrade/src/__tests__/fixtures/nextjs-catalog-resolved/package.json @@ -0,0 +1,9 @@ +{ + "name": "test-nextjs-catalog-resolved", + "version": "1.0.0", + "dependencies": { + "@clerk/nextjs": "catalog:", + "next": "^14.0.0", + "react": "^18.0.0" + } +} diff --git a/packages/upgrade/src/__tests__/fixtures/nextjs-catalog-resolved/pnpm-lock.yaml b/packages/upgrade/src/__tests__/fixtures/nextjs-catalog-resolved/pnpm-lock.yaml new file mode 100644 index 00000000000..085ee1031f5 --- /dev/null +++ b/packages/upgrade/src/__tests__/fixtures/nextjs-catalog-resolved/pnpm-lock.yaml @@ -0,0 +1,4 @@ +lockfileVersion: '9.0' +settings: + autoInstallPeers: true + excludeLinksFromLockfile: false diff --git a/packages/upgrade/src/__tests__/fixtures/nextjs-catalog-resolved/pnpm-workspace.yaml b/packages/upgrade/src/__tests__/fixtures/nextjs-catalog-resolved/pnpm-workspace.yaml new file mode 100644 index 00000000000..50b5f10c5b7 --- /dev/null +++ b/packages/upgrade/src/__tests__/fixtures/nextjs-catalog-resolved/pnpm-workspace.yaml @@ -0,0 +1,7 @@ +packages: + - 'packages/*' + +catalog: + '@clerk/nextjs': ^6.0.0 + next: ^14.0.0 + react: ^18.0.0 diff --git a/packages/upgrade/src/__tests__/fixtures/nextjs-catalog-resolved/src/app.tsx b/packages/upgrade/src/__tests__/fixtures/nextjs-catalog-resolved/src/app.tsx new file mode 100644 index 00000000000..19d46c9c6b0 --- /dev/null +++ b/packages/upgrade/src/__tests__/fixtures/nextjs-catalog-resolved/src/app.tsx @@ -0,0 +1,9 @@ +import { ClerkProvider } from '@clerk/nextjs'; + +export default function App() { + return ( + +
Hello
+ + ); +} diff --git a/packages/upgrade/src/__tests__/integration/cli.test.js b/packages/upgrade/src/__tests__/integration/cli.test.js index c2ffb8cd8d7..c08c90197e1 100644 --- a/packages/upgrade/src/__tests__/integration/cli.test.js +++ b/packages/upgrade/src/__tests__/integration/cli.test.js @@ -116,6 +116,15 @@ describe('CLI Integration', () => { expect(output).toContain('--sdk'); expect(result.exitCode).toBe(1); }); + + it('shows example command in non-interactive SDK detection error', async () => { + const dir = getFixturePath('no-clerk'); + const result = await runCli(['--dir', dir, '--dry-run', '--skip-codemods'], { timeout: 5000 }); + + const output = result.stdout + result.stderr; + expect(output).toContain('Example:'); + expect(output).toContain('npx @clerk/upgrade --sdk='); + }); }); describe('--sdk flag', () => { diff --git a/packages/upgrade/src/__tests__/integration/detect-sdk.test.js b/packages/upgrade/src/__tests__/integration/detect-sdk.test.js index 356606ef9ee..ff29e64e7b9 100644 --- a/packages/upgrade/src/__tests__/integration/detect-sdk.test.js +++ b/packages/upgrade/src/__tests__/integration/detect-sdk.test.js @@ -1,7 +1,11 @@ -import { describe, expect, it } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; -import { detectSdk, getMajorVersion, getSdkVersion, normalizeSdkName } from '../../util/detect-sdk.js'; -import { detectPackageManager } from '../../util/package-manager.js'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +import { detectSdk, getMajorVersion, getSdkVersion, normalizeSdkName, resolveCatalogVersion } from '../../util/detect-sdk.js'; +import { detectPackageManager, getInstallCommand, getUninstallCommand } from '../../util/package-manager.js'; import { getFixturePath } from '../helpers/create-fixture.js'; describe('detectSdk', () => { @@ -57,10 +61,15 @@ describe('getSdkVersion', () => { expect(version).toBeNull(); }); - it('returns null for catalog: protocol versions', () => { + it('returns null for catalog: protocol versions without pnpm-workspace.yaml', () => { const version = getSdkVersion('nextjs', getFixturePath('nextjs-catalog')); expect(version).toBeNull(); }); + + it('resolves catalog: protocol versions from pnpm-workspace.yaml', () => { + const version = getSdkVersion('nextjs', getFixturePath('nextjs-catalog-resolved')); + expect(version).toBe(6); + }); }); describe('getMajorVersion', () => { @@ -135,8 +144,120 @@ describe('detectPackageManager', () => { expect(pm).toBe('npm'); }); - it('defaults to npm when no lock file exists', () => { - const pm = detectPackageManager(getFixturePath('no-clerk')); - expect(pm).toBe('npm'); + it('defaults to npm when no lock file exists in any parent', () => { + // Create a temp dir outside the monorepo so no parent lockfile is found + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'clerk-pm-test-')); + try { + const pm = detectPackageManager(tmpDir); + expect(pm).toBe('npm'); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + describe('parent directory traversal', () => { + let tmpRoot; + let childDir; + + beforeEach(() => { + tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'clerk-pm-traversal-')); + childDir = path.join(tmpRoot, 'packages', 'web'); + fs.mkdirSync(childDir, { recursive: true }); + }); + + afterEach(() => { + fs.rmSync(tmpRoot, { recursive: true, force: true }); + }); + + it('finds pnpm-lock.yaml in parent directory', () => { + fs.writeFileSync(path.join(tmpRoot, 'pnpm-lock.yaml'), ''); + expect(detectPackageManager(childDir)).toBe('pnpm'); + }); + + it('finds yarn.lock in parent directory', () => { + fs.writeFileSync(path.join(tmpRoot, 'yarn.lock'), ''); + expect(detectPackageManager(childDir)).toBe('yarn'); + }); + + it('finds packageManager field in parent package.json', () => { + fs.writeFileSync( + path.join(tmpRoot, 'package.json'), + JSON.stringify({ packageManager: 'pnpm@9.0.0' }), + ); + expect(detectPackageManager(childDir)).toBe('pnpm'); + }); + + it('prefers lockfile over packageManager field at same level', () => { + fs.writeFileSync(path.join(tmpRoot, 'yarn.lock'), ''); + fs.writeFileSync( + path.join(tmpRoot, 'package.json'), + JSON.stringify({ packageManager: 'pnpm@9.0.0' }), + ); + expect(detectPackageManager(childDir)).toBe('yarn'); + }); + }); +}); + +describe('resolveCatalogVersion', () => { + it('resolves version from pnpm-workspace.yaml in same directory', () => { + const version = resolveCatalogVersion('@clerk/nextjs', getFixturePath('nextjs-catalog-resolved')); + expect(version).toBe('^6.0.0'); + }); + + it('returns null when pnpm-workspace.yaml does not exist', () => { + const version = resolveCatalogVersion('@clerk/nextjs', getFixturePath('nextjs-catalog')); + expect(version).toBeNull(); + }); + + it('returns null when package is not in catalog', () => { + const version = resolveCatalogVersion('@clerk/unknown-pkg', getFixturePath('nextjs-catalog-resolved')); + expect(version).toBeNull(); + }); + + it('traverses parent directories to find pnpm-workspace.yaml', () => { + // The nextjs-catalog-resolved fixture has pnpm-workspace.yaml at root + // Searching from its src/ subdirectory should still find it + const srcDir = path.join(getFixturePath('nextjs-catalog-resolved'), 'src'); + const version = resolveCatalogVersion('@clerk/nextjs', srcDir); + expect(version).toBe('^6.0.0'); + }); +}); + +describe('getInstallCommand', () => { + it('adds -w flag for pnpm at workspace root', () => { + const dir = getFixturePath('nextjs-catalog-resolved'); + const [cmd, args] = getInstallCommand('pnpm', '@clerk/nextjs', '7.0.0', dir); + expect(cmd).toBe('pnpm'); + expect(args).toContain('-w'); + }); + + it('does not add -w flag for pnpm without pnpm-workspace.yaml', () => { + const dir = getFixturePath('nextjs-v6'); + const [cmd, args] = getInstallCommand('pnpm', '@clerk/nextjs', '7.0.0', dir); + expect(cmd).toBe('pnpm'); + expect(args).not.toContain('-w'); + }); + + it('does not add -w flag for non-pnpm managers', () => { + const dir = getFixturePath('nextjs-catalog-resolved'); + const [cmd, args] = getInstallCommand('npm', '@clerk/nextjs', '7.0.0', dir); + expect(cmd).toBe('npm'); + expect(args).not.toContain('-w'); + }); +}); + +describe('getUninstallCommand', () => { + it('adds -w flag for pnpm at workspace root', () => { + const dir = getFixturePath('nextjs-catalog-resolved'); + const [cmd, args] = getUninstallCommand('pnpm', '@clerk/nextjs', dir); + expect(cmd).toBe('pnpm'); + expect(args).toContain('-w'); + }); + + it('does not add -w flag for pnpm without pnpm-workspace.yaml', () => { + const dir = getFixturePath('nextjs-v6'); + const [cmd, args] = getUninstallCommand('pnpm', '@clerk/nextjs', dir); + expect(cmd).toBe('pnpm'); + expect(args).not.toContain('-w'); }); }); diff --git a/packages/upgrade/src/cli.js b/packages/upgrade/src/cli.js index dfcb003e130..f6418103e99 100644 --- a/packages/upgrade/src/cli.js +++ b/packages/upgrade/src/cli.js @@ -49,8 +49,12 @@ const cli = meow( $ npx @clerk/upgrade --canary $ npx @clerk/upgrade --dry-run - Non-interactive mode: + Non-interactive mode (CI): When running in CI or piped environments, --sdk is required if it cannot be auto-detected. + If your version cannot be resolved (e.g. catalog: protocol), also provide --release. + + Example: + $ npx @clerk/upgrade --sdk=nextjs --release=core-3 --dir=./packages/web `, { importMeta: import.meta, @@ -103,6 +107,8 @@ async function main() { .map(s => s.value) .join(', '), ); + renderText(''); + renderText('Example: npx @clerk/upgrade --sdk=nextjs --dir=./packages/web'); process.exit(1); } @@ -140,8 +146,10 @@ async function main() { renderNewline(); if (!isInteractive) { - renderError('Please provide --release flag in non-interactive mode.'); + renderError('Could not detect version. Please provide --release flag in non-interactive mode.'); renderText('Available releases: ' + availableReleases.join(', ')); + renderText(''); + renderText(`Example: npx @clerk/upgrade --sdk=${sdk} --release=${availableReleases[0]}`); process.exit(1); } diff --git a/packages/upgrade/src/codemods/index.js b/packages/upgrade/src/codemods/index.js index cf8c6c8a97a..152557a116e 100644 --- a/packages/upgrade/src/codemods/index.js +++ b/packages/upgrade/src/codemods/index.js @@ -55,6 +55,7 @@ export async function runCodemod(transform = 'transform-async-request', patterns ...options, dry: true, silent: true, + verbose: 0, }); let result = {}; @@ -64,6 +65,7 @@ export async function runCodemod(transform = 'transform-async-request', patterns ...options, dry: false, silent: true, + verbose: 0, }); } diff --git a/packages/upgrade/src/render.js b/packages/upgrade/src/render.js index 5847135a016..923f6f9f74f 100644 --- a/packages/upgrade/src/render.js +++ b/packages/upgrade/src/render.js @@ -192,7 +192,15 @@ export function createSpinner(label) { } export function renderCodemodResults(transform, result) { - console.log(` ${result.ok ?? 0} file(s) modified, ${chalk.red(` ${result.error ?? 0} errors`)}`); + const errorCount = result.error ?? 0; + const okCount = result.ok ?? 0; + + if (errorCount > 0) { + console.log(` ${okCount} file(s) modified, ${chalk.red(`${errorCount} error(s)`)}`); + console.log(chalk.gray(' Error details for failed files are printed above by jscodeshift.')); + } else { + console.log(` ${okCount} file(s) modified`); + } console.log(''); } diff --git a/packages/upgrade/src/runner.js b/packages/upgrade/src/runner.js index 43b3a5f8408..f5767bff574 100644 --- a/packages/upgrade/src/runner.js +++ b/packages/upgrade/src/runner.js @@ -49,7 +49,11 @@ export async function runCodemods(config, sdk, options) { try { const result = await runCodemod(transform, patterns, options); - spinner.success(`Codemod applied: ${chalk.dim(transform)}`); + if (result.error > 0) { + spinner.error(`Codemod applied with errors: ${chalk.dim(transform)}`); + } else { + spinner.success(`Codemod applied: ${chalk.dim(transform)}`); + } renderCodemodResults(transform, result); const codemodConfig = getCodemodConfig(transform); diff --git a/packages/upgrade/src/util/detect-sdk.js b/packages/upgrade/src/util/detect-sdk.js index 4f8384d0107..5b40291eca1 100644 --- a/packages/upgrade/src/util/detect-sdk.js +++ b/packages/upgrade/src/util/detect-sdk.js @@ -49,6 +49,33 @@ export function detectSdk(dir) { return null; } +export function resolveCatalogVersion(packageName, dir) { + let current = path.resolve(dir); + const root = path.parse(current).root; + + while (current !== root) { + const wsPath = path.join(current, 'pnpm-workspace.yaml'); + if (fs.existsSync(wsPath)) { + try { + const content = fs.readFileSync(wsPath, 'utf8'); + // Match both catalog.default and catalog. sections + // Format: `'packageName': version` or `packageName: version` under catalog(s) sections + const escapedName = packageName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + const pattern = new RegExp(`^\\s*['"]?${escapedName}['"]?\\s*:\\s*['"]?([^'"\\s#]+)['"]?`, 'm'); + const match = content.match(pattern); + if (match) { + return match[1]; + } + } catch { + /* continue */ + } + } + current = path.dirname(current); + } + + return null; +} + export function getSdkVersion(sdk, dir) { let pkg; try { @@ -70,6 +97,14 @@ export function getSdkVersion(sdk, dir) { return null; } + if (version.startsWith('catalog:')) { + const resolvedVersion = resolveCatalogVersion(pkgName, dir) || (oldPkgName && resolveCatalogVersion(oldPkgName, dir)); + if (resolvedVersion) { + return getMajorVersion(resolvedVersion); + } + return null; + } + return getMajorVersion(version); } diff --git a/packages/upgrade/src/util/package-manager.js b/packages/upgrade/src/util/package-manager.js index 266b24d7063..c0692179083 100644 --- a/packages/upgrade/src/util/package-manager.js +++ b/packages/upgrade/src/util/package-manager.js @@ -3,28 +3,47 @@ import fs from 'node:fs'; import path from 'node:path'; export function detectPackageManager(dir) { - if (fs.existsSync(path.join(dir, 'pnpm-lock.yaml'))) { - return 'pnpm'; - } - if (fs.existsSync(path.join(dir, 'yarn.lock'))) { - return 'yarn'; - } - if (fs.existsSync(path.join(dir, 'bun.lockb')) || fs.existsSync(path.join(dir, 'bun.lock'))) { - return 'bun'; - } - if (fs.existsSync(path.join(dir, 'package-lock.json'))) { - return 'npm'; + let current = path.resolve(dir); + const root = path.parse(current).root; + + while (current !== root) { + if (fs.existsSync(path.join(current, 'pnpm-lock.yaml'))) return 'pnpm'; + if (fs.existsSync(path.join(current, 'yarn.lock'))) return 'yarn'; + if (fs.existsSync(path.join(current, 'bun.lockb')) || fs.existsSync(path.join(current, 'bun.lock'))) return 'bun'; + if (fs.existsSync(path.join(current, 'package-lock.json'))) return 'npm'; + + try { + const pkgPath = path.join(current, 'package.json'); + if (fs.existsSync(pkgPath)) { + const pkg = JSON.parse(fs.readFileSync(pkgPath, 'utf8')); + if (pkg.packageManager) { + const pmName = pkg.packageManager.split('@')[0]; + if (['pnpm', 'yarn', 'bun', 'npm'].includes(pmName)) return pmName; + } + } + } catch { + /* continue */ + } + + current = path.dirname(current); } return 'npm'; } -export function getInstallCommand(packageManager, packageName, version = 'latest') { +export function isPnpmWorkspaceRoot(dir) { + return fs.existsSync(path.join(dir, 'pnpm-workspace.yaml')); +} + +export function getInstallCommand(packageManager, packageName, version = 'latest', cwd) { const pkg = version === 'latest' ? packageName : `${packageName}@${version}`; switch (packageManager) { - case 'pnpm': - return ['pnpm', ['add', pkg]]; + case 'pnpm': { + const args = ['add', pkg]; + if (cwd && isPnpmWorkspaceRoot(cwd)) args.push('-w'); + return ['pnpm', args]; + } case 'yarn': return ['yarn', ['add', pkg]]; case 'bun': @@ -35,10 +54,13 @@ export function getInstallCommand(packageManager, packageName, version = 'latest } } -export function getUninstallCommand(packageManager, packageName) { +export function getUninstallCommand(packageManager, packageName, cwd) { switch (packageManager) { - case 'pnpm': - return ['pnpm', ['remove', packageName]]; + case 'pnpm': { + const args = ['remove', packageName]; + if (cwd && isPnpmWorkspaceRoot(cwd)) args.push('-w'); + return ['pnpm', args]; + } case 'yarn': return ['yarn', ['remove', packageName]]; case 'bun': @@ -81,12 +103,12 @@ export async function runPackageManagerCommand(command, args, cwd) { } export async function upgradePackage(packageManager, packageName, version, cwd) { - const [cmd, args] = getInstallCommand(packageManager, packageName, version); + const [cmd, args] = getInstallCommand(packageManager, packageName, version, cwd); return runPackageManagerCommand(cmd, args, cwd); } export async function removePackage(packageManager, packageName, cwd) { - const [cmd, args] = getUninstallCommand(packageManager, packageName); + const [cmd, args] = getUninstallCommand(packageManager, packageName, cwd); return runPackageManagerCommand(cmd, args, cwd); }