Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(workspaces): explicitly error in global mode #3417

Merged
merged 1 commit into from
Jun 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class Audit extends ArboristWorkspaceCmd {
audit: true,
path: this.npm.prefix,
reporter,
workspaces: this.workspaces,
workspaces: this.workspaceNames,
}

const arb = new Arborist(opts)
Expand Down
9 changes: 9 additions & 0 deletions lib/base-command.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Base class for npm.commands[cmd]
const usageUtil = require('./utils/usage.js')
const ConfigDefinitions = require('./utils/config/definitions.js')
const getWorkspaces = require('./workspaces/get-workspaces.js')

class BaseCommand {
constructor (npm) {
Expand Down Expand Up @@ -72,5 +73,13 @@ class BaseCommand {
{ code: 'ENOWORKSPACES' }
)
}

async setWorkspaces (filters) {
// TODO npm guards workspaces/global mode so we should use this.npm.prefix?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer using this.npm.localPrefix to avoid the extra jump when reading across those

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not like that we have both. It is prone to error.

const ws = await getWorkspaces(filters, { path: this.npm.localPrefix })
this.workspaces = ws
this.workspaceNames = [...ws.keys()]
this.workspacePaths = [...ws.values()]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😍

}
}
module.exports = BaseCommand
2 changes: 1 addition & 1 deletion lib/ci.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class CI extends ArboristWorkspaceCmd {
path: where,
log: this.npm.log,
save: false, // npm ci should never modify the lockfile or package.json
workspaces: this.workspaces,
workspaces: this.workspaceNames,
}

const arb = new Arborist(opts)
Expand Down
2 changes: 1 addition & 1 deletion lib/dedupe.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class Dedupe extends ArboristWorkspaceCmd {
log: this.npm.log,
path: where,
dryRun,
workspaces: this.workspaces,
workspaces: this.workspaceNames,
}
const arb = new Arborist(opts)
await arb.dedupe(opts)
Expand Down
7 changes: 2 additions & 5 deletions lib/diff.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ const npmlog = require('npmlog')
const pacote = require('pacote')
const pickManifest = require('npm-pick-manifest')

const getWorkspaces = require('./workspaces/get-workspaces.js')
const readPackageName = require('./utils/read-package-name.js')
const BaseCommand = require('./base-command.js')

Expand Down Expand Up @@ -90,9 +89,8 @@ class Diff extends BaseCommand {
}

