Skip to content

Commit 5fc56b6

Browse files
ruyadornonlf
ruyadorno
authored and
nlf
committed
fix: npm unstar
- Refactored lib/star.js - Fixes `npm unstar` by adding a lib/unstar.js alias/cmd - Add tests for lib/star.js and lib/unstar.js Fixes: npm/statusboard#174 PR-URL: #2204 Credit: @ruyadorno Close: #2204 Reviewed-by: @nlf
1 parent e1a2837 commit 5fc56b6

6 files changed

Lines changed: 244 additions & 62 deletions

File tree

‎lib/star.js‎

Lines changed: 61 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -3,73 +3,75 @@
33
const fetch = require('npm-registry-fetch')
44
const log = require('npmlog')
55
const npa = require('npm-package-arg')
6+
67
const npm = require('./npm.js')
78
const output = require('./utils/output.js')
8-
const usage = require('./utils/usage.js')
9-
const getItentity = require('./utils/get-identity')
9+
const usageUtil = require('./utils/usage.js')
10+
const getIdentity = require('./utils/get-identity')
11+
const completion = require('./utils/completion/none.js')
1012

11-
star.usage = usage(
13+
const usage = usageUtil(
1214
'star',
1315
'npm star [<pkg>...]\n' +
1416
'npm unstar [<pkg>...]'
1517
)
1618

17-
star.completion = function (opts, cb) {
18-
// FIXME: there used to be registry completion here, but it stopped making
19-
// sense somewhere around 50,000 packages on the registry
20-
cb()
21-
}
19+
const cmd = (args, cb) => star(args).then(() => cb()).catch(cb)
20+
21+
const star = async args => {
22+
if (!args.length)
23+
throw new Error(usage)
24+
25+
// if we're unstarring, then show an empty star image
26+
// otherwise, show the full star image
27+
const { unicode } = npm.flatOptions
28+
const unstar = npm.config.get('star.unstar')
29+
const full = unicode ? '\u2605 ' : '(*)'
30+
const empty = unicode ? '\u2606 ' : '( )'
31+
const show = unstar ? empty : full
32+
33+
const pkgs = args.map(npa)
34+
for (const pkg of pkgs) {
35+
const [username, fullData] = await Promise.all([
36+
getIdentity(npm.flatOptions),
37+
fetch.json(pkg.escapedName, {
38+
...npm.flatOptions,
39+
spec: pkg,
40+
query: { write: true },
41+
preferOnline: true,
42+
}),
43+
])
2244

23-
module.exports = star
24-
function star (args, cb) {
25-
const opts = npm.flatOptions
26-
return Promise.resolve().then(() => {
27-
if (!args.length)
28-
throw new Error(star.usage)
29-
// if we're unstarring, then show an empty star image
30-
// otherwise, show the full star image
31-
const unstar = /^un/.test(npm.command)
32-
const full = opts.unicode ? '\u2605 ' : '(*)'
33-
const empty = opts.unicode ? '\u2606 ' : '( )'
34-
const show = unstar ? empty : full
35-
return Promise.all(args.map(npa).map(pkg => {
36-
return Promise.all([
37-
getItentity(opts),
38-
fetch.json(pkg.escapedName, {
39-
...opts,
40-
spec: pkg,
41-
query: { write: true },
42-
preferOnline: true,
43-
}),
44-
]).then(([username, fullData]) => {
45-
if (!username)
46-
throw new Error('You need to be logged in!')
47-
const body = {
48-
_id: fullData._id,
49-
_rev: fullData._rev,
50-
users: fullData.users || {},
51-
}
45+
if (!username)
46+
throw new Error('You need to be logged in!')
5247

53-
if (!unstar) {
54-
log.info('star', 'starring', body._id)
55-
body.users[username] = true
56-
log.verbose('star', 'starring', body)
57-
} else {
58-
delete body.users[username]
59-
log.info('star', 'unstarring', body._id)
60-
log.verbose('star', 'unstarring', body)
61-
}
62-
return fetch.json(pkg.escapedName, {
63-
...opts,
64-
spec: pkg,
65-
method: 'PUT',
66-
body,
67-
})
68-
}).then(data => {
69-
output(show + ' ' + pkg.name)
70-
log.verbose('star', data)
71-
return data
72-
})
73-
}))
74-
}).then(() => cb(), cb)
48+
const body = {
49+
_id: fullData._id,
50+
_rev: fullData._rev,
51+
users: fullData.users || {},
52+
}
53+
54+
if (!unstar) {
55+
log.info('star', 'starring', body._id)
56+
body.users[username] = true
57+
log.verbose('star', 'starring', body)
58+
} else {
59+
delete body.users[username]
60+
log.info('unstar', 'unstarring', body._id)
61+
log.verbose('unstar', 'unstarring', body)
62+
}
63+
64+
const data = await fetch.json(pkg.escapedName, {
65+
...npm.flatOptions,
66+
spec: pkg,
67+
method: 'PUT',
68+
body,
69+
})
70+
71+
output(show + ' ' + pkg.name)
72+
log.verbose('star', data)
73+
return data
74+
}
7575
}
76+
77+
module.exports = Object.assign(cmd, { completion, usage })

‎lib/unstar.js‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
const { usage, completion } = require('./star.js')
2+
const npm = require('./npm.js')
3+
4+
const unstar = (args, cb) => {
5+
npm.config.set('star.unstar', true)
6+
return npm.commands.star(args, cb)
7+
}
8+
9+
module.exports = Object.assign(unstar, { usage, completion })

‎lib/utils/cmd-list.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ const shorthands = {
1212
c: 'config',
1313
s: 'search',
1414
se: 'search',
15-
unstar: 'star', // same function
1615
tst: 'test',
1716
t: 'test',
1817
ddp: 'dedupe',
@@ -88,6 +87,7 @@ const cmdList = [
8887
'publish',
8988
'star',
9089
'stars',
90+
'unstar',
9191
'adduser',
9292
'login', // This is an alias for `adduser` but it can be confusing
9393
'logout',

‎tap-snapshots/test-lib-utils-cmd-list.js-TAP.test.js‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,6 @@ Object {
9494
"udpate": "update",
9595
"un": "uninstall",
9696
"unlink": "uninstall",
97-
"unstar": "star",
9897
"up": "update",
9998
"upgrade": "update",
10099
"urn": "run-script",
@@ -125,6 +124,7 @@ Object {
125124
"publish",
126125
"star",
127126
"stars",
127+
"unstar",
128128
"adduser",
129129
"login",
130130
"logout",
@@ -189,7 +189,6 @@ Object {
189189
"t": "test",
190190
"tst": "test",
191191
"un": "uninstall",
192-
"unstar": "star",
193192
"up": "update",
194193
"v": "view",
195194
"why": "explain",

‎test/lib/star.js‎

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
const requireInject = require('require-inject')
2+
const t = require('tap')
3+
4+
let result = ''
5+
6+
const noop = () => null
7+
const npm = { config: { get () {} }, flatOptions: { unicode: false } }
8+
const npmFetch = { json: noop }
9+
const npmlog = { error: noop, info: noop, verbose: noop }
10+
const mocks = {
11+
npmlog,
12+
'npm-registry-fetch': npmFetch,
13+
'../../lib/npm.js': npm,
14+
'../../lib/utils/output.js': (...msg) => {
15+
result += msg.join('\n')
16+
},
17+
'../../lib/utils/get-identity.js': async () => 'foo',
18+
'../../lib/utils/usage.js': () => 'usage instructions',
19+
}
20+
21+
const star = requireInject('../../lib/star.js', mocks)
22+
23+
t.afterEach(cb => {
24+
npm.config = { get () {} }
25+
npm.flatOptions.unicode = false
26+
npmlog.info = noop
27+
result = ''
28+
cb()
29+
})
30+
31+
t.test('no args', t => {
32+
star([], err => {
33+
t.match(
34+
err,
35+
/usage instructions/,
36+
'should throw usage instructions'
37+
)
38+
t.end()
39+
})
40+
})
41+
42+
t.test('star a package', t => {
43+
t.plan(4)
44+
const pkgName = '@npmcli/arborist'
45+
npmFetch.json = async (uri, opts) => ({
46+
_id: pkgName,
47+
_rev: 'hash',
48+
users: (
49+
opts.method === 'PUT'
50+
? { foo: true }
51+
: {}
52+
),
53+
})
54+
npmlog.info = (title, msg, id) => {
55+
t.equal(title, 'star', 'should use expected title')
56+
t.equal(msg, 'starring', 'should use expected msg')
57+
t.equal(id, pkgName, 'should use expected id')
58+
}
59+
star([pkgName], err => {
60+
if (err)
61+
throw err
62+
t.equal(
63+
result,
64+
'(*) @npmcli/arborist',
65+
'should output starred package msg'
66+
)
67+
})
68+
})
69+
70+
t.test('unstar a package', t => {
71+
t.plan(4)
72+
const pkgName = '@npmcli/arborist'
73+
npm.config.get = key => key === 'star.unstar'
74+
npmFetch.json = async (uri, opts) => ({
75+
_id: pkgName,
76+
_rev: 'hash',
77+
...(opts.method === 'PUT'
78+
? {}
79+
: { foo: true }
80+
),
81+
})
82+
npmlog.info = (title, msg, id) => {
83+
t.equal(title, 'unstar', 'should use expected title')
84+
t.equal(msg, 'unstarring', 'should use expected msg')
85+
t.equal(id, pkgName, 'should use expected id')
86+
}
87+
star([pkgName], err => {
88+
if (err)
89+
throw err
90+
t.equal(
91+
result,
92+
'( ) @npmcli/arborist',
93+
'should output unstarred package msg'
94+
)
95+
})
96+
})
97+
98+
t.test('unicode', async t => {
99+
t.test('star a package', t => {
100+
npm.flatOptions.unicode = true
101+
npmFetch.json = async (uri, opts) => ({})
102+
star(['pkg'], err => {
103+
if (err)
104+
throw err
105+
t.equal(
106+
result,
107+
'\u2605 pkg',
108+
'should output unicode starred package msg'
109+
)
110+
t.end()
111+
})
112+
})
113+
114+
t.test('unstar a package', t => {
115+
npm.flatOptions.unicode = true
116+
npm.config.get = key => key === 'star.unstar'
117+
npmFetch.json = async (uri, opts) => ({})
118+
star(['pkg'], err => {
119+
if (err)
120+
throw err
121+
t.equal(
122+
result,
123+
'\u2606 pkg',
124+
'should output unstarred package msg'
125+
)
126+
t.end()
127+
})
128+
})
129+
})
130+
131+
t.test('logged out user', t => {
132+
const star = requireInject('../../lib/star.js', {
133+
...mocks,
134+
'../../lib/utils/get-identity.js': async () => undefined,
135+
})
136+
star(['@npmcli/arborist'], err => {
137+
t.match(
138+
err,
139+
/You need to be logged in/,
140+
'should throw login required error'
141+
)
142+
t.end()
143+
})
144+
})

‎test/lib/unstar.js‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
const requireInject = require('require-inject')
2+
const t = require('tap')
3+
4+
t.test('unstar', t => {
5+
t.plan(3)
6+
7+
const unstar = requireInject('../../lib/unstar.js', {
8+
'../../lib/npm.js': {
9+
config: {
10+
set: (key, value) => {
11+
t.equal(key, 'star.unstar', 'should set unstar config value')
12+
t.equal(value, true, 'should set a truthy value')
13+
},
14+
},
15+
commands: {
16+
star: (args, cb) => {
17+
t.deepEqual(args, ['pkg'], 'should forward packages')
18+
cb()
19+
},
20+
},
21+
},
22+
})
23+
24+
unstar(['pkg'], err => {
25+
if (err)
26+
throw err
27+
})
28+
})

0 commit comments

Comments
 (0)