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

[BUG] bd.includes is not a function #98

Open
1 task done
jinganix opened this issue Aug 15, 2023 · 7 comments
Open
1 task done

[BUG] bd.includes is not a function #98

jinganix opened this issue Aug 15, 2023 · 7 comments
Labels
Priority 2 will get attention when we're freed up

Comments

@jinganix
Copy link

jinganix commented Aug 15, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

We get the the error bd.includes is not a function when run npm audit, in this line:

const bundled = bd && bd.includes(this[_source].name)

There are some codes in github that the bundleDependencies is not an array:
https://github.com/search?q=%22%5C%22bundleDependencies%5C%22%3A+true%22+path%3A**%2Fpackage.json&type=code

As described in npm Docs: https://docs.npmjs.com/cli/v9/configuring-npm/package-json#bundledependencies
Alternatively, "bundleDependencies" can be defined as a boolean value. A value of true will bundle all dependencies, a value of false will bundle none.

Expected Behavior

No errors when running npm audit if bundleDependencies in some dependecies is not an array. We need check bundleDependencies is an array before call includes.

Steps To Reproduce

in

bundleDependencies: ['minimist'],

change from bundleDependencies: ['minimist'], to bundleDependencies: true,
then npm run test

Environment

  • npm: 9.6.3
  • Node: 18.14.2
  • OS: MacOS Ventura 13.5
  • platform: Macbook Pro
@jinganix jinganix added the Needs Triage needs an initial review label Aug 15, 2023
@wraithgar
Copy link
Member

The package.json that this module gets has been passed through a normalize function. It is not intended to get the un-normalized package.json. This is why it is making the assumption that if bundleDependencies exists it is an array.

Do you have steps to reproduce that aren't manually changing a test file?

@jinganix
Copy link
Author

We get the issue when retrieve packages from cloudsmith repo, it seems cloudsmith doesn't convert bundleDependencies: true to bundleDependencies: [...] in the response. The npm audit works before node 14 in our repo, and fails from node 16. It's hard to make a standalone demo.

@jinganix
Copy link
Author

We are using node 18.14.2 and npm 9.6.3, the package-json version is 3.0.0. The package-json doesn't have a normalize.js in the release zip file. I will try with node 18.17.1.

@jinganix
Copy link
Author

jinganix commented Aug 16, 2023

It still fails in 18.17.1. I found in this line:

const [cached, packument] = await Promise.all([

packument.versions has many versions, some of them has bundleDependencies: true

It seems npm registry will convert bundleDependencies: true to bundleDependencies: [...], but cloudsmith won't. Is the convertion a standard rule that registries should follow?

@wraithgar
Copy link
Member

If the cloudsmith registry is returning the bundleDependencies: true but npm itself is not normalizing that before audit, then it's inside npm itself not this module. The fix needs to happen in the audit workflow in either npm or arborist.

@jinganix
Copy link
Author

jinganix commented Aug 16, 2023

It still fails in 18.17.1. I found in this line:

const [cached, packument] = await Promise.all([

packument.versions has many versions, some of them has bundleDependencies: true

It seems npm registry will convert bundleDependencies: true to bundleDependencies: [...], but cloudsmith won't. Is the convertion a standard rule that registries should follow?

The request to cloudsmith is in this module:

const p = pacote.packument(name, { ...this[_options] })

The response with bundleDependencies: true is:

return paku

Should I raise the issue in https://github.com/npm/cli?

@wraithgar
Copy link
Member

const p = pacote.packument(name, { ...this[_options] })

Ah, good find. It's this module that's fetching the packument. This module needs to normalize it with @npmcli/package-json#normalize. It's intended to accept a package.json not a full packument so it'll need to normalize each version before processing.

@wraithgar wraithgar added Priority 2 will get attention when we're freed up and removed Needs Triage needs an initial review labels Aug 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority 2 will get attention when we're freed up
Projects
None yet
Development

No branches or pull requests

2 participants