[BUG] Cannot read properties of null (reading 'isWorkspace')

[ThisIsMissEm]

Is there an existing issue for this?

• I have searched the existing issues

This issue exists in the latest npm version

• I am using the latest npm

Current Behavior

When doing npm ls --include-workspace-root --workspace eslint-config-react @babel/polyfill in https://github.com/inrupt/javascript-style-configs I receive the following error:

38 verbose stack TypeError: Cannot read properties of null (reading 'isWorkspace')
38 verbose stack     at filterBySelectedWorkspaces (~/.fnm/node-versions/v16.13.1/installation/lib/node_modules/npm/lib/commands/ls.js:95:23)
38 verbose stack     at Array.filter (<anonymous>)
38 verbose stack     at getChildren (~/.fnm/node-versions/v16.13.1/installation/lib/node_modules/npm/lib/commands/ls.js:136:14)
38 verbose stack     at kidNodes (~/.fnm/node-versions/v16.13.1/installation/lib/node_modules/npm/node_modules/treeverse/lib/breadth.js:52:18)
38 verbose stack     at ~/.fnm/node-versions/v16.13.1/installation/lib/node_modules/npm/node_modules/treeverse/lib/breadth.js:41:16

I believe this may be related to #5007 and the fix described in the latest comment.

Expected Behavior

npm ls should find the given package without error, if the package is used.

Steps To Reproduce

See above, using npm 8.13.x and node 16.13.x

Environment

• npm: 8.13.2
• Node.js: 16.13.1
• OS Name: MacOS latest
• System Model Name: MacOS
• npm config: no additional npm config.

[ThisIsMissEm]

There's also similar code to this comment in the ls command:

        return (shouldSkipChildren)
          ? []
          : [...(node.target).edgesOut.values()]
            .filter(filterBySelectedWorkspaces)

So I'm guessing that treeverse or Arborist is returning null instead of an array in certain conditions, which leads to an evaluation of null property access.

[ThisIsMissEm]

I added in some logging to the ls command, and managed to get this out:

const filterBySelectedWorkspaces = edge => {
      console.log({edge: edge, spec: edge.spec, to: edge.to?.location});

ArboristNode {
  name: 'jest',
  version: '28.1.1',
  location: 'node_modules/jest',
  path: '[trunc]/javascript-style-configs/node_modules/jest',
  edgesOut: Map(5) {
    '@jest/core' => { prod @jest/core@^28.1.1 -> node_modules/@jest/core },
    '@jest/types' => { prod @jest/types@^28.1.1 -> node_modules/@jest/types },
    'import-local' => { prod import-local@^3.0.2 -> node_modules/import-local },
    'jest-cli' => { prod jest-cli@^28.1.1 -> node_modules/jest-cli },
    'node-notifier' => { peerOptional node-notifier@^8.0.1 || ^9.0.0 || ^10.0.0 }
  },
  edgesIn: Set(1) { { eslint-config-base prod jest@>=26.0.0 } }
}
{
  edge: ArboristEdge {
    name: 'node-notifier',
    spec: '^8.0.1 || ^9.0.0 || ^10.0.0',
    type: 'peerOptional',
    from: 'node_modules/jest'
  },
  spec: '^8.0.1 || ^9.0.0 || ^10.0.0',
  to: undefined
}

before it crashes. So it looks like edge.to can be undefined or null, and the ls code doesn't seem to handle this case.

In my case, I don't believe the peerDep of node-notifier is installed

[ThisIsMissEm]

Update: I installed node-notifier manually to the root workspace, and then re-ran things, and it crashed at another peerDep, so it's definitely not liking peer dependencies that aren't installed.

[nlf]

the condition in lib/commands/ls.js is wrong, i opened #5164 that fixes the bug but it won't get landed until i can get a regression test in place. i'll probably work on that tomorrow.

[ThisIsMissEm]

@nlf thanks! no real rush on getting this fixed, as for now it's a minor annoyance in our multi-package repo (it's one we don't need to touch much, thankfully)

[nlf]

test and fix were landed, this is fixed in [email protected]