fix: account for new npm-package-arg behavior

wraithgar · lukekarrys · commit 3f1fcf054276 · 2022-10-18T16:35:55.000-07:00
`npm`, `npm@`, and `npm@*` are all now the same spec
diff --git a/lib/commands/deprecate.js b/lib/commands/deprecate.js
@@ -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}`)
diff --git a/lib/commands/diff.js b/lib/commands/diff.js
@@ -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 =
@@ -269,7 +269,7 @@ class Diff extends BaseCommand {
 
     return specs.map(i => {
       const spec = npa(i)
-      if (spec.rawSpec) {
+      if (spec.rawSpec !== '*') {
         return i
       }
 
diff --git a/lib/commands/init.js b/lib/commands/init.js
@@ -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 +
diff --git a/lib/commands/rebuild.js b/lib/commands/rebuild.js
@@ -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
         }
 
diff --git a/lib/commands/view.js b/lib/commands/view.js
@@ -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}`)
diff --git a/tap-snapshots/test/lib/commands/dist-tag.js.test.cjs b/tap-snapshots/test/lib/commands/dist-tag.js.test.cjs
@@ -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@* 
 
 `
 
diff --git a/test/lib/commands/diff.js b/test/lib/commands/diff.js
@@ -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')
     }
@@ -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')
       },
     })
@@ -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')
     }
 
@@ -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')
     }
 
@@ -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']
@@ -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
@@ -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']
@@ -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')
@@ -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')}`,
@@ -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')
     }
 
@@ -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']
@@ -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']
diff --git a/test/lib/commands/dist-tag.js b/test/lib/commands/dist-tag.js
@@ -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')
   })
 
diff --git a/test/lib/commands/init.js b/test/lib/commands/init.js
@@ -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)
@@ -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'
       )
     },
@@ -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'
       )
     },
@@ -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'
       )
     },
@@ -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(
diff --git a/workspaces/arborist/lib/add-rm-pkg-deps.js b/workspaces/arborist/lib/add-rm-pkg-deps.js
@@ -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
diff --git a/workspaces/arborist/lib/arborist/build-ideal-tree.js b/workspaces/arborist/lib/arborist/build-ideal-tree.js
@@ -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
       }
     }
diff --git a/workspaces/arborist/lib/consistent-resolve.js b/workspaces/arborist/lib/consistent-resolve.js
@@ -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.
diff --git a/workspaces/arborist/lib/edge.js b/workspaces/arborist/lib/edge.js
@@ -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
diff --git a/workspaces/arborist/lib/override-set.js b/workspaces/arborist/lib/override-set.js
@@ -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
     }
 
@@ -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
       }
     }
@@ -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
       }
@@ -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
       }
diff --git a/workspaces/arborist/lib/query-selector-all.js b/workspaces/arborist/lib/query-selector-all.js
@@ -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)
   }
 
diff --git a/workspaces/arborist/tap-snapshots/test/node.js.test.cjs b/workspaces/arborist/tap-snapshots/test/node.js.test.cjs
@@ -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",
diff --git a/workspaces/arborist/test/consistent-resolve.js b/workspaces/arborist/test/consistent-resolve.js
diff --git a/workspaces/arborist/test/override-set.js b/workspaces/arborist/test/override-set.js
diff --git a/workspaces/libnpmexec/lib/index.js b/workspaces/libnpmexec/lib/index.js

Original file line number	Diff line number	Diff line change
`@@ -39,7 +39,7 @@ class Rebuild extends ArboristWorkspaceCmd {`
`39`	`39`	`const tree = await arb.loadActual()`
`40`	`40`	`const specs = args.map(arg => {`
`41`	`41`	`const spec = npa(arg)`
`42`		`- if (spec.type === 'tag' && spec.rawSpec === '') {`
	`42`	`+ if (spec.rawSpec === '*') {`
`43`	`43`	`return spec`
`44`	`44`	`}`
`45`	`45`
Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,7 @@ latest: 1.0.0`
`21`	`21`	`
`22`	`22`
`23`	`23`	exports[`test/lib/commands/dist-tag.js TAP ls on missing package > should log no dist-tag found msg 1`] = `
`24`		`-dist-tag ls Couldn't get dist-tag data for foo@latest`
	`24`	`+dist-tag ls Couldn't get dist-tag data for foo@*`
`25`	`25`
`26`	`26`	`
`27`	`27`