Skip to content

Commit

Permalink
fix: move to @npmcli/package-json where possible
Browse files Browse the repository at this point in the history
Some commands have tests that mock the library that parses package.json
and will need to be fixed before we can move them over
  • Loading branch information
wraithgar committed May 22, 2023
1 parent b21e11f commit 636e29e
Show file tree
Hide file tree
Showing 15 changed files with 58 additions and 66 deletions.
8 changes: 3 additions & 5 deletions lib/commands/access.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
const path = require('path')

const libnpmaccess = require('libnpmaccess')
const npa = require('npm-package-arg')
const readPackageJson = require('read-package-json-fast')
const pkgJson = require('@npmcli/package-json')
const localeCompare = require('@isaacs/string-locale-compare')('en')

const otplease = require('../utils/otplease.js')
Expand Down Expand Up @@ -178,8 +176,8 @@ class Access extends BaseCommand {
async #getPackage (name, requireScope) {
if (!name) {
try {
const pkg = await readPackageJson(path.resolve(this.npm.prefix, 'package.json'))
name = pkg.name
const { content } = await pkgJson.normalize(this.npm.prefix)
name = content.name
} catch (err) {
if (err.code === 'ENOENT') {
throw Object.assign(new Error('no package name given and no package.json found'), {
Expand Down
12 changes: 6 additions & 6 deletions lib/commands/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const { spawn } = require('child_process')
const { EOL } = require('os')
const ini = require('ini')
const localeCompare = require('@isaacs/string-locale-compare')('en')
const rpj = require('read-package-json-fast')
const pkgJson = require('@npmcli/package-json')
const log = require('../utils/log-shim.js')

// These are the configs that we can nerf-dart. Not all of them currently even
Expand Down Expand Up @@ -346,15 +346,15 @@ ${defData}
}

if (!this.npm.global) {
const pkgPath = resolve(this.npm.prefix, 'package.json')
const pkg = await rpj(pkgPath).catch(() => ({}))
const { content } = await pkgJson.normalize(this.npm.prefix).catch(() => ({ content: {} }))

if (pkg.publishConfig) {
if (content.publishConfig) {
const pkgPath = resolve(this.npm.prefix, 'package.json')
msg.push(`; "publishConfig" from ${pkgPath}`)
msg.push('; This set of config values will be used at publish-time.', '')
const pkgKeys = Object.keys(pkg.publishConfig).sort(localeCompare)
const pkgKeys = Object.keys(content.publishConfig).sort(localeCompare)
for (const k of pkgKeys) {
const v = publicVar(k) ? JSON.stringify(pkg.publishConfig[k]) : '(protected)'
const v = publicVar(k) ? JSON.stringify(content.publishConfig[k]) : '(protected)'
msg.push(`${k} = ${v}`)
}
msg.push('')
Expand Down
8 changes: 4 additions & 4 deletions lib/commands/diff.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const npa = require('npm-package-arg')
const pacote = require('pacote')
const pickManifest = require('npm-pick-manifest')
const log = require('../utils/log-shim')
const readPackage = require('read-package-json-fast')
const pkgJson = require('@npmcli/package-json')
const BaseCommand = require('../base-command.js')

class Diff extends BaseCommand {
Expand Down Expand Up @@ -81,7 +81,7 @@ class Diff extends BaseCommand {
async packageName (path) {
let name
try {
const pkg = await readPackage(resolve(this.prefix, 'package.json'))
const { content: pkg } = await pkgJson.normalize(this.prefix)
name = pkg.name
} catch (e) {
log.verbose('diff', 'could not read project dir package.json')
Expand Down Expand Up @@ -115,7 +115,7 @@ class Diff extends BaseCommand {
let noPackageJson
let pkgName
try {
const pkg = await readPackage(resolve(this.prefix, 'package.json'))
const { content: pkg } = await pkgJson.normalize(this.prefix)
pkgName = pkg.name
} catch (e) {
log.verbose('diff', 'could not read project dir package.json')
Expand Down Expand Up @@ -228,7 +228,7 @@ class Diff extends BaseCommand {
if (semverA && semverB) {
let pkgName
try {
const pkg = await readPackage(resolve(this.prefix, 'package.json'))
const { content: pkg } = await pkgJson.normalize(this.prefix)
pkgName = pkg.name
} catch (e) {
log.verbose('diff', 'could not read project dir package.json')
Expand Down
5 changes: 2 additions & 3 deletions lib/commands/dist-tag.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
const npa = require('npm-package-arg')
const path = require('path')
const regFetch = require('npm-registry-fetch')
const semver = require('semver')
const log = require('../utils/log-shim')
const otplease = require('../utils/otplease.js')
const readPackage = require('read-package-json-fast')
const pkgJson = require('@npmcli/package-json')
const BaseCommand = require('../base-command.js')

class DistTag extends BaseCommand {
Expand Down Expand Up @@ -152,7 +151,7 @@ class DistTag extends BaseCommand {
if (this.npm.global) {
throw this.usageError()
}
const { name } = await readPackage(path.resolve(this.npm.prefix, 'package.json'))
const { content: { name } } = await pkgJson.normalize(this.npm.prefix)
if (!name) {
throw this.usageError()
}
Expand Down
5 changes: 2 additions & 3 deletions lib/commands/init.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ const { relative, resolve } = require('path')
const { mkdir } = require('fs/promises')
const initJson = require('init-package-json')
const npa = require('npm-package-arg')
const rpj = require('read-package-json-fast')
const libexec = require('libnpmexec')
const mapWorkspaces = require('@npmcli/map-workspaces')
const PackageJson = require('@npmcli/package-json')
Expand Down Expand Up @@ -54,7 +53,7 @@ class Init extends BaseCommand {
// reads package.json for the top-level folder first, by doing this we
// ensure the command throw if no package.json is found before trying
// to create a workspace package.json file or its folders
const pkg = await rpj(resolve(this.npm.localPrefix, 'package.json')).catch((err) => {
const { content: pkg } = await PackageJson.normalize(this.npm.localPrefix).catch(err => {
if (err.code === 'ENOENT') {
log.warn('Missing package.json. Try with `--include-workspace-root`.')
}
Expand Down Expand Up @@ -217,7 +216,7 @@ class Init extends BaseCommand {
// translate workspaces paths into an array containing workspaces names
const workspaces = []
for (const path of workspacesPaths) {
const { name } = await rpj(resolve(path, 'package.json')).catch(() => ({}))
const { content: { name } } = await PackageJson.normalize(path).catch(() => ({ content: {} }))

if (name) {
workspaces.push(name)
Expand Down
13 changes: 7 additions & 6 deletions lib/commands/link.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const readdir = util.promisify(fs.readdir)
const { resolve } = require('path')

const npa = require('npm-package-arg')
const rpj = require('read-package-json-fast')
const pkgJson = require('@npmcli/package-json')
const semver = require('semver')

const reifyFinish = require('../utils/reify-finish.js')
Expand Down Expand Up @@ -96,11 +96,12 @@ class Link extends ArboristWorkspaceCmd {
const names = []
for (const a of args) {
const arg = npa(a)
names.push(
arg.type === 'directory'
? (await rpj(resolve(arg.fetchSpec, 'package.json'))).name
: arg.name
)
if (arg.type === 'directory') {
const { content } = await pkgJson.normalize(arg.fetchSpec)
names.push(content.name)
} else {
names.push(arg.name)
}
}

// npm link should not save=true by default unless you're
Expand Down
13 changes: 6 additions & 7 deletions lib/commands/owner.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,13 @@ const npmFetch = require('npm-registry-fetch')
const pacote = require('pacote')
const log = require('../utils/log-shim')
const otplease = require('../utils/otplease.js')
const readPackageJsonFast = require('read-package-json-fast')
const pkgJson = require('@npmcli/package-json')
const BaseCommand = require('../base-command.js')
const { resolve } = require('path')

const readJson = async (pkg) => {
const readJson = async (path) => {
try {
const json = await readPackageJsonFast(pkg)
return json
const { content } = await pkgJson.normalize(path)
return content
} catch {
return {}
}
Expand Down Expand Up @@ -54,7 +53,7 @@ class Owner extends BaseCommand {
if (this.npm.global) {
return []
}
const { name } = await readJson(resolve(this.npm.prefix, 'package.json'))
const { name } = await readJson(this.npm.prefix)
if (!name) {
return []
}
Expand Down Expand Up @@ -130,7 +129,7 @@ class Owner extends BaseCommand {
if (this.npm.global) {
throw this.usageError()
}
const { name } = await readJson(resolve(prefix, 'package.json'))
const { name } = await readJson(prefix)
if (!name) {
throw this.usageError()
}
Expand Down
11 changes: 4 additions & 7 deletions lib/commands/publish.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
const util = require('util')
const log = require('../utils/log-shim.js')
const semver = require('semver')
const pack = require('libnpmpack')
Expand All @@ -17,11 +16,7 @@ const { getContents, logTar } = require('../utils/tar.js')
// revisit this at some point, and have a minimal set that's a SemVer-major
// change that ought to get a RFC written on it.
const { flatten } = require('../utils/config/index.js')

// this is the only case in the CLI where we want to use the old full slow
// 'read-package-json' module, because we want to pull in all the defaults and
// metadata, like git sha's and default scripts and all that.
const readJson = util.promisify(require('read-package-json'))
const pkgJson = require('@npmcli/package-json')

const BaseCommand = require('../base-command.js')
class Publish extends BaseCommand {
Expand Down Expand Up @@ -204,7 +199,9 @@ class Publish extends BaseCommand {
async getManifest (spec, opts) {
let manifest
if (spec.type === 'directory') {
manifest = await readJson(`${spec.fetchSpec}/package.json`)
// Prepare is the special function for publishing, different than normalize
const { content } = await pkgJson.prepare(spec.fetchSpec)
manifest = content
} else {
manifest = await pacote.manifest(spec, {
...opts,
Expand Down
23 changes: 12 additions & 11 deletions lib/commands/run-script.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
const { resolve } = require('path')
const runScript = require('@npmcli/run-script')
const { isServerPackage } = runScript
const rpj = require('read-package-json-fast')
const pkgJson = require('@npmcli/package-json')
const log = require('../utils/log-shim.js')
const didYouMean = require('../utils/did-you-mean.js')
const { isWindowsShell } = require('../utils/is-windows.js')
Expand Down Expand Up @@ -39,9 +38,8 @@ class RunScript extends BaseCommand {
async completion (opts) {
const argv = opts.conf.argv.remain
if (argv.length === 2) {
// find the script name
const json = resolve(this.npm.localPrefix, 'package.json')
const { scripts = {} } = await rpj(json).catch(er => ({}))
const { content: { scripts = {} } } = await pkgJson.normalize(this.npm.localPrefix)
.catch(er => ({ content: {} }))
if (opts.isFish) {
return Object.keys(scripts).map(s => `${s}\t${scripts[s].slice(0, 30)}`)
}
Expand Down Expand Up @@ -70,7 +68,10 @@ class RunScript extends BaseCommand {
// null value
const scriptShell = this.npm.config.get('script-shell') || undefined

pkg = pkg || (await rpj(`${path}/package.json`))
if (!pkg) {
const { content } = await pkgJson.normalize(path)
pkg = content
}
const { scripts = {} } = pkg

if (event === 'restart' && !scripts.restart) {
Expand Down Expand Up @@ -126,8 +127,8 @@ class RunScript extends BaseCommand {
}

async list (args, path) {
path = path || this.npm.localPrefix
const { scripts, name, _id } = await rpj(`${path}/package.json`)
/* eslint-disable-next-line max-len */
const { content: { scripts, name, _id } } = await pkgJson.normalize(path || this.npm.localPrefix)
const pkgid = _id || name

if (!scripts) {
Expand Down Expand Up @@ -197,7 +198,7 @@ class RunScript extends BaseCommand {
await this.setWorkspaces()

for (const workspacePath of this.workspacePaths) {
const pkg = await rpj(`${workspacePath}/package.json`)
const { content: pkg } = await pkgJson.normalize(workspacePath)
const runResult = await this.run(args, {
path: workspacePath,
pkg,
Expand Down Expand Up @@ -236,7 +237,7 @@ class RunScript extends BaseCommand {
if (this.npm.config.get('json')) {
const res = {}
for (const workspacePath of this.workspacePaths) {
const { scripts, name } = await rpj(`${workspacePath}/package.json`)
const { content: { scripts, name } } = await pkgJson.normalize(workspacePath)
res[name] = { ...scripts }
}
this.npm.output(JSON.stringify(res, null, 2))
Expand All @@ -245,7 +246,7 @@ class RunScript extends BaseCommand {

if (this.npm.config.get('parseable')) {
for (const workspacePath of this.workspacePaths) {
const { scripts, name } = await rpj(`${workspacePath}/package.json`)
const { content: { scripts, name } } = await pkgJson.normalize(workspacePath)
for (const [script, cmd] of Object.entries(scripts || {})) {
this.npm.output(`${name}:${script}:${cmd}`)
}
Expand Down
4 changes: 2 additions & 2 deletions lib/commands/uninstall.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const { resolve } = require('path')
const rpj = require('read-package-json-fast')
const pkgJson = require('@npmcli/package-json')

const reifyFinish = require('../utils/reify-finish.js')
const completion = require('../utils/completion/installed-shallow.js')
Expand All @@ -24,7 +24,7 @@ class Uninstall extends ArboristWorkspaceCmd {
throw new Error('Must provide a package name to remove')
} else {
try {
const pkg = await rpj(resolve(this.npm.localPrefix, 'package.json'))
const { content: pkg } = await pkgJson.normalize(this.npm.localPrefix)
args.push(pkg.name)
} catch (er) {
if (er.code !== 'ENOENT' && er.code !== 'ENOTDIR') {
Expand Down
8 changes: 3 additions & 5 deletions lib/commands/unpublish.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@ const libaccess = require('libnpmaccess')
const libunpub = require('libnpmpublish').unpublish
const npa = require('npm-package-arg')
const npmFetch = require('npm-registry-fetch')
const path = require('path')
const util = require('util')
const readJson = util.promisify(require('read-package-json'))
const pkgJson = require('@npmcli/package-json')

const { flatten } = require('../utils/config/index.js')
const getIdentity = require('../utils/get-identity.js')
Expand Down Expand Up @@ -96,8 +94,8 @@ class Unpublish extends BaseCommand {
let manifest
let manifestErr
try {
const pkgJson = path.join(this.npm.localPrefix, 'package.json')
manifest = await readJson(pkgJson)
const { content } = await pkgJson.prepare(this.npm.localPrefix)
manifest = content
} catch (err) {
manifestErr = err
}
Expand Down
4 changes: 2 additions & 2 deletions lib/utils/did-you-mean.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const { distance } = require('fastest-levenshtein')
const readJson = require('read-package-json-fast')
const pkgJson = require('@npmcli/package-json')
const { commands } = require('./cmd-list.js')

const didYouMean = async (npm, path, scmd) => {
Expand All @@ -13,7 +13,7 @@ const didYouMean = async (npm, path, scmd) => {
// We would already be suggesting this in `npm x` so omit them here
const runScripts = ['stop', 'start', 'test', 'restart']
try {
const { bin, scripts } = await readJson(`${path}/package.json`)
const { content: { scripts, bin } } = await pkgJson.normalize(path)
best = best.concat(
Object.keys(scripts || {})
.filter(cmd => distance(scmd, cmd) < scmd.length * 0.4 && !runScripts.includes(cmd))
Expand Down
6 changes: 3 additions & 3 deletions lib/workspaces/get-workspaces.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
const { resolve, relative } = require('path')
const mapWorkspaces = require('@npmcli/map-workspaces')
const { minimatch } = require('minimatch')
const rpj = require('read-package-json-fast')
const pkgJson = require('@npmcli/package-json')

// minimatch wants forward slashes only for glob patterns
const globify = pattern => pattern.split('\\').join('/')

// Returns an Map of paths to workspaces indexed by workspace name
// { foo => '/path/to/foo' }
const getWorkspaces = async (filters, { path, includeWorkspaceRoot, relativeFrom }) => {
// TODO we need a better error to be bubbled up here if this rpj call fails
const pkg = await rpj(resolve(path, 'package.json'))
// TODO we need a better error to be bubbled up here if this call fails
const { content: pkg } = await pkgJson.normalize(path)
const workspaces = await mapWorkspaces({ cwd: path, pkg })
let res = new Map()
if (includeWorkspaceRoot) {
Expand Down
2 changes: 1 addition & 1 deletion test/lib/commands/init.js
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ t.test('workspaces', async t => {
await t.test('fail parsing top-level package.json to set workspace', async t => {
const { npm } = await mockNpm(t, {
prefixDir: {
'package.json': 'not json['
'package.json': 'not json[',
},
config: { workspace: 'a', yes: true },
noLog: true,
Expand Down
2 changes: 1 addition & 1 deletion test/lib/commands/unpublish.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ t.test('no args --force error reading package.json', async t => {

await t.rejects(
npm.exec('unpublish', []),
/Failed to parse json/,
/Invalid package.json/,
'should throw error from reading package.json'
)
})
Expand Down

0 comments on commit 636e29e

Please sign in to comment.