fix: stop working around bug fixed in npm-package-arg@12.0.2

TrevorBurnham · wraithgar · commit 811ca2927eed · 2025-02-18T10:55:43.000-08:00
Removes all usage of `.replace(/#/g, '%23')` for compatibility with the new version This is the code changes from #8112 isloated from the dependency update itself. Closes: npm/npm-package-arg#203 Credit: @TrevorBurnham
diff --git a/lib/commands/diff.js b/lib/commands/diff.js
@@ -106,7 +106,7 @@ class Diff extends BaseCommand {
       const pkgName = await this.packageName()
       return [
         `${pkgName}@${this.npm.config.get('tag')}`,
-        `file:${this.prefix.replace(/#/g, '%23')}`,
+        `file:${this.prefix}`,
       ]
     }
 
@@ -134,7 +134,7 @@ class Diff extends BaseCommand {
       }
       return [
         `${pkgName}@${a}`,
-        `file:${this.prefix.replace(/#/g, '%23')}`,
+        `file:${this.prefix}`,
       ]
     }
 
@@ -166,7 +166,7 @@ class Diff extends BaseCommand {
         }
         return [
           `${spec.name}@${spec.fetchSpec}`,
-          `file:${this.prefix.replace(/#/g, '%23')}`,
+          `file:${this.prefix}`,
         ]
       }
 
@@ -179,7 +179,7 @@ class Diff extends BaseCommand {
         }
       }
 
-      const aSpec = `file:${node.realpath.replace(/#/g, '%23')}`
+      const aSpec = `file:${node.realpath}`
 
       // finds what version of the package to compare against, if a exact
       // version or tag was passed than it should use that, otherwise
@@ -212,8 +212,8 @@ class Diff extends BaseCommand {
       ]
     } else if (spec.type === 'directory') {
       return [
-        `file:${spec.fetchSpec.replace(/#/g, '%23')}`,
-        `file:${this.prefix.replace(/#/g, '%23')}`,
+        `file:${spec.fetchSpec}`,
+        `file:${this.prefix}`,
       ]
     } else {
       throw this.usageError(`Spec type ${spec.type} not supported.`)
@@ -281,7 +281,7 @@ class Diff extends BaseCommand {
 
       const res = !node || !node.package || !node.package.version
         ? spec.fetchSpec
-        : `file:${node.realpath.replace(/#/g, '%23')}`
+        : `file:${node.realpath}`
 
       return `${spec.name}@${res}`
     })
diff --git a/lib/commands/link.js b/lib/commands/link.js
@@ -124,7 +124,7 @@ class Link extends ArboristWorkspaceCmd {
       ...this.npm.flatOptions,
       prune: false,
       path: this.npm.prefix,
-      add: names.map(l => `file:${resolve(globalTop, 'node_modules', l).replace(/#/g, '%23')}`),
+      add: names.map(l => `file:${resolve(globalTop, 'node_modules', l)}`),
       save,
       workspaces: this.workspaceNames,
     })
