Using "ignore" lead to UnhandledPromiseRejection

[SDemonUA]

I've got an error "UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'toLowerCase' of undefined" trying to use "ignore" filter. (licensee\index.js:327:17)

It caused by package node_modules\\@svgr\\webpack\\node_modules\\@babel\\plugin-proposal-private-methods. This package results in empty object inside tree.package in resultForPackage func.

[ljharb]

Can you provide the entire stack trace?

[SDemonUA]

Sure

    at startsWith (C:\Users\sokol\AppData\Roaming\npm-cache\_npx\12424\node_modules\licensee\index.js:325:17)
    at C:\Users\sokol\AppData\Roaming\npm-cache\_npx\12424\node_modules\licensee\index.js:268:9
    at Array.some (<anonymous>)
    at resultForPackage (C:\Users\sokol\AppData\Roaming\npm-cache\_npx\12424\node_modules\licensee\index.js:258:26)
    at C:\Users\sokol\AppData\Roaming\npm-cache\_npx\12424\node_modules\licensee\index.js:179:22
    at Array.forEach (<anonymous>)
    at findIssues (C:\Users\sokol\AppData\Roaming\npm-cache\_npx\12424\node_modules\licensee\index.js:174:14)
    at C:\Users\sokol\AppData\Roaming\npm-cache\_npx\12424\node_modules\licensee\index.js:199:9
    at Array.forEach (<anonymous>)
    at findIssues (C:\Users\sokol\AppData\Roaming\npm-cache\_npx\12424\node_modules\licensee\index.js:174:14)

[SDemonUA]

One more thing, I work on Windows PC, but my colleague on Mac PC does not have this issue

[kemitchell]

Line 268 accesses the name property of a package metadata object.

https://github.com/jslicense/licensee.js/blob/62f4548d4463672d3e749f61ae4eaccaa0042c11/index.js#L268

Is there a package.json file in your node_modules without a name property?

[SDemonUA]

Actually, I have some modules w/o package.json at all

[ljharb]

The .bin folder isn’t a package, it’s where package binaries go.

[SDemonUA]

@babel is

P.S. And I have tried yarn install --force

[ljharb]

No, @babel is a scope, not a package - but @babel/core certainly is a package, and if that lacks a package.json that suggests your yarn install didn’t work properly.

[lencioni]

I am currently trying to update from v8.2.0 to v9.0.0 and have run into what appears to be this issue as well. I have been unable to reproduce it when running locally (Mac), but when it runs in our CI environment (Linux) it fails like this.

/path/to/repo/node_modules/licensee/index.js:216
  return string.toLowerCase().indexOf(prefix.toLowerCase()) === 0
                ^
TypeError: Cannot read properties of undefined (reading 'toLowerCase')
    at startsWith (/path/to/repo/node_modules/licensee/index.js:216:17)
    at /path/to/repo/node_modules/licensee/index.js:154:9
    at Array.some (<anonymous>)
    at resultForPackage (/path/to/repo/node_modules/licensee/index.js:149:26)
    at /path/to/repo/node_modules/licensee/index.js:86:18
    at Array.forEach (<anonymous>)
    at findIssues (/path/to/repo/node_modules/licensee/index.js:81:16)
    at /path/to/repo/node_modules/licensee/index.js:46:24

[ljharb]

That line comes from https://github.com/jslicense/licensee.js/blob/main/index.js#L154, which suggests it's trying to install a package that has no "name" field.

@lencioni if your local node/npm version matches what's in CI, does it reproduce locally? Alternatively, can you patch the file in CI to log out the result object when result.name is falsy?

[lencioni]

@ljharb I have not yet been able to get this to reproduce locally, even with the same node/npm versions.

Here's the object when result.name is falsy:

{
  name: undefined,
  version: '',
  license: undefined,
  author: undefined,
  contributors: undefined,
  repository: undefined,
  homepage: undefined,
  parent: null,
  path: '/path/to/repo-cache-dir'
}

We do have a package.json file at the root, but it has a name field, among other things.

[lencioni]

I am realizing that our node_modules directory is symlinked into place in our CI, which I am wondering if that could be the source of the problem for us. I'm going to test this theory soon.

[lencioni]

Yep, it was the symlink that triggered this error for us. This seems to work okay when node_modules is not a symlink.

[ljharb]

Indeed, I've seen lots of issues caused by things attempting to symlink node_modules. If there's something in licensee that could help (adding an extra fs.realpath in somewhere) I'm happy to land it!

[kemitchell]

Not against reading through links of all kinds, assuming there's a reasonable abstraction for doing so. We just don't want this repo becoming mostly about filesystem quirks.

[lencioni]

I have tried to reproduce this locally on my mac by moving my node_modules somewhere and symlinking it into my project, but I can't seem to make it trigger this issue. This makes debugging this pretty annoying.

In my case, the problematic "dependency" seems to be pointing to the root directory that contains the actual copy of node_modules, not the root of the repo that has the symlink in it (and the location where the script is being run from). I'm not sure there's any place in licensee where something like a fs.realpath would help, since it seems like we are in the inverse scenario where we are at the realpath for one of these nodes and would need to get back to the repo somehow.

I looked through the code a bit and I'm not entirely sure where the root of the issue lies. I suspect it could lie somewhere in the Arborist code, but I wasn't able to figure out exactly where. I noticed they have a v6 release out, so it might be worth at least trying to update?

I discovered a filterPackages configuration option (undocumented) that was added by https://github.com/jslicense/licensee.js/pull/63 that I can to use to filter this package out and unblock the update for myself like this:

filterPackages: (arboristLinks) => {
  return arboristLinks.filter((arboristLink) => {
    if (arboristLink.package.name == null) {
      return false;
    }

    return true;
  });
},

In this process, I discovered that when the name is undefined on the result object, the arborist link object actually has a name field that is a string that matches the name of the directory that the node_modules is in. (i.e. "shared" if our real node_modules dir was located in /root/shared). But, I don't think we want to use that, since it could be a "valid" npm package name but is not an actual package here.

I think it might make sense to have licensee detect a null dependency and skip over it by default, probably expanding the isProjectRoot check. https://github.com/jslicense/licensee.js/blob/42aae52e2c07fe2f978188b0503fa84e9d123620/index.js#L40-L42

What do you think about that?

[kemitchell]

I'd like to separate the linked node_modules issue and a potential change to null-dependency handling, unless one is causing the other.

Would like to:

• [ ] come up with a package.json and node_modules working tree that reproduces the error
• [ ] write a test that fails
• [ ] configure GitHub Actions to run that test on Windows and OS X with all the same supported Node versions

From there we should be able to better diagnose.

[lencioni]

I've added a test case in https://github.com/jslicense/licensee.js/pull/79 but I'm not sure how to see it run and tell if it reproduces the error.