Skip to content
New issue

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

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

Already on GitHub? Sign in 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/[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
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')

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

Successfully merging this pull request may close these issues.

2 participants