Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix: Fix error reading matches of null when install dependencies #7588

Open
wants to merge 2 commits into
base: latest
Choose a base branch
from

Conversation

GiveMeSomething
Copy link

@GiveMeSomething GiveMeSomething commented Jun 5, 2024

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/jest-environment-node@29.7.0/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/esbuild@0.20.2/node_modules/esbuild',
  path: '/Users/mint/code/cli/cv3-frontend/node_modules/.pnpm/esbuild@0.20.2/node_modules/esbuild',
  dev: true,
  edgesOut: Map(23) {
    '@esbuild/aix-ppc64' => { optional @esbuild/aix-ppc64@0.20.2 },
    '@esbuild/android-arm' => { optional @esbuild/android-arm@0.20.2 },
    '@esbuild/android-arm64' => { optional @esbuild/android-arm64@0.20.2 },
    '@esbuild/android-x64' => { optional @esbuild/android-x64@0.20.2 },
    '@esbuild/darwin-arm64' => { optional @esbuild/darwin-arm64@0.20.2 -> node_modules/.pnpm/esbuild@0.20.2/node_modules/@esbuild/darwin-arm64 },
    '@esbuild/darwin-x64' => { optional @esbuild/darwin-x64@0.20.2 },
    '@esbuild/freebsd-arm64' => { optional @esbuild/freebsd-arm64@0.20.2 },
    '@esbuild/freebsd-x64' => { optional @esbuild/freebsd-x64@0.20.2 },
    '@esbuild/linux-arm' => { optional @esbuild/linux-arm@0.20.2 },
    '@esbuild/linux-arm64' => { optional @esbuild/linux-arm64@0.20.2 },
    '@esbuild/linux-ia32' => { optional @esbuild/linux-ia32@0.20.2 },
    '@esbuild/linux-loong64' => { optional @esbuild/linux-loong64@0.20.2 },
    '@esbuild/linux-mips64el' => { optional @esbuild/linux-mips64el@0.20.2 },
    '@esbuild/linux-ppc64' => { optional @esbuild/linux-ppc64@0.20.2 },
    '@esbuild/linux-riscv64' => { optional @esbuild/linux-riscv64@0.20.2 },
    '@esbuild/linux-s390x' => { optional @esbuild/linux-s390x@0.20.2 },
    '@esbuild/linux-x64' => { optional @esbuild/linux-x64@0.20.2 },
    '@esbuild/netbsd-x64' => { optional @esbuild/netbsd-x64@0.20.2 },
    '@esbuild/openbsd-x64' => { optional @esbuild/openbsd-x64@0.20.2 },
    '@esbuild/sunos-x64' => { optional @esbuild/sunos-x64@0.20.2 },
    '@esbuild/win32-arm64' => { optional @esbuild/win32-arm64@0.20.2 },
    '@esbuild/win32-ia32' => { optional @esbuild/win32-ia32@0.20.2 },
    '@esbuild/win32-x64' => { optional @esbuild/win32-x64@0.20.2 }
  }
}
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
Copy link
Member

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.

@GiveMeSomething
Copy link
Author

GiveMeSomething commented Jun 16, 2024

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')

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants