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

ishowta commented 2 years ago

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

Related React Issue: https://github.com/facebook/react/issues/13991

ljharb commented 2 years ago

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 commented 2 years ago

@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. https://github.com/npm/rfcs/pull/637 & https://github.com/npm/rfcs/pull/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 commented 2 years ago

@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.

npm / rfcs