From 195256872495b4fe8f5943e1ec8e8ff2fb433576 Mon Sep 17 00:00:00 2001 From: Joseph Pender Date: Mon, 12 Oct 2026 11:58:22 -0500 Subject: [PATCH 1/8] reporting any packages that were blocked during install --- src/index.ts | 37 ++++++++++++++++++++++++++-- src/install-scripts.ts | 55 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 2 deletions(-) create mode 100644 src/install-scripts.ts diff --git a/src/index.ts b/src/index.ts index c1c5b46..052a338 100644 --- a/src/index.ts +++ b/src/index.ts @@ -12,6 +12,7 @@ import { join, resolve } from 'path'; import { rimraf } from 'rimraf'; import { coerce, gte, lt, valid } from 'semver'; +import { formatBlockedScriptsError, npmSupportsInstallScripts, parseBlockedPackages } from './install-scripts'; import { logger } from './log'; import { runCommand } from './subprocess'; @@ -144,6 +145,11 @@ export const run = async (): Promise => { pluginJSON.devDependencies['@capacitor/docgen'] = docgenVersion; } + if (pluginJSON.scripts?.['prepare']) { + delete pluginJSON.scripts['prepare']; + logger.info('Removed prepare script from package.json.'); + } + if (pluginJSON.version?.startsWith('7.')) { pluginJSON.version = '8.0.0'; } @@ -185,12 +191,16 @@ export const run = async (): Promise => { rimraf.sync(join(dir, 'node_modules/@capacitor')); rimraf.sync(join(dir, 'package-lock.json')); + let installFailed = false; + let blockedPackages: string[] = []; try { await runCommand('npm', ['install'], { ...opts, cwd: dir, }); + blockedPackages = await findBlockedInstallScripts(dir); } catch (e: any) { + installFailed = true; logger.warn('npm install failed, please, install the dependencies using your package manager of choice'); } @@ -280,17 +290,40 @@ export const run = async (): Promise => { } } - logger.info('Plugin migrated to Capacitor 9!'); - if (prettierUpdatedFromV2) { logger.info(''); logger.info('⚠️ Note: Prettier has been updated from v2 to v3.'); logger.info( 'We recommend running your formatting and linting scripts to ensure your codebase is formatted correctly.', ); + logger.info(''); + } + + if (installFailed) { + throw 'The plugin files were migrated, but npm install failed, so the migration is incomplete.\nInstall the dependencies using your package manager of choice.'; + } + + if (blockedPackages.length > 0) { + throw `The plugin files were migrated, but ${formatBlockedScriptsError(blockedPackages)}`; } + + logger.info('Plugin migrated to Capacitor 9!'); }; +async function findBlockedInstallScripts(dir: string): Promise { + try { + const npmVersion = await runCommand('npm', ['--version'], { cwd: dir }); + if (!npmSupportsInstallScripts(npmVersion)) { + return []; + } + + const output = await runCommand('npm', ['install-scripts', 'ls', '--json'], { cwd: dir }); + return parseBlockedPackages(output); + } catch { + return []; + } +} + async function updateGradleProperties(filename: string): Promise { const txt = readFile(filename); if (!txt) { diff --git a/src/install-scripts.ts b/src/install-scripts.ts new file mode 100644 index 0000000..337f41c --- /dev/null +++ b/src/install-scripts.ts @@ -0,0 +1,55 @@ +import { major, valid } from 'semver'; + +export function npmSupportsInstallScripts(version: string): boolean { + const cleanVersion = valid(version.trim()); + if (!cleanVersion) { + return false; + } + return major(cleanVersion) >= 12; +} + +export function parseBlockedPackages(stdout: string): string[] { + let parsed: any; + try { + parsed = JSON.parse(stdout); + } catch { + return []; + } + + if (!Array.isArray(parsed?.allowScripts)) { + return []; + } + + 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; +} + +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 = blocked.length === 1 ? 'package' : 'packages'; + + return [ + `npm blocked the install scripts of ${blocked.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', + ].join('\n'); +} From d631d2b6a39913518297ba3f72e4a8f7442ff309 Mon Sep 17 00:00:00 2001 From: Joseph Pender Date: Mon, 12 Oct 2026 11:58:30 -0500 Subject: [PATCH 2/8] adding tests --- package.json | 5 +- src/install-scripts.test.ts | 138 ++++++++++++++++++++++++++++++++++++ 2 files changed, 142 insertions(+), 1 deletion(-) create mode 100644 src/install-scripts.test.ts diff --git a/package.json b/package.json index f8b7425..94faa37 100644 --- a/package.json +++ b/package.json @@ -10,9 +10,12 @@ "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" + }, "repository": { "type": "git", "url": "git+https://github.com/ionic-team/migrate-capacitor-plugin.git" diff --git a/src/install-scripts.test.ts b/src/install-scripts.test.ts new file mode 100644 index 0000000..2a13b44 --- /dev/null +++ b/src/install-scripts.test.ts @@ -0,0 +1,138 @@ +import assert from 'node:assert/strict'; +import { describe, it } from 'node:test'; + +import { + formatBlockedScriptsError, + npmSupportsInstallScripts, + 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: 'fsevents', changes: [{ key: 'fsevents@2.3.3', change: 'pending' }] }, + { name: 'swiftlint', changes: [{ key: 'swiftlint@2.0.0', change: 'pending' }] }, + ], + }); + + assert.deepEqual(parseBlockedPackages(stdout), ['fsevents@2.3.3', 'swiftlint@2.0.0']); + }); + + it('returns every pending version of the same package', () => { + const stdout = JSON.stringify({ + allowScripts: [ + { + name: 'esbuild', + changes: [ + { key: 'esbuild@0.25.12', change: 'pending' }, + { key: 'esbuild@0.21.5', change: 'pending' }, + ], + }, + ], + }); + + assert.deepEqual(parseBlockedPackages(stdout), ['esbuild@0.25.12', 'esbuild@0.21.5']); + }); + + it('ignores changes that are not pending', () => { + const stdout = JSON.stringify({ + allowScripts: [ + { + name: 'swiftlint', + changes: [ + { key: 'swiftlint@2.0.0', change: 'approved' }, + { key: 'swiftlint@2.1.0', change: 'pending' }, + ], + }, + ], + }); + + assert.deepEqual(parseBlockedPackages(stdout), ['swiftlint@2.1.0']); + }); + + it('returns nothing when no package is blocked', () => { + assert.deepEqual(parseBlockedPackages(JSON.stringify({ allowScripts: [] })), []); + }); + + it('returns nothing for unexpected or malformed output', () => { + assert.deepEqual(parseBlockedPackages('not json'), []); + assert.deepEqual(parseBlockedPackages(''), []); + assert.deepEqual(parseBlockedPackages(JSON.stringify({})), []); + assert.deepEqual(parseBlockedPackages(JSON.stringify({ allowScripts: 'nope' })), []); + assert.deepEqual(parseBlockedPackages(JSON.stringify({ allowScripts: [{ name: 'swiftlint' }] })), []); + }); +}); + +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('@capacitor/core@9.0.0-alpha.7'), '@capacitor/core'); + }); + + it('passes through a key with no version', () => { + assert.equal(packageNameFromKey('swiftlint'), 'swiftlint'); + assert.equal(packageNameFromKey('@capacitor/core'), '@capacitor/core'); + }); +}); + +describe('formatBlockedScriptsError', () => { + it('names the packages and gives the recovery commands', () => { + const message = formatBlockedScriptsError(['fsevents@2.3.3', 'swiftlint@2.0.0']); + + assert.equal( + message, + [ + 'npm blocked the install scripts of 2 packages, so the migration is incomplete:', + ' fsevents@2.3.3', + ' swiftlint@2.0.0', + '', + 'Review those packages, then approve and rebuild them:', + ' npm install-scripts approve fsevents swiftlint', + ' npm rebuild', + ].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('lists a package once in the approve command when several versions are blocked', () => { + const message = formatBlockedScriptsError(['esbuild@0.25.12', 'esbuild@0.21.5']); + + assert.match(message, /^ {2}npm install-scripts approve esbuild$/m); + }); +}); From 3ebba7fbf89b537e07ed04129bd2afadfb60befc Mon Sep 17 00:00:00 2001 From: Joseph Pender Date: Mon, 12 Oct 2026 12:16:15 -0500 Subject: [PATCH 3/8] document blocked install scripts in README --- README.md | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index d64cea2..70cf10d 100644 --- a/README.md +++ b/README.md @@ -6,4 +6,22 @@ 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 +``` + +Review each package before approving it. The migration itself does not need to be run +again. \ No newline at end of file From 8c574e6eae7ba0e154b2103fd5bdd4dd8fa460e1 Mon Sep 17 00:00:00 2001 From: Joseph Pender Date: Mon, 12 Oct 2026 13:19:05 -0500 Subject: [PATCH 4/8] keep the prepare script in package.json during migration --- src/index.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/index.ts b/src/index.ts index 052a338..2a9e04d 100644 --- a/src/index.ts +++ b/src/index.ts @@ -145,11 +145,6 @@ export const run = async (): Promise => { pluginJSON.devDependencies['@capacitor/docgen'] = docgenVersion; } - if (pluginJSON.scripts?.['prepare']) { - delete pluginJSON.scripts['prepare']; - logger.info('Removed prepare script from package.json.'); - } - if (pluginJSON.version?.startsWith('7.')) { pluginJSON.version = '8.0.0'; } From c2068cd6d187c6236bb88291134685525f91b680 Mon Sep 17 00:00:00 2001 From: Joseph Pender Date: Wed, 14 Oct 2026 11:20:34 -0500 Subject: [PATCH 5/8] only report install scripts this migration blocked --- .github/workflows/ci.yml | 27 +++++++++ README.md | 18 +++++- package.json | 9 ++- src/index.ts | 57 +++++++++++------- src/install-scripts.test.ts | 113 +++++++++++++++++++++++++++++++++--- src/install-scripts.ts | 40 +++++++++++-- src/package-manager.test.ts | 47 +++++++++++++++ src/package-manager.ts | 21 +++++++ 8 files changed, 296 insertions(+), 36 deletions(-) create mode 100644 .github/workflows/ci.yml create mode 100644 src/package-manager.test.ts create mode 100644 src/package-manager.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..ebeca8f --- /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: + # 22.18 is the devEngines floor: the oldest Node with both `node --test` + # globs and unflagged TypeScript type stripping. + node: ['22.18', '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 70cf10d..408de81 100644 --- a/README.md +++ b/README.md @@ -21,7 +21,21 @@ The plugin files were migrated, but npm blocked the install scripts of 1 package 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. The migration itself does not need to be run -again. \ No newline at end of file +Review each package before approving it. Only packages that this migration made pending +are reported — anything already covered by your own `allowScripts` policy before the +migration ran is left alone. + +## 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 94faa37..a15fa4c 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,14 @@ "prepublishOnly": "npm run build" }, "engines": { - "node": ">=24.0.0" + "node": "20 || >=22" + }, + "devEngines": { + "runtime": { + "name": "node", + "version": ">=22.18.0", + "onFail": "error" + } }, "repository": { "type": "git", diff --git a/src/index.ts b/src/index.ts index 2a9e04d..9cffc2e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -12,8 +12,15 @@ import { join, resolve } from 'path'; import { rimraf } from 'rimraf'; import { coerce, gte, lt, valid } from 'semver'; -import { formatBlockedScriptsError, npmSupportsInstallScripts, parseBlockedPackages } from './install-scripts'; +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.7'; @@ -183,20 +190,44 @@ export const run = async (): Promise => { writeFileSync(packageJson, packageJsonText, 'utf-8'); + const runInDir: CommandRunner = (command, args) => runCommand(command, args, { cwd: dir }); + + // Take the baseline before the lockfile and node_modules go away: `npm install-scripts ls` + // enumerates through arborist, so once they are gone it can no longer see 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'); + } + rimraf.sync(join(dir, 'node_modules/@capacitor')); rimraf.sync(join(dir, 'package-lock.json')); - let installFailed = false; + let installError: string | undefined; let blockedPackages: string[] = []; try { await runCommand('npm', ['install'], { ...opts, cwd: dir, }); - blockedPackages = await findBlockedInstallScripts(dir); + 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) { - installFailed = true; 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) { @@ -294,8 +325,8 @@ export const run = async (): Promise => { logger.info(''); } - if (installFailed) { - throw 'The plugin files were migrated, but npm install failed, so the migration is incomplete.\nInstall the dependencies using your package manager of choice.'; + 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) { @@ -305,20 +336,6 @@ export const run = async (): Promise => { logger.info('Plugin migrated to Capacitor 9!'); }; -async function findBlockedInstallScripts(dir: string): Promise { - try { - const npmVersion = await runCommand('npm', ['--version'], { cwd: dir }); - if (!npmSupportsInstallScripts(npmVersion)) { - return []; - } - - const output = await runCommand('npm', ['install-scripts', 'ls', '--json'], { cwd: dir }); - return parseBlockedPackages(output); - } catch { - return []; - } -} - async function updateGradleProperties(filename: string): Promise { const txt = readFile(filename); if (!txt) { diff --git a/src/install-scripts.test.ts b/src/install-scripts.test.ts index 2a13b44..d878f3d 100644 --- a/src/install-scripts.test.ts +++ b/src/install-scripts.test.ts @@ -2,8 +2,11 @@ import assert from 'node:assert/strict'; import { describe, it } from 'node:test'; import { + diffBlockedKeys, + findInstallScriptKeys, formatBlockedScriptsError, npmSupportsInstallScripts, + npmSupportsInstallScriptsIn, packageNameFromKey, parseBlockedPackages, } from './install-scripts.ts'; @@ -78,16 +81,101 @@ describe('parseBlockedPackages', () => { assert.deepEqual(parseBlockedPackages(stdout), ['swiftlint@2.1.0']); }); - it('returns nothing when no package is blocked', () => { + 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 nothing for unexpected or malformed output', () => { - assert.deepEqual(parseBlockedPackages('not json'), []); - assert.deepEqual(parseBlockedPackages(''), []); - assert.deepEqual(parseBlockedPackages(JSON.stringify({})), []); - assert.deepEqual(parseBlockedPackages(JSON.stringify({ allowScripts: 'nope' })), []); - 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, + ); + }); +}); + +describe('diffBlockedKeys', () => { + it('ignores a package that was already pending before the migration', () => { + const before = ['esbuild@0.25.12']; + const after = ['esbuild@0.25.12']; + + assert.deepEqual(diffBlockedKeys(before, after), []); + }); + + it('reports a package the migration itself installed', () => { + const before = ['esbuild@0.25.12']; + const after = ['esbuild@0.25.12', 'swiftlint@2.0.0']; + + assert.deepEqual(diffBlockedKeys(before, after), ['swiftlint@2.0.0']); + }); + + it('reports everything when nothing was pending beforehand', () => { + assert.deepEqual(diffBlockedKeys([], ['fsevents@2.3.3', 'swiftlint@2.0.0']), ['fsevents@2.3.3', 'swiftlint@2.0.0']); + }); + + it('reports a new version of a package that was already pending', () => { + assert.deepEqual(diffBlockedKeys(['swiftlint@1.9.0'], ['swiftlint@2.0.0']), ['swiftlint@2.0.0']); + }); + + it('ignores a package that stopped being pending', () => { + assert.deepEqual(diffBlockedKeys(['swiftlint@1.9.0', 'esbuild@0.25.12'], ['esbuild@0.25.12']), []); }); }); @@ -120,6 +208,8 @@ describe('formatBlockedScriptsError', () => { 'Review those packages, then approve and rebuild them:', ' npm install-scripts approve fsevents swiftlint', ' npm rebuild', + '', + 'The migration itself does not need to be run again.', ].join('\n'), ); }); @@ -130,9 +220,16 @@ describe('formatBlockedScriptsError', () => { assert.match(message, /^npm blocked the install scripts of 1 package, so/); }); - it('lists a package once in the approve command when several versions are blocked', () => { + it('counts packages, not versions, when several versions are blocked', () => { const message = formatBlockedScriptsError(['esbuild@0.25.12', 'esbuild@0.21.5']); + assert.match(message, /^npm blocked the install scripts of 1 package, so/); + assert.match(message, /^ {2}esbuild@0\.25\.12$/m); + assert.match(message, /^ {2}esbuild@0\.21\.5$/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 index 337f41c..3fa8ec3 100644 --- a/src/install-scripts.ts +++ b/src/install-scripts.ts @@ -1,5 +1,7 @@ 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) { @@ -8,16 +10,37 @@ export function npmSupportsInstallScripts(version: string): boolean { return major(cleanVersion) >= 12; } -export function parseBlockedPackages(stdout: string): string[] { +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 []; + return null; } if (!Array.isArray(parsed?.allowScripts)) { - return []; + return null; } const blocked: string[] = []; @@ -35,6 +58,11 @@ export function parseBlockedPackages(stdout: string): string[] { return blocked; } +export function diffBlockedKeys(before: string[], after: string[]): string[] { + const seen = new Set(before); + return after.filter((key) => !seen.has(key)); +} + export function packageNameFromKey(key: string): string { const separator = key.lastIndexOf('@'); return separator > 0 ? key.slice(0, separator) : key; @@ -42,14 +70,16 @@ export function packageNameFromKey(key: string): string { export function formatBlockedScriptsError(blocked: string[]): string { const names = [...new Set(blocked.map(packageNameFromKey))]; - const plural = blocked.length === 1 ? 'package' : 'packages'; + const plural = names.length === 1 ? 'package' : 'packages'; return [ - `npm blocked the install scripts of ${blocked.length} ${plural}, so the migration is incomplete:`, + `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; +} From d96469a81fcb0772607eb8488634367744f0636f Mon Sep 17 00:00:00 2001 From: Joseph Pender Date: Wed, 14 Oct 2026 12:02:46 -0500 Subject: [PATCH 6/8] compare blocked install scripts by package name --- .github/workflows/ci.yml | 6 ++-- README.md | 5 +-- package.json | 4 +-- src/index.ts | 6 ++-- src/install-scripts.test.ts | 66 ++++++++++++++++++++++--------------- src/install-scripts.ts | 9 +++-- 6 files changed, 58 insertions(+), 38 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ebeca8f..4498848 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,9 +12,9 @@ jobs: strategy: fail-fast: false matrix: - # 22.18 is the devEngines floor: the oldest Node with both `node --test` - # globs and unflagged TypeScript type stripping. - node: ['22.18', '24'] + # 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 diff --git a/README.md b/README.md index 408de81..2b41fc5 100644 --- a/README.md +++ b/README.md @@ -26,8 +26,9 @@ 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 — anything already covered by your own `allowScripts` policy before the -migration ran is left alone. +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 diff --git a/package.json b/package.json index a15fa4c..796eed8 100644 --- a/package.json +++ b/package.json @@ -14,12 +14,12 @@ "prepublishOnly": "npm run build" }, "engines": { - "node": "20 || >=22" + "node": ">=24.0.0" }, "devEngines": { "runtime": { "name": "node", - "version": ">=22.18.0", + "version": ">=24.0.0", "onFail": "error" } }, diff --git a/src/index.ts b/src/index.ts index 9cffc2e..e6ee99e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -192,9 +192,9 @@ export const run = async (): Promise => { const runInDir: CommandRunner = (command, args) => runCommand(command, args, { cwd: dir }); - // Take the baseline before the lockfile and node_modules go away: `npm install-scripts ls` - // enumerates through arborist, so once they are gone it can no longer see what was already - // pending before this migration started. + // 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) { diff --git a/src/install-scripts.test.ts b/src/install-scripts.test.ts index d878f3d..22d2eff 100644 --- a/src/install-scripts.test.ts +++ b/src/install-scripts.test.ts @@ -41,12 +41,12 @@ describe('parseBlockedPackages', () => { it('returns the pending keys', () => { const stdout = JSON.stringify({ allowScripts: [ - { name: 'fsevents', changes: [{ key: 'fsevents@2.3.3', change: 'pending' }] }, + { name: 'esbuild', changes: [{ key: 'esbuild@0.28.0', change: 'pending' }] }, { name: 'swiftlint', changes: [{ key: 'swiftlint@2.0.0', change: 'pending' }] }, ], }); - assert.deepEqual(parseBlockedPackages(stdout), ['fsevents@2.3.3', 'swiftlint@2.0.0']); + assert.deepEqual(parseBlockedPackages(stdout), ['esbuild@0.28.0', 'swiftlint@2.0.0']); }); it('returns every pending version of the same package', () => { @@ -55,14 +55,14 @@ describe('parseBlockedPackages', () => { { name: 'esbuild', changes: [ - { key: 'esbuild@0.25.12', change: 'pending' }, - { key: 'esbuild@0.21.5', change: 'pending' }, + { key: 'esbuild@0.28.0', change: 'pending' }, + { key: 'esbuild@0.27.7', change: 'pending' }, ], }, ], }); - assert.deepEqual(parseBlockedPackages(stdout), ['esbuild@0.25.12', 'esbuild@0.21.5']); + assert.deepEqual(parseBlockedPackages(stdout), ['esbuild@0.28.0', 'esbuild@0.27.7']); }); it('ignores changes that are not pending', () => { @@ -71,14 +71,14 @@ describe('parseBlockedPackages', () => { { name: 'swiftlint', changes: [ - { key: 'swiftlint@2.0.0', change: 'approved' }, - { key: 'swiftlint@2.1.0', change: 'pending' }, + { key: 'swiftlint@1.0.2', change: 'approved' }, + { key: 'swiftlint@2.0.0', change: 'pending' }, ], }, ], }); - assert.deepEqual(parseBlockedPackages(stdout), ['swiftlint@2.1.0']); + assert.deepEqual(parseBlockedPackages(stdout), ['swiftlint@2.0.0']); }); it('returns an empty list when no package is blocked', () => { @@ -151,31 +151,45 @@ describe('npmSupportsInstallScriptsIn', () => { }); }); +// 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 that was already pending before the migration', () => { - const before = ['esbuild@0.25.12']; - const after = ['esbuild@0.25.12']; + 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 itself installed', () => { - const before = ['esbuild@0.25.12']; - const after = ['esbuild@0.25.12', 'swiftlint@2.0.0']; + 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', () => { - assert.deepEqual(diffBlockedKeys([], ['fsevents@2.3.3', 'swiftlint@2.0.0']), ['fsevents@2.3.3', 'swiftlint@2.0.0']); + // 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('reports a new version of a package that was already pending', () => { - assert.deepEqual(diffBlockedKeys(['swiftlint@1.9.0'], ['swiftlint@2.0.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.9.0', 'esbuild@0.25.12'], ['esbuild@0.25.12']), []); + assert.deepEqual(diffBlockedKeys(['swiftlint@1.0.2', 'esbuild@0.28.0'], ['esbuild@0.28.0']), []); }); }); @@ -185,28 +199,28 @@ describe('packageNameFromKey', () => { }); it('keeps the scope of a scoped package', () => { - assert.equal(packageNameFromKey('@capacitor/core@9.0.0-alpha.7'), '@capacitor/core'); + 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('@capacitor/core'), '@capacitor/core'); + assert.equal(packageNameFromKey('@swc/core'), '@swc/core'); }); }); describe('formatBlockedScriptsError', () => { it('names the packages and gives the recovery commands', () => { - const message = formatBlockedScriptsError(['fsevents@2.3.3', 'swiftlint@2.0.0']); + 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:', - ' fsevents@2.3.3', + ' esbuild@0.28.0', ' swiftlint@2.0.0', '', 'Review those packages, then approve and rebuild them:', - ' npm install-scripts approve fsevents swiftlint', + ' npm install-scripts approve esbuild swiftlint', ' npm rebuild', '', 'The migration itself does not need to be run again.', @@ -221,11 +235,11 @@ describe('formatBlockedScriptsError', () => { }); it('counts packages, not versions, when several versions are blocked', () => { - const message = formatBlockedScriptsError(['esbuild@0.25.12', 'esbuild@0.21.5']); + 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\.25\.12$/m); - assert.match(message, /^ {2}esbuild@0\.21\.5$/m); + 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); }); diff --git a/src/install-scripts.ts b/src/install-scripts.ts index 3fa8ec3..c93cc7c 100644 --- a/src/install-scripts.ts +++ b/src/install-scripts.ts @@ -58,9 +58,14 @@ export function parseBlockedPackages(stdout: string): string[] | null { 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 +// the author already reviewed and left pending comes back under a new version it was never +// asked about. Diffing exact keys would report that as newly blocked when nothing changed +// but the number. export function diffBlockedKeys(before: string[], after: string[]): string[] { - const seen = new Set(before); - return after.filter((key) => !seen.has(key)); + const seen = new Set(before.map(packageNameFromKey)); + return after.filter((key) => !seen.has(packageNameFromKey(key))); } export function packageNameFromKey(key: string): string { From 744841e88a2731f06df413cf4d64e6fbfa068513 Mon Sep 17 00:00:00 2001 From: Joey Pender Date: Mon, 26 Oct 2026 09:40:29 -0500 Subject: [PATCH 7/8] Update src/install-scripts.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: André Destro <254790+andredestro@users.noreply.github.com> --- src/install-scripts.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/install-scripts.ts b/src/install-scripts.ts index c93cc7c..b73f7ff 100644 --- a/src/install-scripts.ts +++ b/src/install-scripts.ts @@ -64,8 +64,8 @@ export function parseBlockedPackages(stdout: string): string[] | null { // asked about. Diffing exact keys would report that as newly blocked when nothing changed // but the number. export function diffBlockedKeys(before: string[], after: string[]): string[] { - const seen = new Set(before.map(packageNameFromKey)); - return after.filter((key) => !seen.has(packageNameFromKey(key))); + const seen = new Set(before); + return after.filter((key) => !seen.has(key)); } export function packageNameFromKey(key: string): string { From 8d313fcfcc74031ea74cd52ef68d62182f55e1b9 Mon Sep 17 00:00:00 2001 From: Joseph Pender Date: Mon, 26 Oct 2026 13:54:42 -0500 Subject: [PATCH 8/8] compare blocked packages by name instead of name@version --- src/install-scripts.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/install-scripts.ts b/src/install-scripts.ts index b73f7ff..7c0abf0 100644 --- a/src/install-scripts.ts +++ b/src/install-scripts.ts @@ -60,12 +60,13 @@ export function parseBlockedPackages(stdout: string): string[] | null { // 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 -// the author already reviewed and left pending comes back under a new version it was never -// asked about. Diffing exact keys would report that as newly blocked when nothing changed -// but the number. +// 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); - return after.filter((key) => !seen.has(key)); + const seen = new Set(before.map(packageNameFromKey)); + return after.filter((key) => !seen.has(packageNameFromKey(key))); } export function packageNameFromKey(key: string): string {