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

lib/star.js

@@ -3,73 +3,75 @@
 const fetch = require('npm-registry-fetch')
 const log = require('npmlog')
 const npa = require('npm-package-arg')
+
 const npm = require('./npm.js')
 const output = require('./utils/output.js')
-const usage = require('./utils/usage.js')
-const getItentity = require('./utils/get-identity')
+const usageUtil = require('./utils/usage.js')
+const getIdentity = require('./utils/get-identity')
+const completion = require('./utils/completion/none.js')
 
-star.usage = usage(
+const usage = usageUtil(
  'star',
  'npm star [<pkg>...]\n' +
  'npm unstar [<pkg>...]'
 )
 
-star.completion = function (opts, cb) {
- // FIXME: there used to be registry completion here, but it stopped making
- // sense somewhere around 50,000 packages on the registry
- cb()
-}
+const cmd = (args, cb) => star(args).then(() => cb()).catch(cb)
+
+const star = async args => {
+ if (!args.length)
+ throw new Error(usage)
+
+ // if we're unstarring, then show an empty star image
+ // otherwise, show the full star image
+ const { unicode } = npm.flatOptions
+ const unstar = npm.config.get('star.unstar')
+ const full = unicode ? '\u2605 ' : '(*)'
+ const empty = unicode ? '\u2606 ' : '( )'
+ const show = unstar ? empty : full
+
+ const pkgs = args.map(npa)
+ for (const pkg of pkgs) {
+ const [username, fullData] = await Promise.all([
+ getIdentity(npm.flatOptions),
+ fetch.json(pkg.escapedName, {
+ ...npm.flatOptions,
+ spec: pkg,
+ query: { write: true },
+ preferOnline: true,
+ }),
+ ])
 
-module.exports = star
-function star (args, cb) {
- const opts = npm.flatOptions
- return Promise.resolve().then(() => {
- if (!args.length)
- throw new Error(star.usage)
- // if we're unstarring, then show an empty star image
- // otherwise, show the full star image
- const unstar = /^un/.test(npm.command)
- const full = opts.unicode ? '\u2605 ' : '(*)'
- const empty = opts.unicode ? '\u2606 ' : '( )'
- const show = unstar ? empty : full
- return Promise.all(args.map(npa).map(pkg => {
- return Promise.all([
- getItentity(opts),
- fetch.json(pkg.escapedName, {
- ...opts,
- spec: pkg,
- query: { write: true },
- preferOnline: true,
- }),
- ]).then(([username, fullData]) => {
- if (!username)
- throw new Error('You need to be logged in!')
- const body = {
- _id: fullData._id,
- _rev: fullData._rev,
- users: fullData.users || {},
- }
+ if (!username)
+ throw new Error('You need to be logged in!')
 
- if (!unstar) {
- log.info('star', 'starring', body._id)
- body.users[username] = true
- log.verbose('star', 'starring', body)
- } else {
- delete body.users[username]
- log.info('star', 'unstarring', body._id)
- log.verbose('star', 'unstarring', body)
- }
- return fetch.json(pkg.escapedName, {
- ...opts,
- spec: pkg,
- method: 'PUT',
- body,
- })
- }).then(data => {
- output(show + ' ' + pkg.name)
- log.verbose('star', data)
- return data
- })
- }))
- }).then(() => cb(), cb)
+ const body = {
+ _id: fullData._id,
+ _rev: fullData._rev,
+ users: fullData.users || {},
+ }
+
+ if (!unstar) {
+ log.info('star', 'starring', body._id)
+ body.users[username] = true
+ log.verbose('star', 'starring', body)
+ } else {
+ delete body.users[username]
+ log.info('unstar', 'unstarring', body._id)
+ log.verbose('unstar', 'unstarring', body)
+ }
+
+ const data = await fetch.json(pkg.escapedName, {
+ ...npm.flatOptions,
+ spec: pkg,
+ method: 'PUT',
+ body,
+ })
+
+ output(show + ' ' + pkg.name)
+ log.verbose('star', data)
+ return data
+ }
 }
+
+module.exports = Object.assign(cmd, { completion, usage })

lib/unstar.js

@@ -0,0 +1,9 @@
+const { usage, completion } = require('./star.js')
+const npm = require('./npm.js')
+
+const unstar = (args, cb) => {
+ npm.config.set('star.unstar', true)
+ return npm.commands.star(args, cb)
+}
+
+module.exports = Object.assign(unstar, { usage, completion })

lib/utils/cmd-list.js

@@ -12,7 +12,6 @@ const shorthands = {
  c: 'config',
  s: 'search',
  se: 'search',
- unstar: 'star', // same function
  tst: 'test',
  t: 'test',
  ddp: 'dedupe',
@@ -88,6 +87,7 @@ const cmdList = [
  'publish',
  'star',
  'stars',
+ 'unstar',
  'adduser',
  'login', // This is an alias for `adduser` but it can be confusing
  'logout',

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

@@ -94,7 +94,6 @@ Object {
  "udpate": "update",
  "un": "uninstall",
  "unlink": "uninstall",
- "unstar": "star",
  "up": "update",
  "upgrade": "update",
  "urn": "run-script",
@@ -125,6 +124,7 @@ Object {
  "publish",
  "star",
  "stars",
+ "unstar",
  "adduser",
  "login",
  "logout",
@@ -189,7 +189,6 @@ Object {
  "t": "test",
  "tst": "test",
  "un": "uninstall",
- "unstar": "star",
  "up": "update",
  "v": "view",
  "why": "explain",

test/lib/star.js

@@ -0,0 +1,144 @@
+const requireInject = require('require-inject')
+const t = require('tap')
+
+let result = ''
+
+const noop = () => null
+const npm = { config: { get () {} }, flatOptions: { unicode: false } }
+const npmFetch = { json: noop }
+const npmlog = { error: noop, info: noop, verbose: noop }
+const mocks = {
+ npmlog,
+ 'npm-registry-fetch': npmFetch,
+ '../../lib/npm.js': npm,
+ '../../lib/utils/output.js': (...msg) => {
+ result += msg.join('\n')
+ },
+ '../../lib/utils/get-identity.js': async () => 'foo',
+ '../../lib/utils/usage.js': () => 'usage instructions',
+}
+
+const star = requireInject('../../lib/star.js', mocks)
+
+t.afterEach(cb => {
+ npm.config = { get () {} }
+ npm.flatOptions.unicode = false
+ npmlog.info = noop
+ result = ''
+ cb()
+})
+
+t.test('no args', t => {
+ star([], err => {
+ t.match(
+ err,
+ /usage instructions/,
+ 'should throw usage instructions'
+ )
+ t.end()
+ })
+})
+
+t.test('star a package', t => {
+ t.plan(4)
+ const pkgName = '@npmcli/arborist'
+ npmFetch.json = async (uri, opts) => ({
+ _id: pkgName,
+ _rev: 'hash',
+ users: (
+ opts.method === 'PUT'
+ ? { foo: true }
+ : {}
+ ),
+ })
+ npmlog.info = (title, msg, id) => {
+ t.equal(title, 'star', 'should use expected title')
+ t.equal(msg, 'starring', 'should use expected msg')
+ t.equal(id, pkgName, 'should use expected id')
+ }
+ star([pkgName], err => {
+ if (err)
+ throw err
+ t.equal(
+ result,
+ '(*) @npmcli/arborist',
+ 'should output starred package msg'
+ )
+ })
+})
+
+t.test('unstar a package', t => {
+ t.plan(4)
+ const pkgName = '@npmcli/arborist'
+ npm.config.get = key => key === 'star.unstar'
+ npmFetch.json = async (uri, opts) => ({
+ _id: pkgName,
+ _rev: 'hash',
+ ...(opts.method === 'PUT'
+ ? {}
+ : { foo: true }
+ ),
+ })
+ npmlog.info = (title, msg, id) => {
+ t.equal(title, 'unstar', 'should use expected title')
+ t.equal(msg, 'unstarring', 'should use expected msg')
+ t.equal(id, pkgName, 'should use expected id')
+ }
+ star([pkgName], err => {
+ if (err)
+ throw err
+ t.equal(
+ result,
+ '( ) @npmcli/arborist',
+ 'should output unstarred package msg'
+ )
+ })
+})
+
+t.test('unicode', async t => {
+ t.test('star a package', t => {
+ npm.flatOptions.unicode = true
+ npmFetch.json = async (uri, opts) => ({})
+ star(['pkg'], err => {
+ if (err)
+ throw err
+ t.equal(
+ result,
+ '\u2605 pkg',
+ 'should output unicode starred package msg'
+ )
+ t.end()
+ })
+ })
+
+ t.test('unstar a package', t => {
+ npm.flatOptions.unicode = true
+ npm.config.get = key => key === 'star.unstar'
+ npmFetch.json = async (uri, opts) => ({})
+ star(['pkg'], err => {
+ if (err)
+ throw err
+ t.equal(
+ result,
+ '\u2606 pkg',
+ 'should output unstarred package msg'
+ )
+ t.end()
+ })
+ })
+})
+
+t.test('logged out user', t => {
+ const star = requireInject('../../lib/star.js', {
+ ...mocks,
+ '../../lib/utils/get-identity.js': async () => undefined,
+ })
+ star(['@npmcli/arborist'], err => {
+ t.match(
+ err,
+ /You need to be logged in/,
+ 'should throw login required error'
+ )
+ t.end()
+ })
+})

test/lib/unstar.js

@@ -0,0 +1,28 @@
+const requireInject = require('require-inject')
+const t = require('tap')
+
+t.test('unstar', t => {
+ t.plan(3)
+
+ const unstar = requireInject('../../lib/unstar.js', {
+ '../../lib/npm.js': {
+ config: {
+ set: (key, value) => {
+ t.equal(key, 'star.unstar', 'should set unstar config value')
+ t.equal(value, true, 'should set a truthy value')
+ },
+ },
+ commands: {
+ star: (args, cb) => {
+ t.deepEqual(args, ['pkg'], 'should forward packages')
+ cb()
+ },
+ },
+ },
+ })
+
+ unstar(['pkg'], err => {
+ if (err)
+ throw err
+ })
+})