Skip to content

Commit

Permalink
feat(diff): add workspace support
Browse files Browse the repository at this point in the history
Refactored a bit so that we can more easily change the `top` and
`prefix` params that were being used, and are different under the
workspace context.

PR-URL: #3368
Credit: @wraithgar
Close: #3368
Reviewed-by: @ruyadorno
  • Loading branch information
wraithgar committed Jun 10, 2021
1 parent 992799c commit ef668ab
Show file tree
Hide file tree
Showing 7 changed files with 242 additions and 173 deletions.
32 changes: 32 additions & 0 deletions docs/content/commands/npm-diff.md
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,38 @@ command, if no explicit tag is given.
When used by the `npm diff` command, this is the tag used to fetch the
tarball that will be compared with the local files by default.
#### `workspace`
* Default:
* Type: String (can be set multiple times)
Enable running a command in the context of the configured workspaces of the
current project while filtering by running only the workspaces defined by
this configuration option.
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)
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
brand new workspace within the project.
This value is not exported to the environment for child processes.
#### `workspaces`
* Default: false
* Type: Boolean
Enable running a command in the context of **all** the configured
workspaces.
This value is not exported to the environment for child processes.
<!-- AUTOGENERATED CONFIG DESCRIPTIONS END -->
## See Also
Expand Down
146 changes: 74 additions & 72 deletions lib/diff.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
const { resolve } = require('path')

const semver = require('semver')
const libdiff = require('libnpmdiff')
const libnpmdiff = require('libnpmdiff')
const npa = require('npm-package-arg')
const Arborist = require('@npmcli/arborist')
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 All @@ -25,10 +26,6 @@ class Diff extends BaseCommand {
static get usage () {
return [
'[...<paths>]',
'--diff=<pkg-name> [...<paths>]',
'--diff=<version-a> [--diff=<version-b>] [...<paths>]',
'--diff=<spec-a> [--diff=<spec-b>] [...<paths>]',
'[--diff-ignore-all-space] [--diff-name-only] [...<paths>] [...<paths>]',
]
}

Expand All @@ -45,19 +42,19 @@ class Diff extends BaseCommand {
'diff-text',
'global',
'tag',
'workspace',
'workspaces',
]
}

get where () {
const globalTop = resolve(this.npm.globalDir, '..')
const global = this.npm.config.get('global')
return global ? globalTop : this.npm.prefix
}

exec (args, cb) {
this.diff(args).then(() => cb()).catch(cb)
}

execWorkspaces (args, filters, cb) {
this.diffWorkspaces(args, filters).then(() => cb()).catch(cb)
}

