Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gar/inline normalize bin #49

Merged
merged 3 commits into from
Jul 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 60 additions & 4 deletions lib/normalize.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,70 @@
const semver = require('semver')
const fs = require('fs/promises')
const { glob } = require('glob')
const normalizePackageBin = require('npm-normalize-package-bin')
const legacyFixer = require('normalize-package-data/lib/fixer.js')
const legacyMakeWarning = require('normalize-package-data/lib/make_warning.js')
const path = require('path')
const log = require('proc-log')
const git = require('@npmcli/git')
const hostedGitInfo = require('hosted-git-info')

// used to be npm-normalize-package-bin
function normalizePackageBin (pkg, changes) {
if (pkg.bin) {
if (typeof pkg.bin === 'string' && pkg.name) {
changes?.push('"bin" was converted to an object')
pkg.bin = { [pkg.name]: pkg.bin }
} else if (Array.isArray(pkg.bin)) {
changes?.push('"bin" was converted to an object')
pkg.bin = pkg.bin.reduce((acc, k) => {
acc[path.basename(k)] = k
return acc
}, {})
}
if (typeof pkg.bin === 'object') {
for (const binKey in pkg.bin) {
if (typeof pkg.bin[binKey] !== 'string') {
delete pkg.bin[binKey]
changes?.push(`removed invalid "bin[${binKey}]"`)
continue
}
const base = path.join('/', path.basename(binKey.replace(/\\|:/g, '/'))).slice(1)
if (!base) {
delete pkg.bin[binKey]
changes?.push(`removed invalid "bin[${binKey}]"`)
continue
}

const binTarget = path.join('/', pkg.bin[binKey].replace(/\\/g, '/'))
.replace(/\\/g, '/').slice(1)

if (!binTarget) {
delete pkg.bin[binKey]
changes?.push(`removed invalid "bin[${binKey}]"`)
continue
}

if (base !== binKey) {
delete pkg.bin[binKey]
changes?.push(`"bin[${binKey}]" was renamed to "bin[${base}]"`)
}
if (binTarget !== pkg.bin[binKey]) {
changes?.push(`"bin[${base}]" script name was cleaned`)
}
pkg.bin[base] = binTarget
}

if (Object.keys(pkg.bin).length === 0) {
changes?.push('empty "bin" was removed')
delete pkg.bin
}

return pkg
}
}
delete pkg.bin
}

function isCorrectlyEncodedName (spec) {
return !spec.match(/[/@\s+%:]/) &&
spec === encodeURIComponent(spec)
Expand Down Expand Up @@ -184,7 +240,7 @@ const normalize = async (pkg, { strict, steps, root, changes, allowLegacyCase })
if (typeof data.scripts[name] !== 'string') {
delete data.scripts[name]
changes?.push(`Invalid scripts."${name}" was removed`)
} else if (steps.includes('scriptpath')) {
} else if (steps.includes('scriptpath') && spre.test(data.scripts[name])) {
data.scripts[name] = data.scripts[name].replace(spre, '')
changes?.push(`scripts entry "${name}" was fixed to remove node_modules/.bin reference`)
}
Expand Down Expand Up @@ -257,7 +313,7 @@ const normalize = async (pkg, { strict, steps, root, changes, allowLegacyCase })
}

if (steps.includes('bin') || steps.includes('binDir') || steps.includes('binRefs')) {
normalizePackageBin(data)
normalizePackageBin(data, changes)
}

// expand "directories.bin"
Expand All @@ -272,7 +328,7 @@ const normalize = async (pkg, { strict, steps, root, changes, allowLegacyCase })
return acc
}, {})
// *sigh*
normalizePackageBin(data)
normalizePackageBin(data, changes)
}

