diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..4498848 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,27 @@ +name: CI + +on: + pull_request: + push: + branches: + - main + +jobs: + test: + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + # 24 is the engines floor, matching the Capacitor 9 CLI. It also covers what + # `npm test` needs: `node --test` globs and unflagged TypeScript type stripping. + node: ['24'] + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version: ${{ matrix.node }} + # package-lock.json is gitignored, so `npm ci` is not available here. + - run: npm install + - run: npm run lint + - run: npm test + - run: npm run build diff --git a/README.md b/README.md index d64cea2..2b41fc5 100644 --- a/README.md +++ b/README.md @@ -6,4 +6,37 @@ Migrate a Capacitor 8 plugin to Capacitor 9. ``` npx @capacitor/plugin-migration-v8-to-v9@latest -``` \ No newline at end of file +``` + +## Blocked install scripts + +npm 12 blocks the install scripts of dependencies that are not listed in `allowScripts`, +and still exits successfully. When that happens the plugin files are migrated but the +dependency tree is incomplete, so the migration exits with an error naming the packages: + +``` +The plugin files were migrated, but npm blocked the install scripts of 1 package, so the migration is incomplete: + swiftlint@2.0.0 + +Review those packages, then approve and rebuild them: + npm install-scripts approve swiftlint + npm rebuild + +The migration itself does not need to be run again. +``` + +Review each package before approving it. Only packages that this migration made pending +are reported. A package you had already reviewed before the migration ran — whether you +approved it or left it blocked — is left alone, even if the migration moved it to a new +version. + +## Failed installs + +The migration runs `npm install` after rewriting your files. If that fails, the plugin +files are still migrated but the dependency tree is incomplete, so the migration exits +non-zero and prints npm's own error. + +Plugins that declare another package manager are exempt: a `packageManager` field naming +pnpm, yarn or bun, or an `only-allow` preinstall guard, makes `npm install` fail by +design. In that case the migration warns, finishes, and exits successfully — install the +dependencies with your own package manager afterwards. diff --git a/package.json b/package.json index 88bc8c5..71f7e67 100644 --- a/package.json +++ b/package.json @@ -10,9 +10,19 @@ "prettier": "prettier \"**/*.{css,html,js,mjs,ts,json}\"", "build": "npm run clean && tsc", "clean": "rimraf ./dist", - "test": "echo \"Error: no test specified\" && exit 1", + "test": "node --test \"src/**/*.test.ts\"", "prepublishOnly": "npm run build" }, + "engines": { + "node": ">=24.0.0" + }, + "devEngines": { + "runtime": { + "name": "node", + "version": ">=24.0.0", + "onFail": "error" + } + }, "repository": { "type": "git", "url": "git+https://github.com/ionic-team/migrate-capacitor-plugin.git" diff --git a/src/index.ts b/src/index.ts index 2152471..247dbf5 100644 --- a/src/index.ts +++ b/src/index.ts @@ -12,7 +12,15 @@ import { join, resolve } from 'path'; import { rimraf } from 'rimraf'; import { coerce, gte, lt, valid } from 'semver'; +import { + diffBlockedKeys, + findInstallScriptKeys, + formatBlockedScriptsError, + npmSupportsInstallScriptsIn, +} from './install-scripts'; +import type { CommandRunner } from './install-scripts'; import { logger } from './log'; +import { declaresOtherPackageManager } from './package-manager'; import { runCommand } from './subprocess'; const coreVersion = '9.0.0-alpha.8'; @@ -276,18 +284,46 @@ export const run = async (): Promise => { } } + const runInDir: CommandRunner = (command, args) => runCommand(command, args, { cwd: dir }); + + // Take the baseline before anything is removed or reinstalled: `npm install-scripts ls` + // enumerates the tree on disk, so this is the last point at which it still reports what was + // already pending before this migration started. + const supportsInstallScripts = await npmSupportsInstallScriptsIn(runInDir); + const baselineBlocked = supportsInstallScripts ? await findInstallScriptKeys(runInDir) : null; + if (supportsInstallScripts && baselineBlocked === null) { + logger.warn('Could not read the blocked install scripts of this plugin, skipping the check'); + } + // start from a clean tree: npm treats what is already installed as the starting point, and an // installed eslint 8 with @ionic/eslint-config 0.4 blocks the move to eslint 10 and 0.5 rimraf.sync(join(dir, 'node_modules')); rimraf.sync(join(dir, 'package-lock.json')); + let installError: string | undefined; + let blockedPackages: string[] = []; try { await runCommand('npm', ['install'], { ...opts, cwd: dir, }); - } catch { + if (baselineBlocked !== null) { + const blockedAfterInstall = await findInstallScriptKeys(runInDir); + if (blockedAfterInstall === null) { + logger.warn('Could not read the blocked install scripts after installing, skipping the check'); + } else { + // Only report what this migration made pending. Packages that were already pending + // are the repo's own allowScripts policy, not something the migration broke. + blockedPackages = diffBlockedKeys(baselineBlocked, blockedAfterInstall); + } + } + } catch (e: any) { logger.warn('npm install failed, please, install the dependencies using your package manager of choice'); + // A repo that declares pnpm, yarn or bun fails `npm install` on purpose, so that is not + // evidence of an incomplete migration. + if (!declaresOtherPackageManager(pluginJSON)) { + installError = String(e); + } } if (pluginJSON.capacitor?.android?.src) { @@ -377,8 +413,6 @@ export const run = async (): Promise => { } } - logger.info('Plugin migrated to Capacitor 9!'); - if (eslintUpdated) { logger.info(''); logger.info('⚠️ Note: ESLint has been updated to v10 and @ionic/eslint-config to v0.5.0, which uses flat config.'); @@ -396,7 +430,18 @@ export const run = async (): Promise => { logger.info( 'We recommend running your formatting and linting scripts to ensure your codebase is formatted correctly.', ); + logger.info(''); + } + + if (installError) { + throw `The plugin files were migrated, but npm install failed, so the migration is incomplete.\nInstall the dependencies using your package manager of choice.\nThe migration itself does not need to be run again.\n\nnpm install said:\n${installError}`; } + + if (blockedPackages.length > 0) { + throw `The plugin files were migrated, but ${formatBlockedScriptsError(blockedPackages)}`; + } + + logger.info('Plugin migrated to Capacitor 9!'); }; // Rules in android/proguard-rules.pro only reach apps through consumerProguardFiles and the published files. diff --git a/src/install-scripts.test.ts b/src/install-scripts.test.ts new file mode 100644 index 0000000..22d2eff --- /dev/null +++ b/src/install-scripts.test.ts @@ -0,0 +1,249 @@ +import assert from 'node:assert/strict'; +import { describe, it } from 'node:test'; + +import { + diffBlockedKeys, + findInstallScriptKeys, + formatBlockedScriptsError, + npmSupportsInstallScripts, + npmSupportsInstallScriptsIn, + packageNameFromKey, + parseBlockedPackages, +} from './install-scripts.ts'; + +describe('npmSupportsInstallScripts', () => { + it('rejects npm 11 and older', () => { + assert.equal(npmSupportsInstallScripts('11.16.0'), false); + assert.equal(npmSupportsInstallScripts('10.9.9'), false); + }); + + it('accepts npm 12 and newer', () => { + assert.equal(npmSupportsInstallScripts('12.0.0'), true); + assert.equal(npmSupportsInstallScripts('12.2.0'), true); + assert.equal(npmSupportsInstallScripts('13.0.0'), true); + }); + + it('accepts a npm 12 prerelease', () => { + assert.equal(npmSupportsInstallScripts('12.0.0-pre.3'), true); + }); + + it('ignores the trailing newline npm --version writes', () => { + assert.equal(npmSupportsInstallScripts('12.2.0\n'), true); + }); + + it('rejects unparseable versions', () => { + assert.equal(npmSupportsInstallScripts(''), false); + assert.equal(npmSupportsInstallScripts('not a version'), false); + }); +}); + +describe('parseBlockedPackages', () => { + it('returns the pending keys', () => { + const stdout = JSON.stringify({ + allowScripts: [ + { name: 'esbuild', changes: [{ key: 'esbuild@0.28.0', change: 'pending' }] }, + { name: 'swiftlint', changes: [{ key: 'swiftlint@2.0.0', change: 'pending' }] }, + ], + }); + + assert.deepEqual(parseBlockedPackages(stdout), ['esbuild@0.28.0', 'swiftlint@2.0.0']); + }); + + it('returns every pending version of the same package', () => { + const stdout = JSON.stringify({ + allowScripts: [ + { + name: 'esbuild', + changes: [ + { key: 'esbuild@0.28.0', change: 'pending' }, + { key: 'esbuild@0.27.7', change: 'pending' }, + ], + }, + ], + }); + + assert.deepEqual(parseBlockedPackages(stdout), ['esbuild@0.28.0', 'esbuild@0.27.7']); + }); + + it('ignores changes that are not pending', () => { + const stdout = JSON.stringify({ + allowScripts: [ + { + name: 'swiftlint', + changes: [ + { key: 'swiftlint@1.0.2', change: 'approved' }, + { key: 'swiftlint@2.0.0', change: 'pending' }, + ], + }, + ], + }); + + assert.deepEqual(parseBlockedPackages(stdout), ['swiftlint@2.0.0']); + }); + + it('returns an empty list when no package is blocked', () => { + assert.deepEqual(parseBlockedPackages(JSON.stringify({ allowScripts: [] })), []); + assert.deepEqual(parseBlockedPackages(JSON.stringify({ allowScripts: [{ name: 'swiftlint' }] })), []); + }); + + it('returns null for unexpected or malformed output', () => { + assert.equal(parseBlockedPackages('not json'), null); + assert.equal(parseBlockedPackages(''), null); + assert.equal(parseBlockedPackages(JSON.stringify({})), null); + assert.equal(parseBlockedPackages(JSON.stringify({ allowScripts: 'nope' })), null); + }); +}); + +describe('findInstallScriptKeys', () => { + it('asks npm for the pending list as JSON', async () => { + const calls: { command: string; args: readonly string[] }[] = []; + const run = async (command: string, args: readonly string[]) => { + calls.push({ command, args }); + return JSON.stringify({ + allowScripts: [{ name: 'swiftlint', changes: [{ key: 'swiftlint@2.0.0', change: 'pending' }] }], + }); + }; + + assert.deepEqual(await findInstallScriptKeys(run), ['swiftlint@2.0.0']); + assert.deepEqual(calls, [{ command: 'npm', args: ['install-scripts', 'ls', '--json'] }]); + }); + + it('returns null when the command fails', async () => { + const run = async () => { + throw 'Unknown command: "install-scripts"'; + }; + + assert.equal(await findInstallScriptKeys(run), null); + }); + + it('returns null when the output cannot be parsed', async () => { + assert.equal(await findInstallScriptKeys(async () => 'not json'), null); + }); + + it('distinguishes a determined empty list from a failure', async () => { + assert.deepEqual(await findInstallScriptKeys(async () => JSON.stringify({ allowScripts: [] })), []); + }); +}); + +describe('npmSupportsInstallScriptsIn', () => { + it('asks npm for its version', async () => { + const calls: { command: string; args: readonly string[] }[] = []; + const run = async (command: string, args: readonly string[]) => { + calls.push({ command, args }); + return '12.2.0\n'; + }; + + assert.equal(await npmSupportsInstallScriptsIn(run), true); + assert.deepEqual(calls, [{ command: 'npm', args: ['--version'] }]); + }); + + it('is false on npm 11', async () => { + assert.equal(await npmSupportsInstallScriptsIn(async () => '11.21.0'), false); + }); + + it('is false when the command fails', async () => { + assert.equal( + await npmSupportsInstallScriptsIn(async () => { + throw 'npm not found'; + }), + false, + ); + }); +}); + +// The scenarios below mirror runs against real Capacitor 8 plugins on npm 12.2.0. The +// migration does not touch esbuild, and node_modules survives it, so an esbuild the author +// left pending comes back at the same version. swiftlint is the package the migration itself +// moves, from ^1.0.0 to ^2.0.0. +describe('diffBlockedKeys', () => { + it('ignores a package the migration does not touch', () => { + const before = ['esbuild@0.28.0']; + const after = ['esbuild@0.28.0']; + + assert.deepEqual(diffBlockedKeys(before, after), []); + }); + + it('reports a package the migration newly blocked', () => { + // swiftlint@1.0.2 was approved, so it is absent from the baseline. The bump to 2.0.0 is + // a version the author never reviewed. + const before = ['esbuild@0.28.0']; + const after = ['esbuild@0.28.0', 'swiftlint@2.0.0']; + + assert.deepEqual(diffBlockedKeys(before, after), ['swiftlint@2.0.0']); + }); + + it('ignores a package the author had already left blocked, bumped to a new version', () => { + assert.deepEqual(diffBlockedKeys(['swiftlint@1.0.2'], ['swiftlint@2.0.0']), []); + }); + + it('reports everything when nothing was pending beforehand', () => { + // A fresh clone has no node_modules, so npm reports an empty pending set as the baseline. + assert.deepEqual(diffBlockedKeys([], ['esbuild@0.28.0', 'swiftlint@2.0.0']), ['esbuild@0.28.0', 'swiftlint@2.0.0']); + }); + + it('matches a scoped package by name', () => { + const before = ['@swc/core@1.16.17']; + const after = ['@swc/core@1.16.18', 'swiftlint@2.0.0']; + + assert.deepEqual(diffBlockedKeys(before, after), ['swiftlint@2.0.0']); + }); + + it('ignores a package that stopped being pending', () => { + assert.deepEqual(diffBlockedKeys(['swiftlint@1.0.2', 'esbuild@0.28.0'], ['esbuild@0.28.0']), []); + }); +}); + +describe('packageNameFromKey', () => { + it('drops the version', () => { + assert.equal(packageNameFromKey('swiftlint@2.0.0'), 'swiftlint'); + }); + + it('keeps the scope of a scoped package', () => { + assert.equal(packageNameFromKey('@swc/core@1.16.18'), '@swc/core'); + }); + + it('passes through a key with no version', () => { + assert.equal(packageNameFromKey('swiftlint'), 'swiftlint'); + assert.equal(packageNameFromKey('@swc/core'), '@swc/core'); + }); +}); + +describe('formatBlockedScriptsError', () => { + it('names the packages and gives the recovery commands', () => { + const message = formatBlockedScriptsError(['esbuild@0.28.0', 'swiftlint@2.0.0']); + + assert.equal( + message, + [ + 'npm blocked the install scripts of 2 packages, so the migration is incomplete:', + ' esbuild@0.28.0', + ' swiftlint@2.0.0', + '', + 'Review those packages, then approve and rebuild them:', + ' npm install-scripts approve esbuild swiftlint', + ' npm rebuild', + '', + 'The migration itself does not need to be run again.', + ].join('\n'), + ); + }); + + it('uses the singular for one package', () => { + const message = formatBlockedScriptsError(['swiftlint@2.0.0']); + + assert.match(message, /^npm blocked the install scripts of 1 package, so/); + }); + + it('counts packages, not versions, when several versions are blocked', () => { + const message = formatBlockedScriptsError(['esbuild@0.28.0', 'esbuild@0.27.7']); + + assert.match(message, /^npm blocked the install scripts of 1 package, so/); + assert.match(message, /^ {2}esbuild@0\.28\.0$/m); + assert.match(message, /^ {2}esbuild@0\.27\.7$/m); + assert.match(message, /^ {2}npm install-scripts approve esbuild$/m); + }); + + it('tells the user the migration does not need to be re-run', () => { + assert.match(formatBlockedScriptsError(['swiftlint@2.0.0']), /does not need to be run again\.$/); + }); +}); diff --git a/src/install-scripts.ts b/src/install-scripts.ts new file mode 100644 index 0000000..7c0abf0 --- /dev/null +++ b/src/install-scripts.ts @@ -0,0 +1,91 @@ +import { major, valid } from 'semver'; + +export type CommandRunner = (command: string, args: readonly string[]) => Promise; + +export function npmSupportsInstallScripts(version: string): boolean { + const cleanVersion = valid(version.trim()); + if (!cleanVersion) { + return false; + } + return major(cleanVersion) >= 12; +} + +export async function npmSupportsInstallScriptsIn(run: CommandRunner): Promise { + try { + return npmSupportsInstallScripts(await run('npm', ['--version'])); + } catch { + return false; + } +} + +// Returns null when the pending set could not be determined, which is deliberately +// distinct from an empty array: treating "unknown" as "nothing pending" would make the +// before/after diff collapse back into the absolute set. +export async function findInstallScriptKeys(run: CommandRunner): Promise { + try { + return parseBlockedPackages(await run('npm', ['install-scripts', 'ls', '--json'])); + } catch { + return null; + } +} + +// The JSON shape is undocumented, so anything we do not recognise is reported as null +// (unknown) rather than as an empty list. +export function parseBlockedPackages(stdout: string): string[] | null { + let parsed: any; + try { + parsed = JSON.parse(stdout); + } catch { + return null; + } + + if (!Array.isArray(parsed?.allowScripts)) { + return null; + } + + const blocked: string[] = []; + for (const entry of parsed.allowScripts) { + if (!Array.isArray(entry?.changes)) { + continue; + } + for (const change of entry.changes) { + if (change?.change === 'pending' && typeof change.key === 'string') { + blocked.push(change.key); + } + } + } + + return blocked; +} + +// Compared by package name, not by `name@version`. The migration rewrites dependency ranges +// — swiftlint, rollup, typescript, prettier and the @ionic/* configs all move — so a package +// that was already blocked before the migration ran comes back under a new version. Diffing +// exact keys would report that as newly blocked when nothing changed but the number. +// Approvals are pinned per version and approved packages are absent from the baseline, so a +// name can only match here when npm was already blocking that package beforehand. +export function diffBlockedKeys(before: string[], after: string[]): string[] { + const seen = new Set(before.map(packageNameFromKey)); + return after.filter((key) => !seen.has(packageNameFromKey(key))); +} + +export function packageNameFromKey(key: string): string { + const separator = key.lastIndexOf('@'); + return separator > 0 ? key.slice(0, separator) : key; +} + +export function formatBlockedScriptsError(blocked: string[]): string { + const names = [...new Set(blocked.map(packageNameFromKey))]; + const plural = names.length === 1 ? 'package' : 'packages'; + + return [ + `npm blocked the install scripts of ${names.length} ${plural}, so the migration is incomplete:`, + ...blocked.map((key) => ` ${key}`), + '', + 'Review those packages, then approve and rebuild them:', + ` npm install-scripts approve ${names.join(' ')}`, + ' npm rebuild', + '', + 'The migration itself does not need to be run again.', + ].join('\n'); +} diff --git a/src/package-manager.test.ts b/src/package-manager.test.ts new file mode 100644 index 0000000..0a6b621 --- /dev/null +++ b/src/package-manager.test.ts @@ -0,0 +1,47 @@ +import assert from 'node:assert/strict'; +import { describe, it } from 'node:test'; + +import { declaresOtherPackageManager } from './package-manager.ts'; + +describe('declaresOtherPackageManager', () => { + it('is false for a plain npm plugin', () => { + assert.equal(declaresOtherPackageManager({}), false); + assert.equal(declaresOtherPackageManager({ scripts: { build: 'tsc' } }), false); + }); + + it('detects a corepack packageManager field', () => { + assert.equal(declaresOtherPackageManager({ packageManager: 'pnpm@9.12.0' }), true); + assert.equal(declaresOtherPackageManager({ packageManager: 'yarn@4.5.0' }), true); + assert.equal(declaresOtherPackageManager({ packageManager: 'bun@1.1.30' }), true); + }); + + it('is false when the packageManager field names npm', () => { + assert.equal(declaresOtherPackageManager({ packageManager: 'npm@11.21.0' }), false); + assert.equal(declaresOtherPackageManager({ packageManager: ' npm@12.2.0 ' }), false); + }); + + it('detects an only-allow preinstall guard', () => { + assert.equal(declaresOtherPackageManager({ scripts: { preinstall: 'npx only-allow pnpm' } }), true); + assert.equal(declaresOtherPackageManager({ scripts: { preinstall: 'npx -y only-allow yarn' } }), true); + }); + + it('is false when only-allow allows npm', () => { + assert.equal(declaresOtherPackageManager({ scripts: { preinstall: 'npx only-allow npm' } }), false); + }); + + it('ignores an unrelated preinstall script', () => { + assert.equal(declaresOtherPackageManager({ scripts: { preinstall: 'node ./check-env.js' } }), false); + }); + + it('ignores a non-string or empty packageManager field', () => { + assert.equal(declaresOtherPackageManager({ packageManager: '' }), false); + assert.equal(declaresOtherPackageManager({ packageManager: 42 }), false); + }); + + it('prefers the packageManager field over the preinstall guard', () => { + assert.equal( + declaresOtherPackageManager({ packageManager: 'npm@12.2.0', scripts: { preinstall: 'npx only-allow pnpm' } }), + false, + ); + }); +}); diff --git a/src/package-manager.ts b/src/package-manager.ts new file mode 100644 index 0000000..cacd03f --- /dev/null +++ b/src/package-manager.ts @@ -0,0 +1,21 @@ +export interface PackageManagerFields { + packageManager?: unknown; + scripts?: Record; +} + +// A repo that declares pnpm, yarn or bun expects `npm install` to fail: corepack's +// `packageManager` field and an `only-allow` preinstall guard are both deliberate +// refusals, not a broken dependency tree. +export function declaresOtherPackageManager(pkg: PackageManagerFields): boolean { + const packageManager = pkg?.packageManager; + if (typeof packageManager === 'string' && packageManager.trim() !== '') { + return !packageManager.trim().startsWith('npm@'); + } + + const preinstall = pkg?.scripts?.['preinstall']; + if (typeof preinstall === 'string' && preinstall.includes('only-allow')) { + return !/only-allow\s+npm\b/.test(preinstall); + } + + return false; +}