RFC: Peer dependencies should be able to match a full range of prerelease versions#397
RFC: Peer dependencies should be able to match a full range of prerelease versions#397alasdairhurst wants to merge 1 commit intonpm:mainfrom
Conversation
|
Meeting notes discussion: https://github.com/npm/rfcs/blob/latest/meetings/2021-07-28.md#pr-397-rfc-peer-dependencies-should-be-able-to-match-a-full-range-of-prerelease-versions---alasdairhurst Next action items:
|
|
I'll try to clarify the use case I have in mind in (hopefully) simpler terms: (cc @wraithgar) Example"library" is being developed using prerelease versions and "library" has a wide range of plugins (let's say there are 100+ of them). Plugins have varying peer dependencies on "Library". let's take two examples: [email protected] is in development and not quite ready, so "[email protected]" is released for testing Personas and issuesThe issue is the following on npm 7:
A few things to keep in mind:
Suggestion:I've come up with the following suggestion based on ideas thrown around during the RFC meeting: My suggestion is either as an opt-in, or even by default, that if pre-release modules ("[email protected]") are directly installed by a package (i.e. an explicit dependency), and that pre-release fits within the semver range provided by a peer dependency (assuming ^4.0.0 includes 4.0.0-0 and 4.1.0-0 but not 5.0.0-0 in this particular calculation), then that particular pre-release peer will be treated as resolved and no other version will be attempted to be installed. This is specifically an opt-in on behalf of the user who creates the package which depends on both "library" and "library-plugin". Let me know if anyone can think that this would result in unexpected/unwanted behavour. Alternatively, the same approach applies, but the plugin itself declares in peerDependenciesMeta that prereleases should be accepted in the semver range. This is ok but I don't believe that it's necessary for a few reasons:
|
|
fwiw, all those categories of people can test it, they just have to use |
|
@ljharb you're absolutely right. This is mentioned as a workaround in the PR, although I'm intentionally ignoring this when discussing the issue since we need to figure out a way for this to work in a modern way, as NPM prefers resolving peers now, and not a legacy one which could be removed at any point in time. |
👍🏼 |
|
Here's a use case: eslint-community/eslint-utils#183 (comment) Basically: ESLint plugins / code with a peer dependency saying that it supports everything in ESLint 8 and later won't install cleanly with ESLint 9 pre-releases, which is not the intention of ESLint plugins / code when they say When we in ESLint plugins / code says
My current suggestion in that issue is to pre-emptively do
Implementation wise, what I would prefer is: When checking if a peer dependency is valid, use |
|
Another use case:
But when used with a pre-release of |
I think this check happens here: https://github.com/npm/cli/blob/762888a3b603704c7c53a94a704b8a7f3edea918/workspaces/arborist/lib/dep-valid.js#L53 But it does so for a lot more cases than when checking if a peer dependency is valid |
The semver spec does not include range specifiers IIRC, so I think the meaning here is more the EDIT: The issue here is that the library allows for specifying |
|
If you're using a prerelease, I believe you can use overrides and it'll ignore the range - which supports application use cases without leaking things into the module graph. |
There are cases where you want this in the published version. Not as much in open source packages but we have many packages which use "git ops" style workflows where packages live with applications and forcing this for PR builds would be nice. In the current state we end up publishing many more versions because we need to hard code the specific prerelease to get it to work. If we had a feature like this we could publish just the changed package as we iterate on the PR. |
My memory played big tricks on me 😳 Semver range specifiers should also have a spec!
I would say that there's no need for a new range specifier. The peer dependency validation logic in npm simply has to change so that a peer dependency range of eg. So: The acceptance check that happens for peer dependencies should change to have My use case with |
This would be a breaking change for sure, and I think my point above is this is a change which is unlikely to land well. It is a tradeoff where this behavior would make a lot of existing uses broken to benefit this smaller use case. Which is why I think the range needs to include it to support both use cases. I think the use case is totally valid, but it is enough in conflict with the current behavior which is relied upon for other legitimate use cases that I think simply changing it is not the best option. |
|
There's an open PR on the semver spec to add ranges (using npm's semantics, ofc). knip would have to do |
|
The range grammar spec @ljharb mentioned: semver/semver#584 The goal of that discussion was to document what node-semver does, and then see what overlap or patterns could be codified, maybe changes that could be made to bring us all into compliance with one another, etc. The outcome was that (a) there really is not much overlap in how different package manager implementations do semver ranges (except for cargo and npm, since cargo copied node-semver's semantics pretty faithfully), and (b) almost any change, no matter how minor, would be an ecosystem-splitting breaking change, so the costs are very high. So, it seems unlikely that the range specifiers will ever be a more normative spec than just "this is how node-semver does it". Which, ok, still useful maybe. But also, any extension or change to it is likely to have very profound negative consequences. If it's strictly additive, that might be fine, especially if npm prevented publishes using those new range grammar extensions (or converted them, like how pnpm and yarn convert tl;dr - as far as semver range expressions go, most likely we're kinda stuck with what we've got. Getting back to the OP issue here, I think allowing prereleases in peerDependencies would be fine, and likely not a breaking change to anyone, as long as non-prerelease versions were still prioritized, with one important exception, which probably kills the idea (or at least, imposes a tricky puzzle to be solved). Let's say that you have The version It's reasonable for us humans to look at that peerDeps spec, and see that while they might be ok with So, we'd have to somehow unambiguously codify exactly which prereleases should be included, and which should not. I don't think this is impossible, by any means, but it would have to be done in order for this to not be a footgun. |
|
Maybe a first stab at that codification would be as simple as: "Include any prereleases, but treat all This would solve the case of I have not exhaustively gone through all the possible cases to verify this doesn't have some other bad effects, though. |
|
I have the same issue as mentioned in: npm/cli#2087 Is there any solution besides overrides at the moment? There should be a flag to include all the prereleases with a range. |
This mostly works, but fails if someone writes something strange like Depending on how conservative you want to be you can resolve this by choosing one or both of the following rules:
(I mentioned x-ranges above, note that using explicit pre-release versions with x-ranges is currently broken in node-semver: npm/node-semver#510 ) |
|
How about this approach? Add an For example: "peerDependencies": {
"foo": "2.x",
},
"peerDependenciesMeta": {
"foo": {
"includePrerelease": true
}
}Looks to me like this would solve a large percentage of use cases without having to meddle with the range spec. |
to work around the lack of pre-release support in modern resolution. see npm/rfcs#397
|
Meddling with the npm resolution behaviour does have two major advantages over an opt-in "includePrerelease":
And I think that there is an important property of the proposed meddling which makes the compatibility risk relatively low. We can say that if it is possible to resolve a set of dependencies without allowing prereleases in peer-dependency ranges, then this should be done preferentially. Meaning that for any existing package with a valid dependency set, the resolution of versions for those dependencies should not change. Only for packages which fail to resolve without allowing prerelease peer deps should behaviour change to become more lenient. |
If this was not clear, I believe this allows us to guarantee that the new behaviour will not break any existing configurations. Are there remaining concerns which are holding this back, or has interest just dwindled? It's a shame to let the discussion peter out if there's a way forward. |
I'd still be actively pushing further If I still had active projects that benefited from this behaviour but unfortunately don't right now :( Honestly was pretty surprised this wasn't made into a bigger deal back when NPM 7 was released, but I was happy to see some references from Eslint which was one of the projects I had considered to end up being closely impacted by this change. There's a lot of discussion above about precedence of pre-release versions in semver resolution, but in terms of peer dependency behaviour there's only really two underlying concerns that I can think of:
The two don't necessarily have to be conflated in the same way. If we allow NPM to be more flexible on acceptance we don't have to impact how it picks versions to install. At least not in the same way. It would make sense to me for NPM to keep traditional version priority when installing peers, but just choose not to install prerelease versions if there's any non-prerelease versions in the range. Then when it comes to manually installing peer dependencies you can opt-in to installing a prerelease version and it's accepted by NPM. The question is if we're need to worry specifically about developers wanting to opt-out from allowing prereleases to match a particular peer dependency range. For example if they want to indicate compatibility starting from 1.0.0 but not 1.0.0-x. This causes complexity again since it forces us to explicitly have to opt-in to peer dependencies within the range, but maybe that's fine? "peer-dependency" : "^1.0.0-0": Satisfies all 1.x versions of peer-dependency including prereleases within the major version range and prereleases of 1.0.0 "peer-dependency" : ">=1.0.0": Satisfies all 1.x versions of peer-dependency including prereleases within the range, but not prereleases of 1.0.0 (since ^1.0.0 is greater) "peer-dependency" : ">=1.0.0-0 < 2.5.0": Satisfies all versions of peer-dependency from 1.0.0 to 2.5.0 and any prerelease of these versions. The prerelease behaviour only really needs to be considered when offering compatibility with prereleases of the minimum supported version. (1.0.0-0).
This basically seems almost equivalent, if not the same as your suggested approach above @Sophos-Elias-Vasylenko |
A User should be able to specify a "semver" range, such that all prerelease versions within that range will match.