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

[RRFC] Add a field to package.json recommending that the package be unique. #649

Open
ishowta opened this issue Oct 29, 2022 · 3 comments
Open

Comments

@ishowta
Copy link

ishowta commented Oct 29, 2022

Motivation ("The Why")

There can be a problem when multiple versions of a package in dependencies. but we generally find it difficult to issue warnings without false positives.

However, in the case of a package that uses a global store, such as React, multiple versions are not expected to be used at the same time in most cases. This often occurs as a runtime error and is a source of confusion.

Therefore, I think it would be better to add a field like recommendUnique: boolean that recommend such a package to be unique and issue a warning on any lint tools or npm ownself.

Example

package.json

{
  "recommendUnique": true
}

How

Current Behaviour

Desired Behaviour

$ some-linting-tool or npm install
[warn] `React` recommend unique version but detect multiple version. please run `npm ls` and solve it.

References

@ljharb
Copy link
Contributor

ljharb commented Oct 29, 2022

This isn’t actually something the package author can know. Even react can be legitimately and safely duplicated in a dep graph, it’s just not a common use case.

@darcyclarke
Copy link
Contributor

darcyclarke commented Oct 31, 2022

@ishowta love how you're thinking about this & thanks for the RFC! I feel like this may get solved by some of the audit filter/policy work/proposals our team has been discussing & working toward for some time (ref. #637 & #636).

The broad-strokes idea here is that we expose a generic way of writing policies/rules using the new query language we support (ie. https://docs.npmjs.com/cli/v9/using-npm/dependency-selectors). This would let users define heuristics & let npm audit log/warn/fail when matching those "policies".

That said, we currently know there's a problem here with finding "unique", "unhoisted" dependencies & so there's likely a missing selector/context we aren't exposing today in @npmcli/arborist's Node.querySelectorAll() API. Currently, :deduped only looks for >1 edgesIn on a Node wherease you likely want to find deps that weren't able to be deduped because of some conflicting range (ie. something like a :nested pseudo selector which would only be applied when the strategy wasn't able to hoist the dep for some reason -> ex. npm install --install-strategy=hoisted but there were conflicting ranges for that dep OR npm install --install-strategy=nested where all deps will get nested as if it were legacy version of npm prior to deduping - ref. https://docs.npmjs.com/cli/v9/using-npm/config#install-strategy)

ex. query you might be able to run today which tells if there's more then a single version of react in your tree: npm query "#react:not(:path(/node_modules/react))"

@ishowta
Copy link
Author

ishowta commented Nov 1, 2022

@darcyclarke Thanks for the reply! I think @ljharb is right, but I also think it is not kind to require the user to investigate the cause and add appropriate queries every time an error occurs (especially if it is a transitive dependency). So I thought it would be helpful for the user if it could be configured on the package author's side if possible.

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

No branches or pull requests

3 participants