Skip to content

Commit

Permalink
fix: make prepare and normalize have feature parity with legacy p…
Browse files Browse the repository at this point in the history
…ackages

Due to npm/cli#6470, I added `read-package-json` and
`read-package-json-fast` to this repo as dev deps to run the new tests
from #31 against those packages. I found a few discrepancies which I
fixed in this PR.

Full list of changes are as follows:

**`normalize` / `read-package-json-fast`**
- convert `bundleDependencies: false` to `[]`

**`prepare` / `read-package-json`**
- ignore `directories.bin` if `bin` is present
- walk up filesystem to look for git directory
- log a warning for inaccessible bin entries but dont delete

These changes will be tracked in npm/statusboard#487 to see if they
should be made as breaking changes with npm 10.

These legacy tests can be removed once we have decided to make these
breaking changes.

Ref: npm/cli#6470
Fixes: #35
  • Loading branch information
lukekarrys committed Jun 6, 2023
1 parent adb0c17 commit 9e0859b
Show file tree
Hide file tree
Showing 5 changed files with 930 additions and 790 deletions.
1 change: 1 addition & 0 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ class PackageJson {
'_attributes',
'bundledDependencies',
'bundleDependencies',
'bundleDependenciesDeleteFalse',
'gypfile',
'serverjs',
'scriptpath',
Expand Down
46 changes: 34 additions & 12 deletions lib/normalize.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,27 @@ const { glob } = require('glob')
const normalizePackageBin = require('npm-normalize-package-bin')
const normalizePackageData = require('normalize-package-data')
const path = require('path')
const log = require('proc-log')
const git = require('@npmcli/git')

const normalize = async (pkg, { strict, steps }) => {
// Replace with https://github.com/npm/git/pull/135 once it lands
const gitFind = async ({ cwd, root }) => {
if (await git.is({ cwd })) {
return cwd
}
while (cwd !== path.dirname(cwd) && cwd !== root) {
cwd = path.dirname(cwd)
if (await git.is({ cwd })) {
return cwd
}
}
return null
}

const normalize = async (pkg, { strict, steps, root }) => {
const data = pkg.content
const scripts = data.scripts || {}
const pkgId = `${data.name ?? ''}@${data.version ?? ''}`

// remove attributes that start with "_"
if (steps.includes('_attributes')) {
Expand All @@ -20,7 +37,7 @@ const normalize = async (pkg, { strict, steps }) => {
// build the "_id" attribute
if (steps.includes('_id')) {
if (data.name && data.version) {
data._id = `${data.name}@${data.version}`
data._id = pkgId
}
}

Expand All @@ -34,7 +51,9 @@ const normalize = async (pkg, { strict, steps }) => {
// expand "bundleDependencies: true or translate from object"
if (steps.includes('bundleDependencies')) {
const bd = data.bundleDependencies
if (bd === true) {
if (bd === false && !steps.includes('bundleDependenciesDeleteFalse')) {
data.bundleDependencies = []
} else if (bd === true) {
data.bundleDependencies = Object.keys(data.dependencies || {})
} else if (bd && typeof bd === 'object') {
if (!Array.isArray(bd)) {
Expand Down Expand Up @@ -158,7 +177,7 @@ const normalize = async (pkg, { strict, steps }) => {
}

// expand "directories.bin"
if (steps.includes('binDir') && data.directories?.bin) {
if (steps.includes('binDir') && data.directories?.bin && !data.bin) {
const binsDir = path.resolve(pkg.path, path.join('.', path.join('/', data.directories.bin)))
const bins = await glob('**', { cwd: binsDir })
data.bin = bins.reduce((acc, binFile) => {
Expand All @@ -174,25 +193,28 @@ const normalize = async (pkg, { strict, steps }) => {

// populate "gitHead" attribute
if (steps.includes('gitHead') && !data.gitHead) {
const gitRoot = await gitFind({ cwd: pkg.path, root })
let head
try {
head = await fs.readFile(path.resolve(pkg.path, '.git/HEAD'), 'utf8')
} catch (err) {
if (gitRoot) {
try {
head = await fs.readFile(path.resolve(gitRoot, '.git/HEAD'), 'utf8')
} catch (err) {
// do nothing
}
}
let headData
if (head) {
if (head.startsWith('ref: ')) {
const headRef = head.replace(/^ref: /, '').trim()
const headFile = path.resolve(pkg.path, '.git', headRef)
const headFile = path.resolve(gitRoot, '.git', headRef)
try {
headData = await fs.readFile(headFile, 'utf8')
headData = headData.replace(/^ref: /, '').trim()
} catch (err) {
// do nothing
}
if (!headData) {
const packFile = path.resolve(pkg.path, '.git/packed-refs')
const packFile = path.resolve(gitRoot, '.git/packed-refs')
try {
let refs = await fs.readFile(packFile, 'utf8')
if (refs) {
Expand Down Expand Up @@ -271,11 +293,11 @@ const normalize = async (pkg, { strict, steps }) => {
// in normalize-package-data if it had access to the file path.
if (steps.includes('binRefs') && data.bin instanceof Object) {
for (const key in data.bin) {
const binPath = path.resolve(pkg.path, data.bin[key])
try {
await fs.access(binPath)
await fs.access(path.resolve(pkg.path, data.bin[key]))
} catch {
delete data.bin[key]
log.warn('package-json', pkgId, `No bin file found at ${data.bin[key]}`)
// XXX: should a future breaking change delete bin entries that cannot be accessed?
}
}
}
Expand Down
6 changes: 5 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,17 @@
"devDependencies": {
"@npmcli/eslint-config": "^4.0.0",
"@npmcli/template-oss": "4.15.1",
"read-package-json": "^6.0.4",
"read-package-json-fast": "^3.0.2",
"tap": "^16.0.1"
},
"dependencies": {
"@npmcli/git": "^4.0.4",
"glob": "^10.2.2",
"json-parse-even-better-errors": "^3.0.0",
"normalize-package-data": "^5.0.0",
"npm-normalize-package-bin": "^3.0.1"
"npm-normalize-package-bin": "^3.0.1",
"proc-log": "^3.0.0"
},
"repository": {
"type": "git",
Expand Down
Loading

0 comments on commit 9e0859b

Please sign in to comment.