Update npm overrides patch based on npm/cli#8089

Author: jdalton

src/commands/optimize/apply-optimization.ts

@@ -130,7 +130,7 @@ export async function applyOptimization(
 
   if (agent === NPM || pkgJsonChanged) {
     // Always update package-lock.json until the npm overrides PR lands:
-    // https://github.com/npm/cli/pull/7025
+    // https://github.com/npm/cli/pull/8089
     await updatePackageLockJson(lockName, agentExecPath, agent, spinner)
   }
 }

src/commands/optimize/update-package-lock-json.ts

@@ -10,7 +10,7 @@ import type { Agent } from '../../utils/package-manager-detector'
 const { NPM, SOCKET_CLI_SAFE_WRAPPER, abortSignal } = constants
 
 const COMMAND_TITLE = 'Socket Optimize'
-const NPM_OVERRIDE_PR_URL = 'https://github.com/npm/cli/pull/7025'
+const NPM_OVERRIDE_PR_URL = 'https://github.com/npm/cli/pull/8089'
 
 export async function updatePackageLockJson(
   lockName: string,

src/shadow/arborist/index.ts

@@ -11,7 +11,7 @@ import {
 
 export function installSafeArborist() {
   // Override '@npmcli/arborist' module exports with patched variants based on
-  // https://github.com/npm/cli/pull/7025.
+  // https://github.com/npm/cli/pull/8089.
   const cache: { [key: string]: any } = require.cache
   cache[getArboristClassPath()] = { exports: SafeArborist }
   cache[getArboristEdgeClassPath()] = { exports: SafeEdge }

src/shadow/arborist/lib/edge.ts

@@ -61,7 +61,7 @@ export const Edge: EdgeClass = require(getArboristEdgeClassPath())
 // The Edge class makes heavy use of private properties which subclasses do NOT
 // have access to. So we have to recreate any functionality that relies on those
 // private properties and use our own "safe" prefixed non-conflicting private
-// properties. Implementation code not related to patch https://github.com/npm/cli/pull/7025
+// properties. Implementation code not related to patch https://github.com/npm/cli/pull/8089
 // is based on https://github.com/npm/cli/blob/v11.0.0/workspaces/arborist/lib/edge.js.
 //
 // The npm application
@@ -70,39 +70,26 @@ export const Edge: EdgeClass = require(getArboristEdgeClassPath())
 //
 // An edge in the dependency graph.
 // Represents a dependency relationship of some kind.
-const initializedSafeEdges = new WeakSet()
-
 export class SafeEdge extends Edge {
-  #safeAccept: string | undefined
   #safeError: ErrorStatus | null
   #safeExplanation: Explanation | undefined
   #safeFrom: SafeNode | null
-  #safeName: string
   #safeTo: SafeNode | null
 
   constructor(options: EdgeOptions) {
-    const { accept, from, name } = options
+    const { from } = options
     // Defer to supper to validate options and assign non-private values.
     super(options)
-    if (accept !== undefined) {
-      this.#safeAccept = accept || '*'
-    }
     if (from.constructor !== SafeNode) {
       Reflect.setPrototypeOf(from, SafeNode.prototype)
     }
     this.#safeError = null
     this.#safeExplanation = null
     this.#safeFrom = from
-    this.#safeName = name
     this.#safeTo = null
-    initializedSafeEdges.add(this)
     this.reload(true)
   }
 
-  override get accept() {
-    return this.#safeAccept
-  }
-
   override get bundled() {
     return !!this.#safeFrom?.package?.bundleDependencies?.includes(this.name)
   }
@@ -118,14 +105,16 @@ export class SafeEdge extends Edge {
       } else if (
         this.peer &&
         this.#safeFrom === this.#safeTo.parent &&
+        // Patch adding "?." use based on
+        // https://github.com/npm/cli/pull/8089.
         !this.#safeFrom?.isTop
       ) {
         this.#safeError = 'PEER LOCAL'
       } else if (!this.satisfiedBy(this.#safeTo)) {
         this.#safeError = 'INVALID'
       }
       // Patch adding "else if" condition is based on
-      // https://github.com/npm/cli/pull/7025.
+      // https://github.com/npm/cli/pull/8089.
       else if (
         this.overrides &&
         this.#safeTo.edgesOut.size &&
@@ -162,21 +151,15 @@ export class SafeEdge extends Edge {
       this.overrides.value !== '*' &&
       this.overrides.name === this.name
     ) {
-      // Patch adding "if" condition is based on
-      // https://github.com/npm/cli/pull/7025.
-      //
-      // If this edge has the same overrides field as the source, then we're not
-      // applying an override for this edge.
-      if (this.overrides === this.#safeFrom?.overrides) {
-        // The Edge rawSpec getter will retrieve the private Edge #spec property.
-        return this.rawSpec
-      }
       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 from the real root, not the virtual one.
+        //
+        // Patch adding "?." use based on
+        // https://github.com/npm/cli/pull/8089.
         const pkg = this.#safeFrom?.sourceReference
-          ? this.#safeFrom.sourceReference.root.package
+          ? this.#safeFrom?.sourceReference.root.package
           : this.#safeFrom?.root?.package
         if (pkg?.devDependencies?.[ref]) {
           return <string>pkg.devDependencies[ref]
@@ -205,10 +188,11 @@ export class SafeEdge extends Edge {
   override detach() {
     this.#safeExplanation = null
     // Patch replacing
-    // if (this.#safeTo) {
-    //   this.#safeTo.edgesIn.delete(this)
+    // if (this.#to) {
+    //   this.#to.edgesIn.delete(this)
     // }
-    // is based on https://github.com/npm/cli/pull/7025.
+    // this.#from.edgesOut.delete(this.#name)
+    // is based on https://github.com/npm/cli/pull/8089.
     this.#safeTo?.deleteEdgeIn(this)
     this.#safeFrom?.edgesOut.delete(this.name)
     this.#safeTo = null
@@ -249,38 +233,34 @@ export class SafeEdge extends Edge {
   }
 
   override reload(hard = false) {
-    if (!initializedSafeEdges.has(this)) {
-      // Skip if called during super constructor.
-      return
-    }
     this.#safeExplanation = null
-    // Patch adding newOverrideSet and oldOverrideSet is based on
-    // https://github.com/npm/cli/pull/7025.
+    // Patch replacing
+    // if (this.#from.overrides) {
+    // is based on https://github.com/npm/cli/pull/8089.
+    let needToUpdateOverrideSet = false
     let newOverrideSet
     let oldOverrideSet
     if (this.#safeFrom?.overrides) {
-      // Patch replacing
-      // this.overrides = this.#safeFrom.overrides.getEdgeRule(this)
-      // is based on https://github.com/npm/cli/pull/7025.
-      const newOverrideSet = this.#safeFrom.overrides.getEdgeRule(this)
+      newOverrideSet = this.#safeFrom.overrides.getEdgeRule(this)
       if (newOverrideSet && !newOverrideSet.isEqual(this.overrides)) {
         // If there's a new different override set we need to propagate it to
         // the nodes. If we're deleting the override set then there's no point
         // propagating it right now since it will be filled with another value
         // later.
+        needToUpdateOverrideSet = true
         oldOverrideSet = this.overrides
         this.overrides = newOverrideSet
       }
     } else {
       this.overrides = undefined
     }
+    // Patch adding "?." use based on
+    // https://github.com/npm/cli/pull/8089.
     const newTo = this.#safeFrom?.resolve(this.name)
     if (newTo !== this.#safeTo) {
       // Patch replacing
-      // if (this.#safeTo) {
-      //   this.#safeTo.edgesIn.delete(this)
-      // }
-      // is based on https://github.com/npm/cli/pull/7025.
+      // this.#to.edgesIn.delete(this)
+      // is based on https://github.com/npm/cli/pull/8089.
       this.#safeTo?.deleteEdgeIn(this)
       this.#safeTo = <SafeNode>newTo ?? null
       this.#safeError = null
@@ -289,10 +269,10 @@ export class SafeEdge extends Edge {
       this.#safeError = null
     }
     // Patch adding "else if" condition based on
-    // https://github.com/npm/cli/pull/7025.
-    else if (oldOverrideSet) {
+    // https://github.com/npm/cli/pull/8089.
+    else if (needToUpdateOverrideSet && this.#safeTo) {
       // Propagate the new override set to the target node.
-      this.#safeTo.updateOverridesEdgeInRemoved(oldOverrideSet)
+      this.#safeTo.updateOverridesEdgeInRemoved(oldOverrideSet!)
       this.#safeTo.updateOverridesEdgeInAdded(newOverrideSet)
     }
   }
@@ -302,46 +282,42 @@ export class SafeEdge extends Edge {
     // if (node.name !== this.#name) {
     //   return false
     // }
-    // is based on https://github.com/npm/cli/pull/7025.
-    if (node.name !== this.#safeName || !this.#safeFrom) {
+    // is based on https://github.com/npm/cli/pull/8089.
+    if (node.name !== this.name || !this.#safeFrom) {
       return false
     }
     // NOTE: this condition means we explicitly do not support overriding
     // bundled or shrinkwrapped dependencies
     if (node.hasShrinkwrap || node.inShrinkwrap || node.inBundle) {
-      return depValid(node, this.rawSpec, this.#safeAccept, this.#safeFrom)
+      return depValid(node, this.rawSpec, this.accept, this.#safeFrom)
     }
     // Patch replacing
     // return depValid(node, this.spec, this.#accept, this.#from)
-    // is based on https://github.com/npm/cli/pull/7025.
+    // is based on https://github.com/npm/cli/pull/8089.
     //
     // If there's no override we just use the spec.
     if (!this.overrides?.keySpec) {
-      return depValid(node, this.spec, this.#safeAccept, this.#safeFrom)
+      return depValid(node, this.spec, this.accept, this.#safeFrom)
     }
     // There's some override. If the target node satisfies the overriding spec
     // then it's okay.
-    if (depValid(node, this.spec, this.#safeAccept, this.#safeFrom)) {
+    if (depValid(node, this.spec, this.accept, this.#safeFrom)) {
       return true
     }
     // If it doesn't, then it should at least satisfy the original spec.
-    if (!depValid(node, this.rawSpec, this.#safeAccept, this.#safeFrom)) {
+    if (!depValid(node, this.rawSpec, this.accept, this.#safeFrom)) {
       return false
     }
     // It satisfies the original spec, not the overriding spec. We need to make
     // sure it doesn't use the overridden spec.
-    // For example, we might have an ^8.0.0 rawSpec, and an override that makes
-    // keySpec=8.23.0 and the override value spec=9.0.0.
-    // If the node is 9.0.0, then it's okay because it's consistent with spec.
-    // If the node is 8.24.0, then it's okay because it's consistent with the rawSpec.
-    // If the node is 8.23.0, then it's not okay because even though it's consistent
-    // with the rawSpec, it's also consistent with the keySpec.
-    // So we're looking for ^8.0.0 or 9.0.0 and not 8.23.0.
-    return !depValid(
-      node,
-      this.overrides.keySpec,
-      this.#safeAccept,
-      this.#safeFrom
-    )
+    // For example:
+    //   we might have an ^8.0.0 rawSpec, and an override that makes
+    //   keySpec=8.23.0 and the override value spec=9.0.0.
+    //   If the node is 9.0.0, then it's okay because it's consistent with spec.
+    //   If the node is 8.24.0, then it's okay because it's consistent with the rawSpec.
+    //   If the node is 8.23.0, then it's not okay because even though it's consistent
+    //   with the rawSpec, it's also consistent with the keySpec.
+    //   So we're looking for ^8.0.0 or 9.0.0 and not 8.23.0.
+    return !depValid(node, this.overrides.keySpec, this.accept, this.#safeFrom)
   }
 }