Fix: Fix error reading matches of null when install dependencies

[GiveMeSomething]

Prerequisite

• Dependencies are installed once with pnpm, creating .pnpm inside node_modules

What

• From my inspection, npm install is throwing error when trying to dedupe dependencies, using matches() function from workspace/lib/node.js to check if 2 dependencies are the same (safe to remove if they are the same)

• Error was thrown when the function try to resolve target from a dependencies inside .pnpm folder, which have non-existent path (please see trace log below)

• Error is thrown at function workspace/arborist/lib/node.js:1118

• Trace

# Error trace
5819 verbose stack TypeError: Cannot read properties of null (reading 'matches')
5819 verbose stack     at Link.matches (/opt/homebrew/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/node.js:1117:41)
5819 verbose stack     at Link.canDedupe (/opt/homebrew/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/node.js:1071:15)
5819 verbose stack     at PlaceDep.pruneDedupable (/opt/homebrew/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/place-dep.js:426:14)
5819 verbose stack     at new PlaceDep (/opt/homebrew/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/place-dep.js:278:14)
5819 verbose stack     at new PlaceDep (/opt/homebrew/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/place-dep.js:314:26)
5819 verbose stack     at #buildDepStep (/opt/homebrew/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/arborist/build-ideal-tree.js:917:18)
5819 verbose stack     at async Arborist.buildIdealTree (/opt/homebrew/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/arborist/build-ideal-tree.js:181:7)
5819 verbose stack     at async Promise.all (index 1)
5819 verbose stack     at async Arborist.reify (/opt/homebrew/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/arborist/reify.js:131:5)
5819 verbose stack     at async Install.exec (/opt/homebrew/lib/node_modules/npm/lib/commands/install.js:150:5)
5820 error Cannot read properties of null (reading 'matches')

# Debug trace
...
CONTEXT - START
this.target ArboristNode {
  name: '@jest/types',
  version: '29.6.3',
  location: 'node_modules/.pnpm/node_modules/@jest/types',
  path: '/Users/mint/code/cli/cv3-frontend/node_modules/.pnpm/node_modules/@jest/types',
  resolved: 'https://registry.npmjs.org/@jest/types/-/types-29.6.3.tgz',
  extraneous: true,
  dev: true,
  optional: true,
  peer: true,
  edgesOut: Map(6) {
    '@jest/schemas' => { prod @jest/schemas@^29.6.3 -> node_modules/@jest/schemas INVALID },
    '@types/istanbul-lib-coverage' => { prod @types/istanbul-lib-coverage@^2.0.0 MISSING },
    '@types/istanbul-reports' => { prod @types/istanbul-reports@^3.0.0 MISSING },
    '@types/node' => { prod @types/node@* -> node_modules/.pnpm/node_modules/@types/node },
    '@types/yargs' => { prod @types/yargs@^17.0.8 -> node_modules/.pnpm/node_modules/@types/yargs INVALID },
    'chalk' => { prod chalk@^4.0.0 -> node_modules/.pnpm/node_modules/chalk }
  },
  edgesIn: Set(2) {
    { node_modules/.pnpm/[email protected]/node_modules/jest-environment-node prod @jest/types@^29.6.3 },
    { node_modules/.pnpm/node_modules/ts-jest peerOptional @jest/types@^29.0.0 }
  }
}
node.target null
CONTEXT - END
...

CONTEXT - START
this.target null
node.target ArboristNode {
  name: 'esbuild',
  version: '0.20.2',
  location: 'node_modules/.pnpm/[email protected]/node_modules/esbuild',
  path: '/Users/mint/code/cli/cv3-frontend/node_modules/.pnpm/[email protected]/node_modules/esbuild',
  dev: true,
  edgesOut: Map(23) {
    '@esbuild/aix-ppc64' => { optional @esbuild/[email protected] },
    '@esbuild/android-arm' => { optional @esbuild/[email protected] },
    '@esbuild/android-arm64' => { optional @esbuild/[email protected] },
    '@esbuild/android-x64' => { optional @esbuild/[email protected] },
    '@esbuild/darwin-arm64' => { optional @esbuild/[email protected] -> node_modules/.pnpm/[email protected]/node_modules/@esbuild/darwin-arm64 },
    '@esbuild/darwin-x64' => { optional @esbuild/[email protected] },
    '@esbuild/freebsd-arm64' => { optional @esbuild/[email protected] },
    '@esbuild/freebsd-x64' => { optional @esbuild/[email protected] },
    '@esbuild/linux-arm' => { optional @esbuild/[email protected] },
    '@esbuild/linux-arm64' => { optional @esbuild/[email protected] },
    '@esbuild/linux-ia32' => { optional @esbuild/[email protected] },
    '@esbuild/linux-loong64' => { optional @esbuild/[email protected] },
    '@esbuild/linux-mips64el' => { optional @esbuild/[email protected] },
    '@esbuild/linux-ppc64' => { optional @esbuild/[email protected] },
    '@esbuild/linux-riscv64' => { optional @esbuild/[email protected] },
    '@esbuild/linux-s390x' => { optional @esbuild/[email protected] },
    '@esbuild/linux-x64' => { optional @esbuild/[email protected] },
    '@esbuild/netbsd-x64' => { optional @esbuild/[email protected] },
    '@esbuild/openbsd-x64' => { optional @esbuild/[email protected] },
    '@esbuild/sunos-x64' => { optional @esbuild/[email protected] },
    '@esbuild/win32-arm64' => { optional @esbuild/[email protected] },
    '@esbuild/win32-ia32' => { optional @esbuild/[email protected] },
    '@esbuild/win32-x64' => { optional @esbuild/[email protected] }
  }
}
CONTEXT - END

• I have added defensive null check for this.target and node.target
• [HELP NEEDED] Maybe we need to add warning for users using pnpm and ignore .pnpm folder when build dependencies tree

References

Related to issue #4367

[wraithgar]

This looks pretty similar to https://github.com/npm/cli/pull/7579. Is this a similar root cause?

In any event this will needs tests showing how to get into this broken state and show how npm now handles it.

[GiveMeSomething]

This looks pretty similar to #7579. Is this a similar root cause?

In any event this will needs tests showing how to get into this broken state and show how npm now handles it.

@wraithgar Sorry for my late response. I think they are related as both errors occur when Link target is null

Further investigation show that the target cannot be null when a Link is initialized, so I assume that setter functions might have set it to null.

• set root inside Node class does set target to null to "temporary break this link ..."
• set target inside Link class able to set target to null

I think we need to resolve this issue inside Link class because I just found another place that might break when a Link target is null

npm error Cannot read properties of null (reading 'isDescendantOf')