Skip to content

Commit

Permalink
fix: npm outdated parsing invalid specs
Browse files Browse the repository at this point in the history 
This commit fixes a problem in which npm outdated was breaking when
trying to read an invalid semver range spec defined for a given
installed dep by performing the `npm-package-arg` parsing within a
try/catch block instead of expecting to read properties from the
returned instance.

Also, adds the missing test for that specific line of code.

Fixes npm#1703
  • Loading branch information
@ruyadorno
ruyadorno committed Sep 24, 2020
1 parent 9bc6966 commit 99ea8a5
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 76 deletions.
14 changes: 8 additions & 6 deletions lib/outdated.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,14 +134,16 @@ async function outdated_ (tree, deps, opts) {
// on disk are not included in the output
if (edge.error === 'MISSING' && type !== 'dependencies') return

// if it's not a range, version, or tag, skip it
try {
npa(`${edge.name}@${edge.spec}`)
} catch (err) {
return null
}

try {
const packument = await getPackument(spec)
const expected = edge.spec
// if it's not a range, version, or tag, skip it
/* istanbul ignore next */
if (!npa(`${edge.name}@${edge.spec}`).registry) {
return null
}
const wanted = pickManifest(packument, expected, npm.flatOptions)
const latest = pickManifest(packument, '*', npm.flatOptions)

Expand Down Expand Up @@ -170,7 +172,7 @@ async function outdated_ (tree, deps, opts) {
err.code === 'E403' ||
err.code === 'E404')
) {
throw (err)
throw err
}
}
}
Expand Down
167 changes: 97 additions & 70 deletions test/lib/outdated.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,53 +3,53 @@ const requireInject = require('require-inject')