async diff (args) {
const specs = this.npm.config.get('diff').filter(d => d)
if (specs.length > 2) {
Expand All @@ -67,86 +64,96 @@ class Diff extends BaseCommand {
)
}

// diffWorkspaces may have set this already
if (!this.prefix)
this.prefix = this.npm.prefix

// this is the "top" directory, one up from node_modules
// in global mode we have to walk one up from globalDir because our
// node_modules is sometimes under ./lib, and in global mode we're only ever
// walking through node_modules (because we will have been given a package
// name already)
if (this.npm.config.get('global'))
this.top = resolve(this.npm.globalDir, '..')
else
this.top = this.prefix

const [a, b] = await this.retrieveSpecs(specs)
npmlog.info('diff', { src: a, dst: b })

const res = await libdiff([a, b], {
const res = await libnpmdiff([a, b], {
...this.npm.flatOptions,
diffFiles: args,
where: this.where,
where: this.top,
})
return this.npm.output(res)
}

async retrieveSpecs ([a, b]) {
// no arguments, defaults to comparing cwd
// to its latest published registry version
if (!a)
return this.defaultSpec()

// single argument, used to compare wanted versions of an
// installed dependency or to compare the cwd to a published version
if (!b)
return this.transformSingleSpec(a)

const specs = await this.convertVersionsToSpecs([a, b])
return this.findVersionsByPackageName(specs)
async diffWorkspaces (args, filters) {
const workspaces =
await getWorkspaces(filters, { path: this.npm.localPrefix })
for (const workspacePath of workspaces.values()) {
this.top = workspacePath
this.prefix = workspacePath
await this.diff(args)
}
}

async defaultSpec () {
let noPackageJson
let pkgName
// get the package name from the packument at `path`
// throws if no packument is present OR if it does not have `name` attribute
async packageName (path) {
let name
try {
pkgName = await readPackageName(this.npm.prefix)
// 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')
noPackageJson = true
}

if (!pkgName || noPackageJson) {
throw new Error(
'Needs multiple arguments to compare or run from a project dir.\n\n' +
`Usage:\n${this.usage}`
)
}
if (!name)
throw this.usageError('Needs multiple arguments to compare or run from a project dir.\n')

return [
`${pkgName}@${this.npm.config.get('tag')}`,
`file:${this.npm.prefix}`,
]
return name
}

async transformSingleSpec (a) {
async retrieveSpecs ([a, b]) {
if (a && b) {
const specs = await this.convertVersionsToSpecs([a, b])
return this.findVersionsByPackageName(specs)
}

// no arguments, defaults to comparing cwd
// to its latest published registry version
if (!a) {
const pkgName = await this.packageName(this.prefix)
return [
`${pkgName}@${this.npm.config.get('tag')}`,
`file:${this.prefix}`,
]
}

// single argument, used to compare wanted versions of an
// installed dependency or to compare the cwd to a published version
let noPackageJson
let pkgName
try {
pkgName = await readPackageName(this.npm.prefix)
pkgName = await readPackageName(this.prefix)
} catch (e) {
npmlog.verbose('diff', 'could not read project dir package.json')
noPackageJson = true
}
const missingPackageJson = new Error(
'Needs multiple arguments to compare or run from a project dir.\n\n' +
`Usage:\n${this.usage}`
)

const specSelf = () => {
if (noPackageJson)
throw missingPackageJson

return `file:${this.npm.prefix}`
}
const missingPackageJson = this.usageError('Needs multiple arguments to compare or run from a project dir.\n')

// using a valid semver range, that means it should just diff
// the cwd against a published version to the registry using the
// same project name and the provided semver range
if (semver.validRange(a)) {
if (!pkgName)
throw missingPackageJson

return [
`${pkgName}@${a}`,
specSelf(),
`file:${this.prefix}`,
]
}

Expand All @@ -160,7 +167,7 @@ class Diff extends BaseCommand {
try {
const opts = {
...this.npm.flatOptions,
path: this.where,
path: this.top,
}
const arb = new Arborist(opts)
actualTree = await arb.loadActual(opts)
Expand All @@ -172,9 +179,11 @@ class Diff extends BaseCommand {
}

if (!node || !node.name || !node.package || !node.package.version) {
if (noPackageJson)
throw missingPackageJson
return [
`${spec.name}@${spec.fetchSpec}`,
specSelf(),
`file:${this.prefix}`,
]
}

Expand Down Expand Up @@ -220,14 +229,10 @@ class Diff extends BaseCommand {
} else if (spec.type === 'directory') {
return [
`file:${spec.fetchSpec}`,
specSelf(),
`file:${this.prefix}`,
]
} else {
throw new Error(
'Spec type not supported.\n\n' +
`Usage:\n${this.usage}`
)
}
} else
throw this.usageError(`Spec type ${spec.type} not supported.\n`)
}

async convertVersionsToSpecs ([a, b]) {
Expand All @@ -238,17 +243,14 @@ class Diff extends BaseCommand {
if (semverA && semverB) {
let pkgName
try {
pkgName = await readPackageName(this.npm.prefix)
pkgName = await readPackageName(this.prefix)
} catch (e) {
npmlog.verbose('diff', 'could not read project dir package.json')
}

if (!pkgName) {
throw new Error(
'Needs to be run from a project dir in order to diff two versions.\n\n' +
`Usage:\n${this.usage}`
)
}
if (!pkgName)
throw this.usageError('Needs to be run from a project dir in order to diff two versions.\n')

return [`${pkgName}@${a}`, `${pkgName}@${b}`]
}

Expand All @@ -269,7 +271,7 @@ class Diff extends BaseCommand {
try {
const opts = {
...this.npm.flatOptions,
path: this.where,
path: this.top,
}
const arb = new Arborist(opts)
actualTree = await arb.loadActual(opts)
Expand Down
8 changes: 6 additions & 2 deletions lib/utils/config/definition.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,12 @@ class Definition {
if (!this.typeDescription)
this.typeDescription = describeType(this.type)
// hint is only used for non-boolean values
if (!this.hint)
this.hint = `<${this.key}>`
if (!this.hint) {
if (this.type === Number)
this.hint = '<number>'
else
this.hint = `<${this.key}>`
}
if (!this.usage)
this.usage = describeUsage(this)
}
Expand Down
3 changes: 3 additions & 0 deletions lib/utils/config/definitions.js
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,7 @@ define('dev', {

define('diff', {
default: [],
hint: '<pkg-name|spec|version>',
type: [String, Array],
description: `
Define arguments to compare in \`npm diff\`.
Expand Down Expand Up @@ -545,6 +546,7 @@ define('diff-no-prefix', {

define('diff-dst-prefix', {
default: 'b/',
hint: '<path>',
type: String,
description: `
Destination prefix to be used in \`npm diff\` output.
Expand All @@ -554,6 +556,7 @@ define('diff-dst-prefix', {

define('diff-src-prefix', {
default: 'a/',
hint: '<path>',
type: String,
description: `
Source prefix to be used in \`npm diff\` output.
Expand Down
12 changes: 5 additions & 7 deletions tap-snapshots/test/lib/load-all-commands.js.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -199,16 +199,14 @@ The registry diff command
Usage:
npm diff [...<paths>]
npm diff --diff=<pkg-name> [...<paths>]
npm diff --diff=<version-a> [--diff=<version-b>] [...<paths>]
npm diff --diff=<spec-a> [--diff=<spec-b>] [...<paths>]
npm diff [--diff-ignore-all-space] [--diff-name-only] [...<paths>] [...<paths>]
Options:
[--diff <diff> [--diff <diff> ...]] [--diff-name-only]
[--diff-unified <diff-unified>] [--diff-ignore-all-space] [--diff-no-prefix]
[--diff-src-prefix <diff-src-prefix>] [--diff-dst-prefix <diff-dst-prefix>]
[--diff <pkg-name|spec|version> [--diff <pkg-name|spec|version> ...]]
[--diff-name-only] [--diff-unified <number>] [--diff-ignore-all-space]
[--diff-no-prefix] [--diff-src-prefix <path>] [--diff-dst-prefix <path>]
[--diff-text] [-g|--global] [--tag <tag>]
[-w|--workspace <workspace-name> [-w|--workspace <workspace-name> ...]]
[-ws|--workspaces]
Run "npm help diff" for more info
`
Expand Down
12 changes: 5 additions & 7 deletions tap-snapshots/test/lib/utils/npm-usage.js.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -336,16 +336,14 @@ All commands:
Usage:
npm diff [...<paths>]
npm diff --diff=<pkg-name> [...<paths>]
npm diff --diff=<version-a> [--diff=<version-b>] [...<paths>]
npm diff --diff=<spec-a> [--diff=<spec-b>] [...<paths>]
npm diff [--diff-ignore-all-space] [--diff-name-only] [...<paths>] [...<paths>]
Options:
[--diff <diff> [--diff <diff> ...]] [--diff-name-only]
[--diff-unified <diff-unified>] [--diff-ignore-all-space] [--diff-no-prefix]
[--diff-src-prefix <diff-src-prefix>] [--diff-dst-prefix <diff-dst-prefix>]
[--diff <pkg-name|spec|version> [--diff <pkg-name|spec|version> ...]]
[--diff-name-only] [--diff-unified <number>] [--diff-ignore-all-space]
[--diff-no-prefix] [--diff-src-prefix <path>] [--diff-dst-prefix <path>]
[--diff-text] [-g|--global] [--tag <tag>]
[-w|--workspace <workspace-name> [-w|--workspace <workspace-name> ...]]
[-ws|--workspaces]
Run "npm help diff" for more info
Expand Down
Loading

0 comments on commit ef668ab

Please sign in to comment.