Skip to content

Commit 3ee8f3b

Browse files
isaacsnlf
authored andcommitted
view: Better errors when package.json is not JSON
Previously, `npm view <noargs>` was just saying "Invalid package.json" if the package.json file was not JSON, or if it was missing. This errors out with the appropriate error in these cases. Also, no need to read the big clunky 'read-package-json' for this, we're literally just checking for a single field. We can be a bit more efficient here. PR-URL: #2051 Credit: @isaacs Close: #2051 Reviewed-by: @nlf
1 parent 5db95b3 commit 3ee8f3b

File tree

2 files changed

+45
-5
lines changed

2 files changed

+45
-5
lines changed

lib/view.js

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,16 @@ const npm = require('./npm.js')
99
const { packument } = require('pacote')
1010
const path = require('path')
1111
const { inspect, promisify } = require('util')
12-
const readJson = promisify(require('read-package-json'))
1312
const relativeDate = require('tiny-relative-date')
1413
const semver = require('semver')
1514
const style = require('ansistyles')
1615
const usageUtil = require('./utils/usage')
1716

17+
const fs = require('fs')
18+
const readFile = promisify(fs.readFile)
19+
const jsonParse = require('json-parse-even-better-errors')
20+
const readJson = async file => jsonParse(await readFile(file, 'utf8'))
21+
1822
const usage = usageUtil(
1923
'view',
2024
'npm view [<@scope>/]<pkg>[@<version>] [<field>[.subfield]...]'
@@ -86,8 +90,8 @@ const view = async args => {
8690
if (local) {
8791
const dir = npm.prefix
8892
const manifest = await readJson(path.resolve(dir, 'package.json'))
89-
if (!manifest || !manifest.name)
90-
throw new Error('Invalid package.json')
93+
if (!manifest.name)
94+
throw new Error('Invalid package.json, no "name" field')
9195
const p = manifest.name
9296
nv = npa(p)
9397
if (pkg && ~pkg.indexOf('@'))

test/lib/view.js

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,43 @@ t.test('throw error if global mode', (t) => {
482482
})
483483
})
484484

485-
t.test('throw error if invalid package.json', (t) => {
485+
t.test('throw ENOENT error if package.json misisng', (t) => {
486+
const testDir = t.testdir({})
487+
488+
const view = requireInject('../../lib/view.js', {
489+
'../../lib/npm.js': {
490+
prefix: testDir,
491+
flatOptions: {
492+
global: false
493+
}
494+
}
495+
})
496+
view([], (err) => {
497+
t.match(err, { code: 'ENOENT' })
498+
t.end()
499+
})
500+
})
501+
502+
t.test('throw EJSONPARSE error if package.json not json', (t) => {
503+
const testDir = t.testdir({
504+
'package.json': 'not json, nope, not even a little bit!'
505+
})
506+
507+
const view = requireInject('../../lib/view.js', {
508+
'../../lib/npm.js': {
509+
prefix: testDir,
510+
flatOptions: {
511+
global: false
512+
}
513+
}
514+
})
515+
view([], (err) => {
516+
t.match(err, { code: 'EJSONPARSE' })
517+
t.end()
518+
})
519+
})
520+
521+
t.test('throw error if package.json has no name', (t) => {
486522
const testDir = t.testdir({
487523
'package.json': '{}'
488524
})
@@ -496,7 +532,7 @@ t.test('throw error if invalid package.json', (t) => {
496532
}
497533
})
498534
view([], (err) => {
499-
t.equals(err.message, 'Invalid package.json')
535+
t.equals(err.message, 'Invalid package.json, no "name" field')
500536
t.end()
501537
})
502538
})

0 commit comments

Comments
 (0)