const packument = spec => {
const mocks = {
'alpha': {
'name': 'alpha',
alpha: {
name: 'alpha',
'dist-tags': {
'latest': '1.0.1'
latest: '1.0.1'
},
'versions': {
versions: {
'1.0.1': {
'version': '1.0.1',
'dependencies': {
'gamma': '2.0.0'
version: '1.0.1',
dependencies: {
gamma: '2.0.0'
}
}
}
},
'beta': {
'name': 'beta',
beta: {
name: 'beta',
'dist-tags': {
'latest': '1.0.1'
latest: '1.0.1'
},
'versions': {
versions: {
'1.0.1': {
'version': '1.0.1'
version: '1.0.1'
}
}
},
'gamma': {
'name': 'gamma',
gamma: {
name: 'gamma',
'dist-tags': {
'latest': '2.0.0'
latest: '2.0.0'
},
'versions': {
versions: {
'1.0.1': {
'version': '1.0.1',
},
version: '1.0.1'
},
'2.0.0': {
'version': '2.0.0'
version: '2.0.0'
}
}
},
'theta': {
'name': 'theta',
theta: {
name: 'theta',
'dist-tags': {
'latest': '1.0.1'
latest: '1.0.1'
},
'versions': {
versions: {
'1.0.1': {
'version': '1.0.1'
version: '1.0.1'
}
}
}
Expand All @@ -73,15 +73,18 @@ const cleanLogs = (done) => {
logs = ''
const fn = (...args) => {
logs += '\n'
args.map(el => logs += el)
args.map(el => {
logs += el
return logs
})
}
console.log = fn
done()
}

const globalDir = t.testdir({
'node_modules': {
'alpha': {
node_modules: {
alpha: {
'package.json': JSON.stringify({
name: 'alpha',
version: '1.0.0'
Expand All @@ -91,14 +94,14 @@ const globalDir = t.testdir({
})

const outdated = (dir, opts) => requireInject(
'../../lib/outdated.js',
'../../lib/outdated.js',
{
'../../lib/npm.js': {
prefix: dir,
globalDir: `${globalDir}/node_modules`,
flatOptions: opts
},
'pacote': {
pacote: {
packument
}
}
Expand Down Expand Up @@ -133,37 +136,37 @@ t.test('should display outdated deps', t => {
beta: '^1.0.0'
}
}, null, 2),
'node_modules': {
'alpha': {
node_modules: {
alpha: {
'package.json': JSON.stringify({
name: 'alpha',
version: '1.0.0',
dependencies: {
gamma: '2.0.0'
}
}, null, 2),
'node_modules': {
'gamma': {
node_modules: {
gamma: {
'package.json': JSON.stringify({
name: 'gamma',
version: '2.0.0'
}, null, 2)
}
}
},
'beta': {
beta: {
'package.json': JSON.stringify({
name: 'beta',
version: '1.0.0'
}, null, 2)
},
'gamma': {
},
gamma: {
'package.json': JSON.stringify({
name: 'gamma',
version: '1.0.1'
}, null, 2)
},
'zeta': {
zeta: {
'package.json': JSON.stringify({
name: 'zeta',
version: '1.0.0'
Expand All @@ -174,7 +177,7 @@ t.test('should display outdated deps', t => {

t.test('outdated global', t => {
outdated(null, {
global: true,
global: true
})([], () => {
t.matchSnapshot(logs)
t.end()
Expand All @@ -183,8 +186,8 @@ t.test('should display outdated deps', t => {

t.test('outdated', t => {
outdated(testDir, {
global: false,
color: true
global: false,
color: true
})([], () => {
t.matchSnapshot(logs)
t.end()
Expand All @@ -193,8 +196,8 @@ t.test('should display outdated deps', t => {

t.test('outdated --long', t => {
outdated(testDir, {
global: false,
long: true,
global: false,
long: true
})([], () => {
t.matchSnapshot(logs)
t.end()
Expand All @@ -203,8 +206,8 @@ t.test('should display outdated deps', t => {

t.test('outdated --json', t => {
outdated(testDir, {
global: false,
json: true,
global: false,
json: true
})([], () => {
t.matchSnapshot(logs)
t.end()
Expand All @@ -213,9 +216,9 @@ t.test('should display outdated deps', t => {

t.test('outdated --json --long', t => {
outdated(testDir, {
global: false,
json: true,
long: true
global: false,
json: true,
long: true
})([], () => {
t.matchSnapshot(logs)
t.end()
Expand All @@ -224,8 +227,8 @@ t.test('should display outdated deps', t => {

t.test('outdated --parseable', t => {
outdated(testDir, {
global: false,
parseable: true,
global: false,
parseable: true
})([], () => {
t.matchSnapshot(logs)
t.end()
Expand All @@ -234,9 +237,9 @@ t.test('should display outdated deps', t => {

t.test('outdated --parseable --long', t => {
outdated(testDir, {
global: false,
parseable: true,
long: true
global: false,
parseable: true,
long: true
})([], () => {
t.matchSnapshot(logs)
t.end()
Expand All @@ -245,7 +248,7 @@ t.test('should display outdated deps', t => {

t.test('outdated --all', t => {
outdated(testDir, {
all: true,
all: true
})([], () => {
t.matchSnapshot(logs)
t.end()
Expand All @@ -254,15 +257,14 @@ t.test('should display outdated deps', t => {

t.test('outdated specific dep', t => {
outdated(testDir, {
global: false,
global: false
})(['alpha'], () => {
t.matchSnapshot(logs)
t.end()
})
})

t.end()

})

t.test('should return if no outdated deps', t => {
Expand All @@ -274,8 +276,8 @@ t.test('should return if no outdated deps', t => {
alpha: '^1.0.0'
}
}, null, 2),
'node_modules': {
'alpha': {
node_modules: {
alpha: {
'package.json': JSON.stringify({
name: 'alpha',
version: '1.0.1'
Expand All @@ -285,11 +287,11 @@ t.test('should return if no outdated deps', t => {
})

outdated(testDir, {
global: false,
})([], () => {
global: false
})([], () => {
t.equals(logs.length, 0, 'no logs')
t.end()
})
})
})

t.test('throws if error with a dep', t => {
Expand All @@ -301,8 +303,8 @@ t.test('throws if error with a dep', t => {
eta: '^1.0.0'
}
}, null, 2),
'node_modules': {
'eta': {
node_modules: {
eta: {
'package.json': JSON.stringify({
name: 'eta',
version: '1.0.1'
Expand All @@ -312,11 +314,11 @@ t.test('throws if error with a dep', t => {
})

outdated(testDir, {
global: false,
})([], (err) => {
global: false
})([], (err) => {
t.equals(err.message, 'There is an error with this package.')
t.end()
})
})
})

t.test('should skip missing non-prod deps', t => {
Expand All @@ -328,13 +330,38 @@ t.test('should skip missing non-prod deps', t => {
beta: '^1.0.0'
}
}, null, 2),
'node_modules': {}
node_modules: {}
})

outdated(testDir, {
global: false,
})([], () => {
//t.equals(logs.length, 0, 'no logs')
global: false
})([], () => {
t.equals(logs.length, 0, 'no logs')
t.end()
})
})

t.test('should skip invalid pkg ranges', t => {
const testDir = t.testdir({
'package.json': JSON.stringify({
name: 'delta',
version: '1.0.0',
dependencies: {
alpha: '>=^2'
}
}, null, 2),
node_modules: {
alpha: {
'package.json': JSON.stringify({
name: 'alpha',
version: '1.0.0'
}, null, 2)
}
}
})

outdated(testDir, {})([], () => {
t.equals(logs.length, 0, 'no logs')
t.end()
})
})
})

0 comments on commit 99ea8a5

Please sign in to comment.