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

acceptDependencies package.json field #72

Closed
wants to merge 1 commit into from

Conversation

coreyfarrell
Copy link
Contributor

What / Why

Make it possible for modules that support node.js 0.1 to accept the latest version of dependencies regardless of the minimum node.js it requires. I know this could use some more details especially about handling of peerDependencies, I wanted to post what I have so we have something concrete to base discussion around.

References

isaacs added a commit to npm/arborist that referenced this pull request Feb 7, 2020
This treats any dep ranges in the acceptDependencies field as an
acceptable alternative for any dependency range specified elsewhere in
the package manifest.

Implementation of npm/rfcs#72
@isaacs
Copy link
Contributor

isaacs commented Feb 14, 2020

Note: this is now implemented in @npmcli/arborist, more or less as described in the draft RFC, as of npm/arborist@705578d

  • Should acceptDependencies explicitly reject anything that is not a semver-range?
    What should happen if other values are used for acceptDependencies versions?
    Install from git, etc.

I think the answer here is that they'll just serve as alternative version specifiers that are also valid resolutions. There's no need for them to be semver ranges per se. It's perfectly fine to say "I want it fetched from this URL if necessary, but this other URL is also fine if present."

  • Handling of module groups could use more thought. peerDependencies should help
    ensure improper deduplication does not occur. Open to suggestions on how to better
    support groups of modules - allow module1@2.x and module1-plugin@2.x to both be
    installed if either is otherwise used.

This is indeed tricky, but I think the peer dep resolution implied by #43 will avoid the worst of it, making it merely sub-optimal rather than an actual footgun.

Let's say you depend on module@1.x and plugin@1.x, and plugin has a peer dep on the corresponding major version of module. You also have module@2.x and plugin@2.x in your acceptDependencies list. There are two possible problems contemplated here:

  1. There is a preexisting deduped copy of module@2.x, but not of plugin@2.x.
  2. There is a preexisting deduped copy of plugin@2.x, but not of module@2.x.

Case (2) is already problematic: the peer dep would be unsatisfied, and npm v7 won't let that happen by default. So, it's highly unlikely that a user will find themselves in that situation.

Case (1) will result (today) in the plugin@1.x being fetched, requiring a peerDep of module@1.x. Either that will conflict (if it's at the same level as your package being installed), or it'll result in a dupe copy of module@1.x shadowing the module@2..x shallower in the tree. Which, is fine, just not optimal.

It would be nice if the tree resolver could note that the peer dep would be acceptable to one of the acceptable versions of your plugin dependency. However, doing that level of resolution takes us squarely into PubGrub land and NP-hard heuristics, which I've tried (successfully, so far) to avoid.

The third case is that plugin does not have a declared peerDep on module, but instead is just sort of implicitly peer depending on it. I don't think there's any straightforward way for us to support module grouping of that sort without imposing some pretty onerous package.json gymnastics on module publishers.

All in all, I like how simple this is now, and I'm perfectly comfortable punting on the hard bits imagined in this question.

  • What should be done with modules listed in acceptDependencies that are not
    dependencies of the local module?

Currently they're just ignored. acceptDependencies is only consulted when constructing the dependency edges, and only then if the name matches.

  • Should null have special meaning for optional dependencies of direct dependents? Lets
    say I want to use small-pkg1 but it has an optional dependency on huge-module.
    Could we say that this optional module is unwanted by adding "huge-module": null to
    our own acceptDependencies? Would need to determine scope of these nullified optional
    dependencies.

That feels like overreaching for this, I think. There's already --no-optional (or the more composable and explicit --omit=optional in npm v7) if the user doesn't want optional deps, and very little preventing an author from just forking a version that doesn't have the optional dep on huge-module, so I'm fine ignoring it here.

@isaacs isaacs added the Agenda will be discussed at the Open RFC call label Feb 27, 2020
@isaacs
Copy link
Contributor

isaacs commented Feb 27, 2020

So, this is implemented in arborist, and status quo will be for it to make its way into npm 7.x. It seems pretty uncontroversial, but adding it to the agenda for us to at least officially say so in the RFC meeting.

@coreyfarrell
Copy link
Contributor Author

Should I plan to join the RFC meeting? If so do you know when it is scheduled?

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

Successfully merging this pull request may close these issues.

3 participants