async diffWorkspaces (args, filters) {
const workspaces =
await getWorkspaces(filters, { path: this.npm.localPrefix })
for (const workspacePath of workspaces.values()) {
await this.setWorkspaces(filters)
for (const workspacePath of this.workspacePaths) {
this.top = workspacePath
this.prefix = workspacePath
await this.diff(args)
Expand All @@ -104,7 +102,6 @@ class Diff extends BaseCommand {
async packageName (path) {
let name
try {
// TODO this won't work as expected in global mode
name = await readPackageName(this.prefix)
} catch (e) {
npmlog.verbose('diff', 'could not read project dir package.json')
Expand Down
6 changes: 2 additions & 4 deletions lib/dist-tag.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ const semver = require('semver')

const otplease = require('./utils/otplease.js')
const readPackageName = require('./utils/read-package-name.js')
const getWorkspaces = require('./workspaces/get-workspaces.js')
const BaseCommand = require('./base-command.js')

class DistTag extends BaseCommand {
Expand Down Expand Up @@ -180,10 +179,9 @@ class DistTag extends BaseCommand {
}

async listWorkspaces (filters) {
const workspaces =
await getWorkspaces(filters, { path: this.npm.localPrefix })
await this.setWorkspaces(filters)

for (const [name] of workspaces) {
for (const name of this.workspaceNames) {
try {
this.npm.output(`${name}:`)
await this.list(npa(name), this.npm.flatOptions)
Expand Down
6 changes: 2 additions & 4 deletions lib/docs.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ const log = require('npmlog')
const pacote = require('pacote')
const openUrl = require('./utils/open-url.js')
const hostedFromMani = require('./utils/hosted-git-info-from-manifest.js')
const getWorkspaces = require('./workspaces/get-workspaces.js')

const BaseCommand = require('./base-command.js')
class Docs extends BaseCommand {
Expand Down Expand Up @@ -42,9 +41,8 @@ class Docs extends BaseCommand {
}

async docsWorkspaces (args, filters) {
const workspaces =
await getWorkspaces(filters, { path: this.npm.localPrefix })
return this.docs([...workspaces.values()])
await this.setWorkspaces(filters)
return this.docs(this.workspacePaths)
}

async getDocs (pkg) {
Expand Down
12 changes: 5 additions & 7 deletions lib/exec.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
const libexec = require('libnpmexec')
const BaseCommand = require('./base-command.js')
const getLocationMsg = require('./exec/get-workspace-location-msg.js')
const getWorkspaces = require('./workspaces/get-workspaces.js')

// it's like this:
//
Expand Down Expand Up @@ -105,16 +104,15 @@ class Exec extends BaseCommand {
}

async _execWorkspaces (args, filters) {
const workspaces =
await getWorkspaces(filters, { path: this.npm.localPrefix })
await this.setWorkspaces(filters)
const color = this.npm.config.get('color')

for (const workspacePath of workspaces.values()) {
const locationMsg = await getLocationMsg({ color, path: workspacePath })
for (const path of this.workspacePaths) {
const locationMsg = await getLocationMsg({ color, path })
await this._exec(args, {
locationMsg,
path: workspacePath,
runPath: workspacePath,
path,
runPath: path,
})
}
}
Expand Down
4 changes: 2 additions & 2 deletions lib/explain.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ class Explain extends ArboristWorkspaceCmd {
const arb = new Arborist({ path: this.npm.prefix, ...this.npm.flatOptions })
const tree = await arb.loadActual()

if (this.workspaces && this.workspaces.length)
this.filterSet = arb.workspaceDependencySet(tree, this.workspaces)
if (this.workspaceNames && this.workspaceNames.length)
this.filterSet = arb.workspaceDependencySet(tree, this.workspaceNames)

const nodes = new Set()
for (const arg of args) {
Expand Down
2 changes: 1 addition & 1 deletion lib/fund.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ class Fund extends ArboristWorkspaceCmd {
const fundingInfo = getFundingInfo(tree, {
...this.flatOptions,
log: this.npm.log,
workspaces: this.workspaces,
workspaces: this.workspaceNames,
})

if (this.npm.config.get('json'))
Expand Down
2 changes: 1 addition & 1 deletion lib/install.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ class Install extends ArboristWorkspaceCmd {
auditLevel: null,
path: where,
add: args,
workspaces: this.workspaces,
workspaces: this.workspaceNames,
}
const arb = new Arborist(opts)
await arb.reify(opts)
Expand Down
2 changes: 1 addition & 1 deletion lib/link.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ class Link extends ArboristWorkspaceCmd {
log: this.npm.log,
add: names.map(l => `file:${resolve(globalTop, 'node_modules', l)}`),
save,
workspaces: this.workspaces,
workspaces: this.workspaceNames,
})

await reifyFinish(this.npm, localArb)
Expand Down
4 changes: 2 additions & 2 deletions lib/ls.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,8 @@ class LS extends ArboristWorkspaceCmd {
// We only have to filter the first layer of edges, so we don't
// explore anything that isn't part of the selected workspace set.
let wsNodes
if (this.workspaces && this.workspaces.length)
wsNodes = arb.workspaceNodes(tree, this.workspaces)
if (this.workspaceNames && this.workspaceNames.length)
wsNodes = arb.workspaceNodes(tree, this.workspaceNames)
const filterBySelectedWorkspaces = edge => {
if (!wsNodes || !wsNodes.length)
return true
Expand Down
3 changes: 3 additions & 0 deletions lib/npm.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ const npm = module.exports = new class extends EventEmitter {
this.output(impl.usage)
cb()
} else if (filterByWorkspaces) {
if (this.config.get('global'))
return cb(new Error('Workspaces not supported for global packages'))

impl.execWorkspaces(args, this.config.get('workspace'), er => {
process.emit('timeEnd', `command:${cmd}`)
cb(er)
Expand Down
6 changes: 4 additions & 2 deletions lib/outdated.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,10 @@ class Outdated extends ArboristWorkspaceCmd {
this.list = []
this.tree = await arb.loadActual()

if (this.workspaces && this.workspaces.length)
this.filterSet = arb.workspaceDependencySet(this.tree, this.workspaces)
if (this.workspaceNames && this.workspaceNames.length) {
this.filterSet =
arb.workspaceDependencySet(this.tree, this.workspaceNames)
}

if (args.length !== 0) {
// specific deps
Expand Down
6 changes: 2 additions & 4 deletions lib/pack.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ const log = require('npmlog')
const pacote = require('pacote')
const libpack = require('libnpmpack')
const npa = require('npm-package-arg')
const getWorkspaces = require('./workspaces/get-workspaces.js')

const { getContents, logTar } = require('./utils/tar.js')

Expand Down Expand Up @@ -97,9 +96,8 @@ class Pack extends BaseCommand {
return this.pack(args)
}

const workspaces =
await getWorkspaces(filters, { path: this.npm.localPrefix })
return this.pack([...workspaces.values(), ...args.filter(a => a !== '.')])
await this.setWorkspaces(filters)
return this.pack([...this.workspacePaths, ...args.filter(a => a !== '.')])
}
}
module.exports = Pack
2 changes: 1 addition & 1 deletion lib/prune.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class Prune extends ArboristWorkspaceCmd {
...this.npm.flatOptions,
path: where,
log: this.npm.log,
workspaces: this.workspaces,
workspaces: this.workspaceNames,
}
const arb = new Arborist(opts)
await arb.prune(opts)
Expand Down
10 changes: 4 additions & 6 deletions lib/publish.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ const chalk = require('chalk')

const otplease = require('./utils/otplease.js')
const { getContents, logTar } = require('./utils/tar.js')
const getWorkspaces = require('./workspaces/get-workspaces.js')

// for historical reasons, publishConfig in package.json can contain ANY config
// keys that npm supports in .npmrc files and elsewhere. We *may* want to
Expand Down Expand Up @@ -138,7 +137,7 @@ class Publish extends BaseCommand {
})
}

if (!this.workspaces) {
if (!this.suppressOutput) {
if (!silent && json)
this.npm.output(JSON.stringify(pkgContents, null, 2))
else if (!silent)
Expand All @@ -150,17 +149,16 @@ class Publish extends BaseCommand {

async publishWorkspaces (args, filters) {
// Suppresses JSON output in publish() so we can handle it here
this.workspaces = true
this.suppressOutput = true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯


const results = {}
const json = this.npm.config.get('json')
const silent = log.level === 'silent'
const noop = a => a
const color = this.npm.color ? chalk : { green: noop, bold: noop }
const workspaces =
await getWorkspaces(filters, { path: this.npm.localPrefix })
await this.setWorkspaces(filters)

for (const [name, workspace] of workspaces.entries()) {
for (const [name, workspace] of this.workspaces.entries()) {
let pkgContents
try {
pkgContents = await this.publish([workspace])
Expand Down
2 changes: 1 addition & 1 deletion lib/rebuild.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class Rebuild extends ArboristWorkspaceCmd {
...this.npm.flatOptions,
path: where,
// TODO when extending ReifyCmd
// workspaces: this.workspaces,
// workspaces: this.workspaceNames,
})

if (args.length) {
Expand Down
6 changes: 2 additions & 4 deletions lib/repo.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
const log = require('npmlog')
const pacote = require('pacote')
const getWorkspaces = require('./workspaces/get-workspaces.js')
const { URL } = require('url')

const hostedFromMani = require('./utils/hosted-git-info-from-manifest.js')
Expand Down Expand Up @@ -44,9 +43,8 @@ class Repo extends BaseCommand {
}

async repoWorkspaces (args, filters) {
const workspaces =
await getWorkspaces(filters, { path: this.npm.localPrefix })
return this.repo([...workspaces.values()])
await this.setWorkspaces(filters)
return this.repo(this.workspacePaths)
}

async get (pkg) {
Expand Down
15 changes: 6 additions & 9 deletions lib/run-script.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ const rpj = require('read-package-json-fast')
const log = require('npmlog')
const didYouMean = require('./utils/did-you-mean.js')
const isWindowsShell = require('./utils/is-windows-shell.js')
const getWorkspaces = require('./workspaces/get-workspaces.js')

const cmdList = [
'publish',
Expand Down Expand Up @@ -195,10 +194,9 @@ class RunScript extends BaseCommand {

async runWorkspaces (args, filters) {
const res = []
const workspaces =
await getWorkspaces(filters, { path: this.npm.localPrefix })
await this.setWorkspaces(filters)

for (const workspacePath of workspaces.values()) {
for (const workspacePath of this.workspacePaths) {
const pkg = await rpj(`${workspacePath}/package.json`)
const runResult = await this.run(args, {
path: workspacePath,
Expand Down Expand Up @@ -227,15 +225,14 @@ class RunScript extends BaseCommand {
}

async listWorkspaces (args, filters) {
const workspaces =
await getWorkspaces(filters, { path: this.npm.localPrefix })
await this.setWorkspaces(filters)

if (log.level === 'silent')
return

if (this.npm.config.get('json')) {
const res = {}
for (const workspacePath of workspaces.values()) {
for (const workspacePath of this.workspacePaths) {
const { scripts, name } = await rpj(`${workspacePath}/package.json`)
res[name] = { ...scripts }
}
Expand All @@ -244,15 +241,15 @@ class RunScript extends BaseCommand {
}

if (this.npm.config.get('parseable')) {
for (const workspacePath of workspaces.values()) {
for (const workspacePath of this.workspacePaths) {
const { scripts, name } = await rpj(`${workspacePath}/package.json`)
for (const [script, cmd] of Object.entries(scripts || {}))
this.npm.output(`${name}:${script}:${cmd}`)
}
return
}

for (const workspacePath of workspaces.values())
for (const workspacePath of this.workspacePaths)
await this.list(args, workspacePath)
}
}
Expand Down
Loading