Skip to content

Commit

Permalink
feat(workspaces): --include-workspace-root
Browse files Browse the repository at this point in the history
Adds a new config item that includes the workspace root when running
non-arborist commands (i.e. repo, version, publish).  Arborist will need
to be udpated to look for this flag to change its behavior to include
the workspace root for its functions.

This also changes --workspaces to a trinary, so that setting it to false
will explicitly exclude workspaces altogether.  This is also going to
require an arborist change so that it ignores workspaces altogether.
  • Loading branch information
wraithgar committed Aug 10, 2021
1 parent 1aec805 commit 726b61c
Show file tree
Hide file tree
Showing 11 changed files with 156 additions and 77 deletions.
10 changes: 7 additions & 3 deletions lib/base-command.js
Expand Up @@ -7,8 +7,6 @@ class BaseCommand {
constructor (npm) {
this.wrapWidth = 80
this.npm = npm
this.workspaces = null
this.workspacePaths = null
}

get name () {
Expand Down Expand Up @@ -75,7 +73,13 @@ class BaseCommand {
}

async setWorkspaces (filters) {
const ws = await getWorkspaces(filters, { path: this.npm.localPrefix })
if (this.isArboristCmd)
this.includeWorkspaceRoot = false

const ws = await getWorkspaces(filters, {
path: this.npm.localPrefix,
includeWorkspaceRoot: this.includeWorkspaceRoot,
})
this.workspaces = ws
this.workspaceNames = [...ws.keys()]
this.workspacePaths = [...ws.values()]
Expand Down
2 changes: 1 addition & 1 deletion lib/config.js
Expand Up @@ -156,7 +156,7 @@ class Config extends BaseCommand {
const out = []
for (const key of keys) {
if (!publicVar(key))
throw `The ${key} option is protected, and cannot be retrieved in this way`
throw new Error(`The ${key} option is protected, and cannot be retrieved in this way`)

const pref = keys.length > 1 ? `${key}=` : ''
out.push(pref + this.npm.config.get(key))
Expand Down
9 changes: 9 additions & 0 deletions lib/npm.js
Expand Up @@ -138,6 +138,15 @@ const npm = module.exports = new class extends EventEmitter {
const workspacesEnabled = this.config.get('workspaces')
const workspacesFilters = this.config.get('workspace')
const filterByWorkspaces = workspacesEnabled || workspacesFilters.length > 0
// normally this would go in the constructor, but our tests don't
// actually use a real npm object so this.npm.config isn't always
// populated. this is the compromise until we can make that a reality
// and then move this into the constructor.
impl.workspaces = this.config.get('npm')
impl.workspacePaths = null
// normally this would be evaluated in base-command#setWorkspaces, see
// above for explanation
impl.includeWorkspaceRoot = this.config.get('include-workspace-root')

if (this.config.get('usage')) {
this.output(impl.usage)
Expand Down
8 changes: 7 additions & 1 deletion lib/repo.js
Expand Up @@ -48,7 +48,13 @@ class Repo extends BaseCommand {
}

async get (pkg) {
const opts = { ...this.npm.flatOptions, fullMetadata: true }
// XXX It is very odd that `where` is how pacote knows to look anywhere
// other than the cwd.
const opts = {
...this.npm.flatOptions,
where: this.npm.localPrefix,
fullMetadata: true,
}
const mani = await pacote.manifest(pkg, opts)

const r = mani.repository
Expand Down
20 changes: 16 additions & 4 deletions lib/utils/config/definitions.js
Expand Up @@ -908,6 +908,15 @@ define('include-staged', {
flatten,
})

define('include-workspace-root', {
default: false,
type: Boolean,
description: `
Include the workspace root when workspaces are enabled for a command.
`,
flatten,
})

define('init-author-email', {
default: '',
type: String,
Expand Down Expand Up @@ -2119,8 +2128,8 @@ define('workspace', {
* Workspace names
* Path to a workspace directory
* Path to a parent workspace directory (will result to selecting all of the
nested workspaces)
* Path to a parent workspace directory (will result in selecting all
workspaces within that folder)
When set for the \`npm init\` command, this may be set to the folder of
a workspace which does not yet exist, to create the folder and set it
Expand All @@ -2129,13 +2138,16 @@ define('workspace', {
})

define('workspaces', {
default: false,
type: Boolean,
default: null,
type: [null, Boolean],
short: 'ws',
envExport: false,
description: `
Enable running a command in the context of **all** the configured
workspaces.
Explicitly setting this to false will cause commands like \`install\` to
ignore workspaces altogether.
`,
})

Expand Down
7 changes: 6 additions & 1 deletion lib/workspaces/arborist-cmd.js
Expand Up @@ -4,6 +4,11 @@

const BaseCommand = require('../base-command.js')
class ArboristCmd extends BaseCommand {
constructor () {
super(...arguments)
this.isArboristCmd = true
}

/* istanbul ignore next - see test/lib/load-all-commands.js */
static get params () {
return [
Expand All @@ -13,7 +18,7 @@ class ArboristCmd extends BaseCommand {
}

execWorkspaces (args, filters, cb) {
this.setWorkspaces(filters)
this.setWorkspaces(filters, true)
.then(() => {
this.exec(args, cb)
})
Expand Down
9 changes: 7 additions & 2 deletions lib/workspaces/get-workspaces.js
Expand Up @@ -5,11 +5,16 @@ const rpj = require('read-package-json-fast')

// Returns an Map of paths to workspaces indexed by workspace name
// { foo => '/path/to/foo' }
const getWorkspaces = async (filters, { path }) => {
const getWorkspaces = async (filters, { path, includeWorkspaceRoot }) => {
// TODO we need a better error to be bubbled up here if this rpj call fails
const pkg = await rpj(resolve(path, 'package.json'))
const workspaces = await mapWorkspaces({ cwd: path, pkg })
const res = filters.length ? new Map() : workspaces
let res = new Map()
if (includeWorkspaceRoot)
res.set(pkg.name, path)

if (!filters.length)
res = new Map([...res, ...workspaces])

for (const filterArg of filters) {
for (const [workspaceName, workspacePath] of workspaces.entries()) {
Expand Down
21 changes: 17 additions & 4 deletions tap-snapshots/test/lib/utils/config/definitions.js.test.cjs
Expand Up @@ -63,6 +63,7 @@ Array [
"ignore-scripts",
"include",
"include-staged",
"include-workspace-root",
"init-author-email",
"init-author-name",
"init-author-url",
Expand Down Expand Up @@ -825,6 +826,15 @@ Allow installing "staged" published packages, as defined by [npm RFC PR
This is experimental, and not implemented by the npm public registry.
`

exports[`test/lib/utils/config/definitions.js TAP > config description for include-workspace-root 1`] = `
#### \`include-workspace-root\`
* Default: false
* Type: Boolean
Include the workspace root when workspaces are enabled for a command.
`

exports[`test/lib/utils/config/definitions.js TAP > config description for init-author-email 1`] = `
#### \`init-author-email\`
Expand Down Expand Up @@ -1833,8 +1843,8 @@ Valid values for the \`workspace\` config are either:
* Workspace names
* Path to a workspace directory
* Path to a parent workspace directory (will result to selecting all of the
nested workspaces)
* Path to a parent workspace directory (will result in selecting all
workspaces within that folder)
When set for the \`npm init\` command, this may be set to the folder of a
workspace which does not yet exist, to create the folder and set it up as a
Expand All @@ -1846,12 +1856,15 @@ This value is not exported to the environment for child processes.
exports[`test/lib/utils/config/definitions.js TAP > config description for workspaces 1`] = `
#### \`workspaces\`
* Default: false
* Type: Boolean
* Default: null
* Type: null or Boolean
Enable running a command in the context of **all** the configured
workspaces.
Explicitly setting this to false will cause commands like \`install\` to
ignore workspaces altogether.
This value is not exported to the environment for child processes.
`

Expand Down
18 changes: 14 additions & 4 deletions tap-snapshots/test/lib/utils/config/describe-all.js.test.cjs
Expand Up @@ -521,6 +521,13 @@ Allow installing "staged" published packages, as defined by [npm RFC PR
This is experimental, and not implemented by the npm public registry.
#### \`include-workspace-root\`
* Default: false
* Type: Boolean
Include the workspace root when workspaces are enabled for a command.
#### \`init-author-email\`
* Default: ""
Expand Down Expand Up @@ -1246,8 +1253,8 @@ Valid values for the \`workspace\` config are either:
* Workspace names
* Path to a workspace directory
* Path to a parent workspace directory (will result to selecting all of the
nested workspaces)
* Path to a parent workspace directory (will result in selecting all
workspaces within that folder)
When set for the \`npm init\` command, this may be set to the folder of a
workspace which does not yet exist, to create the folder and set it up as a
Expand All @@ -1257,12 +1264,15 @@ This value is not exported to the environment for child processes.
#### \`workspaces\`
* Default: false
* Type: Boolean
* Default: null
* Type: null or Boolean
Enable running a command in the context of **all** the configured
workspaces.
Explicitly setting this to false will cause commands like \`install\` to
ignore workspaces altogether.
This value is not exported to the environment for child processes.
#### \`yes\`
Expand Down

0 comments on commit 726b61c

Please sign in to comment.