@@ -135,7 +135,7 @@ class Link extends ArboristWorkspaceCmd {
   async linkPkg () {
     const wsp = this.workspacePaths
     const paths = wsp && wsp.length ? wsp : [this.npm.prefix]
-    const add = paths.map(path => `file:${path.replace(/#/g, '%23')}`)
+    const add = paths.map(path => `file:${path}`)
     const globalTop = resolve(this.npm.globalDir, '..')
     const Arborist = require('@npmcli/arborist')
     const arb = new Arborist({
diff --git a/workspaces/arborist/lib/arborist/build-ideal-tree.js b/workspaces/arborist/lib/arborist/build-ideal-tree.js
@@ -447,7 +447,7 @@ module.exports = cls => class IdealTreeBuilder extends cls {
             .catch(/* istanbul ignore next */ () => null)
           if (st && st.isSymbolicLink()) {
             const target = await readlink(dir)
-            const real = resolve(dirname(dir), target).replace(/#/g, '%23')
+            const real = resolve(dirname(dir), target)
             tree.package.dependencies[name] = `file:${real}`
           } else {
             tree.package.dependencies[name] = '*'
@@ -522,12 +522,12 @@ module.exports = cls => class IdealTreeBuilder extends cls {
 
       const { name } = spec
       if (spec.type === 'file') {
-        spec = npa(`file:${relpath(path, spec.fetchSpec).replace(/#/g, '%23')}`, path)
+        spec = npa(`file:${relpath(path, spec.fetchSpec)}`, path)
         spec.name = name
       } else if (spec.type === 'directory') {
         try {
           const real = await realpath(spec.fetchSpec, this[_rpcache], this[_stcache])
-          spec = npa(`file:${relpath(path, real).replace(/#/g, '%23')}`, path)
+          spec = npa(`file:${relpath(path, real)}`, path)
           spec.name = name
         } catch {
           // TODO: create synthetic test case to simulate realpath failure
diff --git a/workspaces/arborist/lib/arborist/load-actual.js b/workspaces/arborist/lib/arborist/load-actual.js
@@ -216,7 +216,7 @@ module.exports = cls => class ActualLoader extends cls {
       const actualRoot = tree.isLink ? tree.target : tree
       const { dependencies = {} } = actualRoot.package
       for (const [name, kid] of actualRoot.children.entries()) {
-        const def = kid.isLink ? `file:${kid.realpath.replace(/#/g, '%23')}` : '*'
+        const def = kid.isLink ? `file:${kid.realpath}` : '*'
         dependencies[name] = dependencies[name] || def
       }
       actualRoot.package = { ...actualRoot.package, dependencies }
diff --git a/workspaces/arborist/lib/arborist/load-virtual.js b/workspaces/arborist/lib/arborist/load-virtual.js
@@ -149,7 +149,7 @@ module.exports = cls => class VirtualLoader extends cls {
     })
 
     for (const [name, path] of workspaces.entries()) {
-      lockWS[name] = `file:${path.replace(/#/g, '%23')}`
+      lockWS[name] = `file:${path}`
     }
 
     // Should rootNames exclude optional?
diff --git a/workspaces/arborist/lib/arborist/reify.js b/workspaces/arborist/lib/arborist/reify.js
@@ -1364,7 +1364,7 @@ module.exports = cls => class Reifier extends cls {
             // path initially, in which case we can end up with the wrong
             // thing, so just get the ultimate fetchSpec and relativize it.
             const p = req.fetchSpec.replace(/^file:/, '')
-            const rel = relpath(addTree.realpath, p).replace(/#/g, '%23')
+            const rel = relpath(addTree.realpath, p)
             newSpec = `file:${rel}`
           }
         } else {
diff --git a/workspaces/arborist/lib/consistent-resolve.js b/workspaces/arborist/lib/consistent-resolve.js
@@ -20,7 +20,7 @@ const consistentResolve = (resolved, fromPath, toPath, relPaths = false) => {
       raw,
     } = npa(resolved, fromPath)
     if (type === 'file' || type === 'directory') {
-      const cleanFetchSpec = fetchSpec.replace(/#/g, '%23')
+      const cleanFetchSpec = fetchSpec
       if (relPaths && toPath) {
         return `file:${relpath(toPath, cleanFetchSpec)}`
       }
diff --git a/workspaces/arborist/lib/link.js b/workspaces/arborist/lib/link.js
@@ -99,7 +99,7 @@ class Link extends Node {
     // the path/realpath guard is there for the benefit of setting
     // these things in the "wrong" order
     return this.path && this.realpath
-      ? `file:${relpath(dirname(this.path), this.realpath).replace(/#/g, '%23')}`
+      ? `file:${relpath(dirname(this.path), this.realpath)}`
       : null
   }
 
diff --git a/workspaces/arborist/lib/node.js b/workspaces/arborist/lib/node.js
@@ -842,7 +842,7 @@ class Node {
     }
 
     for (const [name, path] of this.#workspaces.entries()) {
-      new Edge({ from: this, name, spec: `file:${path.replace(/#/g, '%23')}`, type: 'workspace' })
+      new Edge({ from: this, name, spec: `file:${path}`, type: 'workspace' })
     }
   }
 
diff --git a/workspaces/arborist/lib/shrinkwrap.js b/workspaces/arborist/lib/shrinkwrap.js
@@ -817,7 +817,7 @@ class Shrinkwrap {
         if (!/^file:/.test(resolved)) {
           pathFixed = resolved
         } else {
-          pathFixed = `file:${resolve(this.path, resolved.slice(5)).replace(/#/g, '%23')}`
+          pathFixed = `file:${resolve(this.path, resolved.slice(5))}`
         }
       }
 
@@ -1011,7 +1011,7 @@ class Shrinkwrap {
     }
 
     if (node.isLink) {
-      lock.version = `file:${relpath(this.path, node.realpath).replace(/#/g, '%23')}`
+      lock.version = `file:${relpath(this.path, node.realpath)}`
     } else if (spec && (spec.type === 'file' || spec.type === 'remote')) {
       lock.version = spec.saveSpec
     } else if (spec && spec.type === 'git' || rSpec.type === 'git') {
@@ -1089,7 +1089,7 @@ class Shrinkwrap {
             // this especially shows up with workspace edges when the root
             // node is also a workspace in the set.
             const p = resolve(node.realpath, spec.slice('file:'.length))
-            set[k] = `file:${relpath(node.realpath, p).replace(/#/g, '%23')}`
+            set[k] = `file:${relpath(node.realpath, p)}`
           } else {
             set[k] = spec
           }

Original file line number	Diff line number	Diff line change
`@@ -106,7 +106,7 @@ class Diff extends BaseCommand {`
`106`	`106`	`const pkgName = await this.packageName()`
`107`	`107`	`return [`
`108`	`108`	`${pkgName}@${this.npm.config.get('tag')}`,
`109`		- `file:${this.prefix.replace(/#/g, '%23')}`,
	`109`	+ `file:${this.prefix}`,
`110`	`110`	`]`
`111`	`111`	`}`
`112`	`112`
`@@ -134,7 +134,7 @@ class Diff extends BaseCommand {`
`134`	`134`	`}`
`135`	`135`	`return [`
`136`	`136`	`${pkgName}@${a}`,
`137`		- `file:${this.prefix.replace(/#/g, '%23')}`,
	`137`	+ `file:${this.prefix}`,
`138`	`138`	`]`
`139`	`139`	`}`
`140`	`140`
`@@ -166,7 +166,7 @@ class Diff extends BaseCommand {`
`166`	`166`	`}`
`167`	`167`	`return [`
`168`	`168`	`${spec.name}@${spec.fetchSpec}`,
`169`		- `file:${this.prefix.replace(/#/g, '%23')}`,
	`169`	+ `file:${this.prefix}`,
`170`	`170`	`]`
`171`	`171`	`}`
`172`	`172`
`@@ -179,7 +179,7 @@ class Diff extends BaseCommand {`
`179`	`179`	`}`
`180`	`180`	`}`
`181`	`181`
`182`		- const aSpec = `file:${node.realpath.replace(/#/g, '%23')}`
	`182`	+ const aSpec = `file:${node.realpath}`
`183`	`183`
`184`	`184`	`// finds what version of the package to compare against, if a exact`
`185`	`185`	`// version or tag was passed than it should use that, otherwise`
`@@ -212,8 +212,8 @@ class Diff extends BaseCommand {`
`212`	`212`	`]`
`213`	`213`	`} else if (spec.type === 'directory') {`
`214`	`214`	`return [`
`215`		- `file:${spec.fetchSpec.replace(/#/g, '%23')}`,
`216`		- `file:${this.prefix.replace(/#/g, '%23')}`,
	`215`	+ `file:${spec.fetchSpec}`,
	`216`	+ `file:${this.prefix}`,
`217`	`217`	`]`
`218`	`218`	`} else {`
`219`	`219`	throw this.usageError(`Spec type ${spec.type} not supported.`)
`@@ -281,7 +281,7 @@ class Diff extends BaseCommand {`
`281`	`281`
`282`	`282`	`const res = !node \|\| !node.package \|\| !node.package.version`
`283`	`283`	`? spec.fetchSpec`
`284`		- : `file:${node.realpath.replace(/#/g, '%23')}`
	`284`	+ : `file:${node.realpath}`
`285`	`285`
`286`	`286`	return `${spec.name}@${res}`
`287`	`287`	`})`
Original file line number	Diff line number	Diff line change
`@@ -216,7 +216,7 @@ module.exports = cls => class ActualLoader extends cls {`
`216`	`216`	`const actualRoot = tree.isLink ? tree.target : tree`
`217`	`217`	`const { dependencies = {} } = actualRoot.package`
`218`	`218`	`for (const [name, kid] of actualRoot.children.entries()) {`
`219`		- const def = kid.isLink ? `file:${kid.realpath.replace(/#/g, '%23')}` : '*'
	`219`	+ const def = kid.isLink ? `file:${kid.realpath}` : '*'
`220`	`220`	`dependencies[name] = dependencies[name] \|\| def`
`221`	`221`	`}`
`222`	`222`	`actualRoot.package = { ...actualRoot.package, dependencies }`
Original file line number	Diff line number	Diff line change
`@@ -149,7 +149,7 @@ module.exports = cls => class VirtualLoader extends cls {`
`149`	`149`	`})`
`150`	`150`
`151`	`151`	`for (const [name, path] of workspaces.entries()) {`
`152`		- lockWS[name] = `file:${path.replace(/#/g, '%23')}`
	`152`	+ lockWS[name] = `file:${path}`
`153`	`153`	`}`
`154`	`154`
`155`	`155`	`// Should rootNames exclude optional?`
Original file line number	Diff line number	Diff line change
`@@ -1364,7 +1364,7 @@ module.exports = cls => class Reifier extends cls {`
`1364`	`1364`	`// path initially, in which case we can end up with the wrong`
`1365`	`1365`	`// thing, so just get the ultimate fetchSpec and relativize it.`
`1366`	`1366`	`const p = req.fetchSpec.replace(/^file:/, '')`
`1367`		`- const rel = relpath(addTree.realpath, p).replace(/#/g, '%23')`
	`1367`	`+ const rel = relpath(addTree.realpath, p)`
`1368`	`1368`	newSpec = `file:${rel}`
`1369`	`1369`	`}`
`1370`	`1370`	`} else {`
Original file line number	Diff line number	Diff line change
`@@ -20,7 +20,7 @@ const consistentResolve = (resolved, fromPath, toPath, relPaths = false) => {`
`20`	`20`	`raw,`
`21`	`21`	`} = npa(resolved, fromPath)`
`22`	`22`	`if (type === 'file' \|\| type === 'directory') {`
`23`		`- const cleanFetchSpec = fetchSpec.replace(/#/g, '%23')`
	`23`	`+ const cleanFetchSpec = fetchSpec`
`24`	`24`	`if (relPaths && toPath) {`
`25`	`25`	return `file:${relpath(toPath, cleanFetchSpec)}`
`26`	`26`	`}`
Original file line number	Diff line number	Diff line change
`@@ -99,7 +99,7 @@ class Link extends Node {`
`99`	`99`	`// the path/realpath guard is there for the benefit of setting`
`100`	`100`	`// these things in the "wrong" order`
`101`	`101`	`return this.path && this.realpath`
`102`		- ? `file:${relpath(dirname(this.path), this.realpath).replace(/#/g, '%23')}`
	`102`	+ ? `file:${relpath(dirname(this.path), this.realpath)}`
`103`	`103`	`: null`
`104`	`104`	`}`
`105`	`105`
Original file line number	Diff line number	Diff line change
`@@ -842,7 +842,7 @@ class Node {`
`842`	`842`	`}`
`843`	`843`
`844`	`844`	`for (const [name, path] of this.#workspaces.entries()) {`
`845`		- new Edge({ from: this, name, spec: `file:${path.replace(/#/g, '%23')}`, type: 'workspace' })
	`845`	+ new Edge({ from: this, name, spec: `file:${path}`, type: 'workspace' })
`846`	`846`	`}`
`847`	`847`	`}`
`848`	`848`
Original file line number	Diff line number	Diff line change
`@@ -817,7 +817,7 @@ class Shrinkwrap {`
`817`	`817`	`if (!/^file:/.test(resolved)) {`
`818`	`818`	`pathFixed = resolved`
`819`	`819`	`} else {`
`820`		- pathFixed = `file:${resolve(this.path, resolved.slice(5)).replace(/#/g, '%23')}`
	`820`	+ pathFixed = `file:${resolve(this.path, resolved.slice(5))}`
`821`	`821`	`}`
`822`	`822`	`}`
`823`	`823`
`@@ -1011,7 +1011,7 @@ class Shrinkwrap {`
`1011`	`1011`	`}`
`1012`	`1012`
`1013`	`1013`	`if (node.isLink) {`
`1014`		- lock.version = `file:${relpath(this.path, node.realpath).replace(/#/g, '%23')}`
	`1014`	+ lock.version = `file:${relpath(this.path, node.realpath)}`
`1015`	`1015`	`} else if (spec && (spec.type === 'file' \|\| spec.type === 'remote')) {`
`1016`	`1016`	`lock.version = spec.saveSpec`
`1017`	`1017`	`} else if (spec && spec.type === 'git' \|\| rSpec.type === 'git') {`
`@@ -1089,7 +1089,7 @@ class Shrinkwrap {`
`1089`	`1089`	`// this especially shows up with workspace edges when the root`
`1090`	`1090`	`// node is also a workspace in the set.`
`1091`	`1091`	`const p = resolve(node.realpath, spec.slice('file:'.length))`
`1092`		- set[k] = `file:${relpath(node.realpath, p).replace(/#/g, '%23')}`
	`1092`	+ set[k] = `file:${relpath(node.realpath, p)}`
`1093`	`1093`	`} else {`
`1094`	`1094`	`set[k] = spec`
`1095`	`1095`	`}`