Skip to content

Commit

Permalink
fix: account for new npm-package-arg behavior
Browse files Browse the repository at this point in the history
`npm`, `npm@`, and `npm@*` are all now the same spec
  • Loading branch information
wraithgar authored and lukekarrys committed Oct 19, 2022
1 parent de2d33f commit 1afe5ba
Show file tree
Hide file tree
Showing 19 changed files with 91 additions and 91 deletions.
4 changes: 1 addition & 3 deletions lib/commands/deprecate.js
Expand Up @@ -39,9 +39,7 @@ class Deprecate extends BaseCommand {

// fetch the data and make sure it exists.
const p = npa(pkg)
// npa makes the default spec "latest", but for deprecation
// "*" is the appropriate default.
const spec = p.rawSpec === '' ? '*' : p.fetchSpec
const spec = p.rawSpec === '*' ? '*' : p.fetchSpec

if (semver.validRange(spec, true) === null) {
throw new Error(`invalid version range: ${spec}`)
Expand Down
4 changes: 2 additions & 2 deletions lib/commands/diff.js
Expand Up @@ -185,7 +185,7 @@ class Diff extends BaseCommand {
// work from the top of the arborist tree to find the original semver
// range declared in the package that depends on the package.
let bSpec
if (spec.rawSpec) {
if (spec.rawSpec !== '*') {
bSpec = spec.rawSpec
} else {
const bTargetVersion =
Expand Down Expand Up @@ -269,7 +269,7 @@ class Diff extends BaseCommand {

return specs.map(i => {
const spec = npa(i)
if (spec.rawSpec) {
if (spec.rawSpec !== '*') {
return i
}

Expand Down
5 changes: 1 addition & 4 deletions lib/commands/init.js
Expand Up @@ -98,10 +98,7 @@ class Init extends BaseCommand {
packageName = initerName
.replace(user + '/' + project, user + '/create-' + project)
} else if (req.registry) {
packageName = req.name.replace(/^(@[^/]+\/)?/, '$1create-')
if (req.rawSpec) {
packageName += '@' + req.rawSpec
}
packageName = `${req.name.replace(/^(@[^/]+\/)?/, '$1create-')}@${req.rawSpec}`
} else {
throw Object.assign(new Error(
'Unrecognized initializer: ' + initerName +
Expand Down
2 changes: 1 addition & 1 deletion lib/commands/rebuild.js
Expand Up @@ -39,7 +39,7 @@ class Rebuild extends ArboristWorkspaceCmd {
const tree = await arb.loadActual()
const specs = args.map(arg => {
const spec = npa(arg)
if (spec.type === 'tag' && spec.rawSpec === '') {
if (spec.rawSpec === '*') {
return spec
}

Expand Down
5 changes: 3 additions & 2 deletions lib/commands/view.js
Expand Up @@ -196,15 +196,16 @@ class View extends BaseCommand {
// get the data about this package
let version = this.npm.config.get('tag')
// rawSpec is the git url if this is from git
if (spec.type !== 'git' && spec.type !== 'directory' && spec.rawSpec) {
if (spec.type !== 'git' && spec.type !== 'directory' && spec.rawSpec !== '*') {
version = spec.rawSpec
}

const pckmnt = await packument(spec, opts)

if (pckmnt['dist-tags'] && pckmnt['dist-tags'][version]) {
if (pckmnt['dist-tags']?.[version]) {
version = pckmnt['dist-tags'][version]
}

if (pckmnt.time && pckmnt.time.unpublished) {
const u = pckmnt.time.unpublished
const er = new Error(`Unpublished on ${u.time}`)
Expand Down
2 changes: 1 addition & 1 deletion tap-snapshots/test/lib/commands/dist-tag.js.test.cjs
Expand Up @@ -21,7 +21,7 @@ latest: 1.0.0
`

exports[`test/lib/commands/dist-tag.js TAP ls on missing package > should log no dist-tag found msg 1`] = `
dist-tag ls Couldn't get dist-tag data for foo@latest
dist-tag ls Couldn't get dist-tag data for foo@*
`

Expand Down
28 changes: 14 additions & 14 deletions test/lib/commands/diff.js
Expand Up @@ -216,7 +216,7 @@ t.test('single arg', t => {
})

libnpmdiff = async ([a, b], opts) => {
t.equal(a, 'simple-output@latest', 'should forward single spec')
t.equal(a, 'simple-output@*', 'should forward single spec')
t.equal(b, `file:${path}`, 'should compare to cwd')
t.match(opts, npm.flatOptions, 'should forward flat options')
}
Expand Down Expand Up @@ -460,7 +460,7 @@ t.test('single arg', t => {
}
},
libnpmdiff: async ([a, b], opts) => {
t.equal(a, 'lorem@latest', 'should target latest version of pkg name')
t.equal(a, 'lorem@*', 'should target any version of pkg name')
t.equal(b, `file:${path}`, 'should target current cwd')
},
})
Expand All @@ -479,7 +479,7 @@ t.test('single arg', t => {
'package.json': JSON.stringify({ version: '1.0.0' }),
})
libnpmdiff = async ([a, b], opts) => {
t.equal(a, 'bar@latest', 'should target latest tag of name')
t.equal(a, 'bar@*', 'should target any version of pkg name')
t.equal(b, `file:${path}`, 'should compare to cwd')
}

Expand All @@ -493,7 +493,7 @@ t.test('single arg', t => {
t.plan(2)

libnpmdiff = async ([a, b], opts) => {
t.equal(a, 'foo@latest', 'should target latest tag of name')
t.equal(a, 'foo@*', 'should target any version of pkg name')
t.equal(b, `file:${fooPath}`, 'should compare to cwd')
}

Expand Down Expand Up @@ -592,7 +592,7 @@ t.test('first arg is a qualified spec', t => {

libnpmdiff = async ([a, b], opts) => {
t.equal(a, 'bar@1.0.0', 'should set expected first spec')
t.equal(b, 'bar-fork@latest', 'should target latest tag if not a dep')
t.equal(b, 'bar-fork@*', 'should target any version if not a dep')
}

config.diff = ['bar@1.0.0', 'bar-fork']
Expand Down Expand Up @@ -753,7 +753,7 @@ t.test('first arg is a known dependency name', async t => {
`bar@file:${resolve(path, 'node_modules/bar')}`,
'should target local node_modules pkg'
)
t.equal(b, 'bar-fork@latest', 'should set expected second spec')
t.equal(b, 'bar-fork@*', 'should set expected second spec')
}

npm.prefix = path
Expand Down Expand Up @@ -840,7 +840,7 @@ t.test('first arg is a valid semver range', t => {

libnpmdiff = async ([a, b], opts) => {
t.equal(a, 'bar@1.0.0', 'should use name from second arg')
t.equal(b, 'bar@latest', 'should compare against latest tag')
t.equal(b, 'bar@*', 'should compare against any version')
}

config.diff = ['1.0.0', 'bar']
Expand Down Expand Up @@ -884,7 +884,7 @@ t.test('first arg is an unknown dependency name', t => {
t.plan(4)

libnpmdiff = async ([a, b], opts) => {
t.equal(a, 'bar@latest', 'should set expected first spec')
t.equal(a, 'bar@*', 'should set expected first spec')
t.equal(b, 'bar@2.0.0', 'should set expected second spec')
t.match(opts, npm.flatOptions, 'should forward flat options')
t.match(opts, { where: fooPath }, 'should forward pacote options')
Expand Down Expand Up @@ -919,7 +919,7 @@ t.test('first arg is an unknown dependency name', t => {
})

libnpmdiff = async ([a, b], opts) => {
t.equal(a, 'bar-fork@latest', 'should use latest tag')
t.equal(a, 'bar-fork@*', 'should use any version')
t.equal(
b,
`bar@file:${resolve(path, 'node_modules/bar')}`,
Expand All @@ -940,7 +940,7 @@ t.test('first arg is an unknown dependency name', t => {
t.plan(2)

libnpmdiff = async ([a, b], opts) => {
t.equal(a, 'bar@latest', 'should use latest tag')
t.equal(a, 'bar@*', 'should use any version')
t.equal(b, 'bar@^1.0.0', 'should use name from first arg')
}

Expand All @@ -956,8 +956,8 @@ t.test('first arg is an unknown dependency name', t => {
t.plan(2)

libnpmdiff = async ([a, b], opts) => {
t.equal(a, 'bar@latest', 'should use latest tag')
t.equal(b, 'bar-fork@latest', 'should use latest tag')
t.equal(a, 'bar@*', 'should use any version')
t.equal(b, 'bar-fork@*', 'should use any version')
}

config.diff = ['bar', 'bar-fork']
Expand All @@ -973,8 +973,8 @@ t.test('first arg is an unknown dependency name', t => {

const path = t.testdir({})
libnpmdiff = async ([a, b], opts) => {
t.equal(a, 'bar@latest', 'should use latest tag')
t.equal(b, 'bar-fork@latest', 'should use latest tag')
t.equal(a, 'bar@*', 'should use any version')
t.equal(b, 'bar-fork@*', 'should use any version')
}

config.diff = ['bar', 'bar-fork']
Expand Down
2 changes: 1 addition & 1 deletion test/lib/commands/dist-tag.js
Expand Up @@ -277,7 +277,7 @@ t.test('workspaces', t => {
await distTag.execWorkspaces([], [])
t.equal(process.exitCode, 1, 'set the error status')
process.exitCode = 0
t.match(log, 'dist-tag ls Couldn\'t get dist-tag data for workspace-d@latest', 'logs the error')
t.match(log, 'dist-tag ls Couldn\'t get dist-tag data for workspace-d@*', 'logs the error')
t.matchSnapshot(result, 'printed the expected output')
})

Expand Down
10 changes: 5 additions & 5 deletions test/lib/commands/init.js
Expand Up @@ -85,7 +85,7 @@ t.test('npm init <arg>', async t => {
libnpmexec: ({ args, cache, npxCache }) => {
t.same(
args,
['create-react-app'],
['create-react-app@*'],
'should npx with listed packages'
)
t.same(cache, flatOptions.cache)
Expand All @@ -106,7 +106,7 @@ t.test('npm init <arg> -- other-args', async t => {
libnpmexec: ({ args }) => {
t.same(
args,
['create-react-app', 'my-path', '--some-option', 'some-value'],
['create-react-app@*', 'my-path', '--some-option', 'some-value'],
'should npm exec with expected args'
)
},
Expand All @@ -125,7 +125,7 @@ t.test('npm init @scope/name', async t => {
libnpmexec: ({ args }) => {
t.same(
args,
['@npmcli/create-something'],
['@npmcli/create-something@*'],
'should npx with scoped packages'
)
},
Expand Down Expand Up @@ -268,7 +268,7 @@ t.test('should not rewrite flatOptions', async t => {
libnpmexec: async ({ args }) => {
t.same(
args,
['create-react-app', 'my-app'],
['create-react-app@*', 'my-app'],
'should npx with extra args'
)
},
Expand Down Expand Up @@ -500,7 +500,7 @@ t.test('workspaces', t => {
libnpmexec: ({ args, path }) => {
t.same(
args,
['create-react-app'],
['create-react-app@*'],
'should npx with listed packages'
)
t.same(
Expand Down
4 changes: 2 additions & 2 deletions workspaces/arborist/lib/add-rm-pkg-deps.js
Expand Up @@ -35,8 +35,8 @@ const add = ({ pkg, add, saveBundle, saveType }) => {
const depType = saveTypeMap.get(addSaveType)

pkg[depType] = pkg[depType] || {}
if (rawSpec !== '' || pkg[depType][name] === undefined) {
pkg[depType][name] = rawSpec || '*'
if (rawSpec !== '*' || pkg[depType][name] === undefined) {
pkg[depType][name] = rawSpec
}
if (addSaveType === 'optional') {
// Affordance for previous npm versions that require this behaviour
Expand Down
7 changes: 3 additions & 4 deletions workspaces/arborist/lib/arborist/build-ideal-tree.js
Expand Up @@ -254,13 +254,12 @@ module.exports = cls => class IdealTreeBuilder extends cls {
for (const name of update.names) {
const spec = npa(name)
const validationError =
new TypeError(`Update arguments must not contain package version specifiers
Try using the package name instead, e.g:
new TypeError(`Update arguments must only contain package names, eg:
npm update ${spec.name}`)
validationError.code = 'EUPDATEARGS'

if (spec.fetchSpec !== 'latest') {
// If they gave us anything other than a bare package name
if (spec.raw !== spec.name) {
throw validationError
}
}
Expand Down
28 changes: 17 additions & 11 deletions workspaces/arborist/lib/consistent-resolve.js
Expand Up @@ -19,17 +19,23 @@ const consistentResolve = (resolved, fromPath, toPath, relPaths = false) => {
rawSpec,
raw,
} = npa(resolved, fromPath)
const isPath = type === 'file' || type === 'directory'
return isPath && !relPaths ? `file:${fetchSpec.replace(/#/g, '%23')}`
: isPath ? 'file:' + (toPath ? relpath(toPath, fetchSpec.replace(/#/g, '%23')) : fetchSpec.replace(/#/g, '%23'))
: hosted ? `git+${
hosted.auth ? hosted.https(hostedOpt) : hosted.sshurl(hostedOpt)
}`
: type === 'git' ? saveSpec
// always return something. 'foo' is interpreted as 'foo@' otherwise.
: rawSpec === '' && raw.slice(-1) !== '@' ? raw
// just strip off the name, but otherwise return as-is
: rawSpec
if (type === 'file' || type === 'directory') {
const cleanFetchSpec = fetchSpec.replace(/#/g, '%23')
if (relPaths && toPath) {
return `file:${relpath(toPath, cleanFetchSpec)}`
}
return `file:${cleanFetchSpec}`
}
if (hosted) {
return `git+${hosted.auth ? hosted.https(hostedOpt) : hosted.sshurl(hostedOpt)}`
}
if (type === 'git') {
return saveSpec
}
if (rawSpec === '*') {
return raw
}
return rawSpec
} catch (_) {
// whatever we passed in was not acceptable to npa.
// leave it 100% untouched.
Expand Down
2 changes: 1 addition & 1 deletion workspaces/arborist/lib/edge.js
Expand Up @@ -166,7 +166,7 @@ class Edge {
}

get spec () {
if (this.overrides && this.overrides.value && this.overrides.name === this.name) {
if (this.overrides?.value && this.overrides.value !== '*' && this.overrides.name === this.name) {
if (this.overrides.value.startsWith('$')) {
const ref = this.overrides.value.slice(1)
// we may be a virtual root, if we are we want to resolve reference overrides
Expand Down
11 changes: 4 additions & 7 deletions workspaces/arborist/lib/override-set.js
Expand Up @@ -25,7 +25,7 @@ class OverrideSet {
this.name = spec.name
spec.name = ''
this.key = key
this.keySpec = spec.rawSpec === '' ? '' : spec.toString()
this.keySpec = spec.toString()
this.value = overrides['.'] || this.keySpec
}

Expand All @@ -50,8 +50,7 @@ class OverrideSet {
continue
}

if (rule.keySpec === '' ||
semver.intersects(edge.spec, rule.keySpec)) {
if (semver.intersects(edge.spec, rule.keySpec)) {
return rule
}
}
Expand All @@ -65,8 +64,7 @@ class OverrideSet {
continue
}

if (rule.keySpec === '' ||
semver.satisfies(node.version, rule.keySpec) ||
if (semver.satisfies(node.version, rule.keySpec) ||
semver.satisfies(node.version, rule.value)) {
return rule
}
Expand All @@ -81,8 +79,7 @@ class OverrideSet {
continue
}

if (rule.keySpec === '' ||
semver.satisfies(node.version, rule.keySpec) ||
if (semver.satisfies(node.version, rule.keySpec) ||
semver.satisfies(node.version, rule.value)) {
return rule
}
Expand Down
8 changes: 4 additions & 4 deletions workspaces/arborist/lib/query-selector-all.js
Expand Up @@ -120,13 +120,13 @@ class Results {
this.#pendingCombinator = combinators[String(this.currentAstNode)]
}

// name selectors (i.e. #foo, #foo@1.0.0)
// name selectors (i.e. #foo)
// css calls this id, we interpret it as name
idType () {
const spec = npa(this.currentAstNode.value)
const name = this.currentAstNode.value
const nextResults = this.initialItems.filter(node =>
(node.name === spec.name || node.package.name === spec.name) &&
(semver.satisfies(node.version, spec.fetchSpec) || !spec.rawSpec))
(name === node.name) || (name === node.package.name)
)
this.processPendingCombinator(nextResults)
}

Expand Down
2 changes: 1 addition & 1 deletion workspaces/arborist/tap-snapshots/test/node.js.test.cjs
Expand Up @@ -32,7 +32,7 @@ exports[`test/node.js TAP basic instantiation > just a lone root node 1`] = `
"foo" => OverrideSet {
"children": Map {},
"key": "foo",
"keySpec": "",
"keySpec": "*",
"name": "foo",
"parent": <*ref_2>,
"value": "1",
Expand Down

0 comments on commit 1afe5ba

Please sign in to comment.