// populate "gitHead" attribute
Expand Down
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
"hosted-git-info": "^6.1.1",
"json-parse-even-better-errors": "^3.0.0",
"normalize-package-data": "^5.0.0",
"npm-normalize-package-bin": "^3.0.1",
"proc-log": "^3.0.0",
"semver": "^7.5.3"
},
Expand Down
32 changes: 31 additions & 1 deletion tap-snapshots/test/fix.js.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,32 @@
* Make sure to inspect the output below. Do not ignore changes!
*/
'use strict'
exports[`test/fix.js TAP with changes binRefs array > must match snapshot 1`] = `
Array [
"\\"bin\\" was converted to an object",
]
`

exports[`test/fix.js TAP with changes binRefs empty bin name > must match snapshot 1`] = `
Array [
"removed invalid \\"bin[/]\\"",
"empty \\"bin\\" was removed",
]
`

exports[`test/fix.js TAP with changes binRefs no bin target > must match snapshot 1`] = `
Array [
"removed invalid \\"bin[test-package]\\"",
"empty \\"bin\\" was removed",
]
`

exports[`test/fix.js TAP with changes binRefs scoped name > must match snapshot 1`] = `
Array []
Array [
"\\"bin\\" was converted to an object",
"\\"bin[@npmcli/test-package]\\" was renamed to \\"bin[test-package]\\"",
"\\"bin[test-package]\\" script name was cleaned",
]
`

exports[`test/fix.js TAP with changes bundleDependencies null > must match snapshot 1`] = `
Expand Down Expand Up @@ -103,3 +127,9 @@ Array [
"Removed invalid \\"scripts\\"",
]
`

exports[`test/fix.js TAP with changes scriptpath strips node_modules/.bin > must match snapshot 1`] = `
Array [
"scripts entry \\"test\\" was fixed to remove node_modules/.bin reference",
]
`
3 changes: 3 additions & 0 deletions tap-snapshots/test/normalize.js.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ exports[`test/normalize.js TAP @npmcli/package-json - with changes cleanup bins
Array [
"Deleted incorrect \\"bundledDependencies\\"",
"Removed invalid \\"scripts\\"",
"\\"bin\\" was converted to an object",
]
`

Expand All @@ -99,6 +100,8 @@ exports[`test/normalize.js TAP @npmcli/package-json - with changes cleanup bins
Array [
"Deleted incorrect \\"bundledDependencies\\"",
"Removed invalid \\"scripts\\"",
"removed invalid \\"bin[y]\\"",
"removed invalid \\"bin[z]\\"",
]
`

Expand Down
28 changes: 28 additions & 0 deletions test/fix.js
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,13 @@ for (const [name, testFix] of Object.entries(testMethods)) {
const { content } = await testFix(t, testdir)
t.strictSame(content.scripts, {})
})
t.test('strips node_modules/.bin', async t => {
const testdir = {
'package.json': pkg({ scripts: { test: './node_modules/.bin/test-script' } }),
}
const { content } = await testFix(t, testdir)
t.strictSame(content.scripts, { test: 'test-script' })
})
})
t.test('bundleDependencies', async t => {
t.test('null', async t => {
Expand All @@ -208,6 +215,27 @@ for (const [name, testFix] of Object.entries(testMethods)) {
const { content } = await testFix(t, testdir)
t.strictSame(content.bin, { 'test-package': '@npmcil/test-package' })
})
t.test('array', async t => {
const testdir = {
'package.json': pkg({ bin: ['@npmcil/test-package'] }),
}
const { content } = await testFix(t, testdir)
t.strictSame(content.bin, { 'test-package': '@npmcil/test-package' })
})
t.test('no bin target', async t => {
const testdir = {
'package.json': pkg({ bin: { 'test-package': '/' } }),
}
const { content } = await testFix(t, testdir)
t.notHas(content, 'bin')
})
t.test('empty bin name', async t => {
const testdir = {
'package.json': pkg({ bin: { '/': 'test-slash' } }),
}
const { content } = await testFix(t, testdir)
t.notHas(content, 'bin')
})
})
t.test('fixDependencies', async t => {
t.test('string dependencies', async t => {
Expand Down