Skip to content

Commit 5baa4a7

Browse files
wraithgarwraithgar
authored
fix: consolidate bugs, docs, repo command logic (#4857)
All three of these commands do the same thing: open a manifest and find a url inside to open it. The finding of that manifest was not very consistent across these three commands. Some work with workspaces while others don't. Some work correctly with `--prefix` while others don't. This PR consolidates these commands so that they all are consistent in how they find the manifest being referenced. The specifics of which url they open are still left to each command. The util that only these three commands were using was consolidated into their base class.
1 parent 86f443e commit 5baa4a7

File tree

11 files changed

+167
-149
lines changed

11 files changed

+167
-149
lines changed

‎docs/content/commands/npm-bugs.md

+63-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ description: Report bugs for a package in a web browser
1111
<!-- see lib/commands/bugs.js -->
1212

1313
```bash
14-
npm bugs [<pkgname>]
14+
npm bugs [<pkgname> [<pkgname> ...]]
1515

1616
alias: issues
1717
```
@@ -58,6 +58,68 @@ The base URL of the npm registry.
5858
<!-- automatically generated, do not edit manually -->
5959
<!-- see lib/utils/config/definitions.js -->
6060
61+
#### `workspace`
62+
63+
* Default:
64+
* Type: String (can be set multiple times)
65+
66+
Enable running a command in the context of the configured workspaces of the
67+
current project while filtering by running only the workspaces defined by
68+
this configuration option.
69+
70+
Valid values for the `workspace` config are either:
71+
72+
* Workspace names
73+
* Path to a workspace directory
74+
* Path to a parent workspace directory (will result in selecting all
75+
workspaces within that folder)
76+
77+
When set for the `npm init` command, this may be set to the folder of a
78+
workspace which does not yet exist, to create the folder and set it up as a
79+
brand new workspace within the project.
80+
81+
This value is not exported to the environment for child processes.
82+
83+
<!-- automatically generated, do not edit manually -->
84+
<!-- see lib/utils/config/definitions.js -->
85+
86+
#### `workspaces`
87+
88+
* Default: null
89+
* Type: null or Boolean
90+
91+
Set to true to run the command in the context of **all** configured
92+
workspaces.
93+
94+
Explicitly setting this to false will cause commands like `install` to
95+
ignore workspaces altogether. When not set explicitly:
96+
97+
- Commands that operate on the `node_modules` tree (install, update, etc.)
98+
will link workspaces into the `node_modules` folder. - Commands that do
99+
other things (test, exec, publish, etc.) will operate on the root project,
100+
_unless_ one or more workspaces are specified in the `workspace` config.
101+
102+
This value is not exported to the environment for child processes.
103+
104+
<!-- automatically generated, do not edit manually -->
105+
<!-- see lib/utils/config/definitions.js -->
106+
107+
#### `include-workspace-root`
108+
109+
* Default: false
110+
* Type: Boolean
111+
112+
Include the workspace root when workspaces are enabled for a command.
113+
114+
When false, specifying individual workspaces via the `workspace` config, or
115+
all workspaces via the `workspaces` flag, will cause npm to operate only on
116+
the specified workspaces, and not on the root project.
117+
118+
This value is not exported to the environment for child processes.
119+
120+
<!-- automatically generated, do not edit manually -->
121+
<!-- see lib/utils/config/definitions.js -->
122+
61123
<!-- AUTOGENERATED CONFIG DESCRIPTIONS END -->
62124
63125
### See Also

‎docs/content/commands/npm-repo.md

+10
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,16 @@ Set to `true` to use default system URL opener.
4646
<!-- automatically generated, do not edit manually -->
4747
<!-- see lib/utils/config/definitions.js -->
4848
49+
#### `registry`
50+
51+
* Default: "https://registry.npmjs.org/"
52+
* Type: URL
53+
54+
The base URL of the npm registry.
55+
56+
<!-- automatically generated, do not edit manually -->
57+
<!-- see lib/utils/config/definitions.js -->
58+
4959
#### `workspace`
5060
5161
* Default:

‎lib/commands/bugs.js

+4-27
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,10 @@
1-
const pacote = require('pacote')
2-
const log = require('../utils/log-shim')
3-
const openUrl = require('../utils/open-url.js')
4-
const hostedFromMani = require('../utils/hosted-git-info-from-manifest.js')
5-
const BaseCommand = require('../base-command.js')
1+
const PackageUrlCmd = require('../package-url-cmd.js')
62

7-
class Bugs extends BaseCommand {
3+
class Bugs extends PackageUrlCmd {
84
static description = 'Report bugs for a package in a web browser'
95
static name = 'bugs'
10-
static usage = ['[<pkgname>]']
11-
static params = ['browser', 'registry']
12-
static ignoreImplicitWorkspace = true
136

14-
async exec (args) {
15-
if (!args || !args.length) {
16-
args = ['.']
17-
}
18-
19-
await Promise.all(args.map(pkg => this.getBugs(pkg)))
20-
}
21-
22-
async getBugs (pkg) {
23-
const opts = { ...this.npm.flatOptions, fullMetadata: true }
24-
const mani = await pacote.manifest(pkg, opts)
25-
const url = this.getBugsUrl(mani)
26-
log.silly('bugs', 'url', url)
27-
await openUrl(this.npm, url, `${mani.name} bug list available at the following URL`)
28-
}
29-
30-
getBugsUrl (mani) {
7+
getUrl (spec, mani) {
318
if (mani.bugs) {
329
if (typeof mani.bugs === 'string') {
3310
return mani.bugs
@@ -43,7 +20,7 @@ class Bugs extends BaseCommand {
4320
}
4421

4522
// try to get it from the repo, if possible
46-
const info = hostedFromMani(mani)
23+
const info = this.hostedFromMani(mani)
4724
if (info) {
4825
return info.bugs()
4926
}

‎lib/commands/docs.js

+5-40
Original file line numberDiff line numberDiff line change
@@ -1,54 +1,19 @@
1-
const pacote = require('pacote')
2-
const openUrl = require('../utils/open-url.js')
3-
const hostedFromMani = require('../utils/hosted-git-info-from-manifest.js')
4-
const log = require('../utils/log-shim')
5-
const BaseCommand = require('../base-command.js')
6-
class Docs extends BaseCommand {
1+
const PackageUrlCmd = require('../package-url-cmd.js')
2+
class Docs extends PackageUrlCmd {
73
static description = 'Open documentation for a package in a web browser'
84
static name = 'docs'
9-
static params = [
10-
'browser',
11-
'registry',
12-
'workspace',
13-
'workspaces',
14-
'include-workspace-root',
15-
]
165

17-
static usage = ['[<pkgname> [<pkgname> ...]]']
18-
static ignoreImplicitWorkspace = false
19-
20-
async exec (args) {
21-
if (!args || !args.length) {
22-
args = ['.']
23-
}
24-
25-
await Promise.all(args.map(pkg => this.getDocs(pkg)))
26-
}
27-
28-
async execWorkspaces (args, filters) {
29-
await this.setWorkspaces(filters)
30-
return this.exec(this.workspacePaths)
31-
}
32-
33-
async getDocs (pkg) {
34-
const opts = { ...this.npm.flatOptions, fullMetadata: true }
35-
const mani = await pacote.manifest(pkg, opts)
36-
const url = this.getDocsUrl(mani)
37-
log.silly('docs', 'url', url)
38-
await openUrl(this.npm, url, `${mani.name} docs available at the following URL`)
39-
}
40-
41-
getDocsUrl (mani) {
6+
getUrl (spec, mani) {
427
if (mani.homepage) {
438
return mani.homepage
449
}
4510

46-
const info = hostedFromMani(mani)
11+
const info = this.hostedFromMani(mani)
4712
if (info) {
4813
return info.docs()
4914
}
5015

51-
return 'https://www.npmjs.com/package/' + mani.name
16+
return `https://www.npmjs.com/package/${mani.name}`
5217
}
5318
}
5419
module.exports = Docs

‎lib/commands/repo.js

+7-38
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,11 @@
1-
const pacote = require('pacote')
21
const { URL } = require('url')
3-
const log = require('../utils/log-shim')
4-
const hostedFromMani = require('../utils/hosted-git-info-from-manifest.js')
5-
const openUrl = require('../utils/open-url.js')
62

7-
const BaseCommand = require('../base-command.js')
8-
class Repo extends BaseCommand {
3+
const PackageUrlCmd = require('../package-url-cmd.js')
4+
class Repo extends PackageUrlCmd {
95
static description = 'Open package repository page in the browser'
106
static name = 'repo'
11-
static params = ['browser', 'workspace', 'workspaces', 'include-workspace-root']
12-
static usage = ['[<pkgname> [<pkgname> ...]]']
13-
static ignoreImplicitWorkspace = false
14-
15-
async exec (args) {
16-
if (!args || !args.length) {
17-
args = ['.']
18-
}
19-
20-
await Promise.all(args.map(pkg => this.get(pkg)))
21-
}
22-
23-
async execWorkspaces (args, filters) {
24-
await this.setWorkspaces(filters)
25-
return this.exec(this.workspacePaths)
26-
}
27-
28-
async get (pkg) {
29-
// XXX It is very odd that `where` is how pacote knows to look anywhere
30-
// other than the cwd.
31-
const opts = {
32-
...this.npm.flatOptions,
33-
where: this.npm.localPrefix,
34-
fullMetadata: true,
35-
}
36-
const mani = await pacote.manifest(pkg, opts)
377

8+
getUrl (spec, mani) {
389
const r = mani.repository
3910
const rurl = !r ? null
4011
: typeof r === 'string' ? r
@@ -43,22 +14,20 @@ class Repo extends BaseCommand {
4314

4415
if (!rurl) {
4516
throw Object.assign(new Error('no repository'), {
46-
pkgid: pkg,
17+
pkgid: spec,
4718
})
4819
}
4920

50-
const info = hostedFromMani(mani)
21+
const info = this.hostedFromMani(mani)
5122
const url = info ?
5223
info.browse(mani.repository.directory) : unknownHostedUrl(rurl)
5324

5425
if (!url) {
5526
throw Object.assign(new Error('no repository: could not get url'), {
56-
pkgid: pkg,
27+
pkgid: spec,
5728
})
5829
}
59-
60-
log.silly('docs', 'url', url)
61-
await openUrl(this.npm, url, `${mani.name} repo available at the following URL`)
30+
return url
6231
}
6332
}
6433
module.exports = Repo

‎lib/package-url-cmd.js

+61
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
// Base command for opening urls from a package manifest (bugs, docs, repo)
2+
3+
const pacote = require('pacote')
4+
const hostedGitInfo = require('hosted-git-info')
5+
6+
const openUrl = require('./utils/open-url.js')
7+
const log = require('./utils/log-shim')
8+
9+
const BaseCommand = require('./base-command.js')
10+
class PackageUrlCommand extends BaseCommand {
11+
static ignoreImplicitWorkspace = false
12+
static params = [
13+
'browser',
14+
'registry',
15+
'workspace',
16+
'workspaces',
17+
'include-workspace-root',
18+
]
19+
20+
static usage = ['[<pkgname> [<pkgname> ...]]']
21+
22+
async exec (args) {
23+
if (!args || !args.length) {
24+
args = ['.']
25+
}
26+
27+
for (const arg of args) {
28+
// XXX It is very odd that `where` is how pacote knows to look anywhere
29+
// other than the cwd.
30+
const opts = {
31+
...this.npm.flatOptions,
32+
where: this.npm.localPrefix,
33+
fullMetadata: true,
34+
}
35+
const mani = await pacote.manifest(arg, opts)
36+
const url = this.getUrl(arg, mani)
37+
log.silly(this.name, 'url', url)
38+
await openUrl(this.npm, url, `${mani.name} ${this.name} available at the following URL`)
39+
}
40+
}
41+
42+
async execWorkspaces (args, filters) {
43+
await this.setWorkspaces(filters)
44+
return this.exec(this.workspacePaths)
45+
}
46+
47+
// given a manifest, try to get the hosted git info from it based on
48+
// repository (if a string) or repository.url (if an object) returns null
49+
// if it's not a valid repo, or not a known hosted repo
50+
hostedFromMani (mani) {
51+
const r = mani.repository
52+
const rurl = !r ? null
53+
: typeof r === 'string' ? r
54+
: typeof r === 'object' && typeof r.url === 'string' ? r.url
55+
: null
56+
57+
// hgi returns undefined sometimes, but let's always return null here
58+
return (rurl && hostedGitInfo.fromUrl(rurl.replace(/^git\+/, ''))) || null
59+
}
60+
}
61+
module.exports = PackageUrlCommand

‎lib/utils/hosted-git-info-from-manifest.js

-14
This file was deleted.

‎tap-snapshots/test/lib/load-all-commands.js.test.cjs

+4-2
Original file line numberDiff line numberDiff line change
@@ -81,10 +81,12 @@ exports[`test/lib/load-all-commands.js TAP load each command bugs > must match s
8181
Report bugs for a package in a web browser
8282
8383
Usage:
84-
npm bugs [<pkgname>]
84+
npm bugs [<pkgname> [<pkgname> ...]]
8585
8686
Options:
8787
[--no-browser|--browser <browser>] [--registry <registry>]
88+
[-w|--workspace <workspace-name> [-w|--workspace <workspace-name> ...]]
89+
[-ws|--workspaces] [--include-workspace-root]
8890
8991
alias: issues
9092
@@ -727,7 +729,7 @@ Usage:
727729
npm repo [<pkgname> [<pkgname> ...]]
728730
729731
Options:
730-
[--no-browser|--browser <browser>]
732+
[--no-browser|--browser <browser>] [--registry <registry>]
731733
[-w|--workspace <workspace-name> [-w|--workspace <workspace-name> ...]]
732734
[-ws|--workspaces] [--include-workspace-root]
733735

‎tap-snapshots/test/lib/utils/npm-usage.js.test.cjs

+4-2
Original file line numberDiff line numberDiff line change
@@ -223,10 +223,12 @@ All commands:
223223
bugs Report bugs for a package in a web browser
224224
225225
Usage:
226-
npm bugs [<pkgname>]
226+
npm bugs [<pkgname> [<pkgname> ...]]
227227
228228
Options:
229229
[--no-browser|--browser <browser>] [--registry <registry>]
230+
[-w|--workspace <workspace-name> [-w|--workspace <workspace-name> ...]]
231+
[-ws|--workspaces] [--include-workspace-root]
230232
231233
alias: issues
232234
@@ -777,7 +779,7 @@ All commands:
777779
npm repo [<pkgname> [<pkgname> ...]]
778780
779781
Options:
780-
[--no-browser|--browser <browser>]
782+
[--no-browser|--browser <browser>] [--registry <registry>]
781783
[-w|--workspace <workspace-name> [-w|--workspace <workspace-name> ...]]
782784
[-ws|--workspaces] [--include-workspace-root]
783785

0 commit comments

Comments
 (0)