Include prerelease versions when deprecating an entire package.#2366
Include prerelease versions when deprecating an entire package.#2366isaacs merged 1 commit intonpm:release/v7.3.0from
Conversation
darcyclarke
added
Needs Review
Release 7.x
isaacs
left a comment
isaacs
left a comment
There was a problem hiding this comment.
Seems good to me, just a comment about whether npm deprecate pkg@* should include prereleases as well.
lib/deprecate.js
Outdated
| // npa makes the default spec "latest", but for deprecation | ||
| // "*" is the appropriate default. | ||
| const spec = p.rawSpec === '' ? '*' : p.fetchSpec | ||
| const options = spec === '*' ? {includePrerelease: true} : {} |
There was a problem hiding this comment.
🤔
Should this check be const options = p.rawSpec === '' ? { includePrerelease: true } : {}?
Ie, if you did npm deprecate foo@* should that include prereleases? Or only if you do npm deprecate foo?
There was a problem hiding this comment.
If so, these lines can be condensed a little cuter:
const [spec, options] = p.rawSpec ? [p.fetchSpec, {}]
: ['*', { includePrerelease: true }]
There was a problem hiding this comment.
I’d expect both. Excluding prereleases is generally the surprising case for non-installs.
There was a problem hiding this comment.
I'd expect both too, but I didn't know about the prerelease rules in the semver library until this week 🤯
What if I added @isaacs 's recommendation and then a note was added to the docs, e.g. note that passing a range '*' will not deprecate prereleases versions.?
PR-URL: npm#2366 Credit: @tiegz Close: npm#2366 Reviewed-by: @isaacs EDIT(@isaacs): updated to make _all_ deprecation ranges include prereleases. If `foo@*` would be expected to deprecate `[email protected]`, then presumably `[email protected]` has the same expectation.
isaacs
force-pushed
the
tiegz/deprecation-fix
branch
from
5db80d7 to
44d4331
Compare
|
Thanks @isaacs ! |
4 participants
The NPM docs say:
This should mark all versions as deprecated, but it currently excludes any prerelease versions.
The filtering of versions in the deprecate command is done by passing a version range to
semver.satisfies(). A wildcard*is passed as the version range when deprecating an entire package, but wildcards don't include prereleases by the semver library's rules, so we need to pass theincludePrerelease: trueoption so that all prereleases are included in the deprecated versions.References
Fixes #1615