fix: update notifier to consider engine range #8050
Conversation
|
We'll want a new test for the behavior this is guarding against. |
|
while thinking on how to write a test for this one, logically the problem this pr solves it to get correct engine matched version, we only request for |
Yes, this is behavior we don't want to accidentally regress from. The update notifier tests will likely need a bit of tweaking to get there |
milaninfy
force-pushed
the
mm/fix-update-notification
branch
from
f95e421 to
07fd64f
Compare
test/lib/cli/update-notifier.js
Outdated
| const NEXT_MINOR = '123.420.70' | ||
| const NEXT_PATCH = '123.421.69' | ||
| const CURRENT_BETA = '124.0.0-beta.99999' | ||
| const HAVE_BETA = '124.0.0-beta.0' | ||
|
|
||
| const pacumentResponse = { |
There was a problem hiding this comment.
packument not pacument (I didn't do a code suggestion cause it will need to be changed in multiple places)
test/lib/cli/update-notifier.js
Outdated
| }) | ||
|
|
||
| if (PACOTE_MOCK_REQ_COUNT > 0) { | ||
| registry.nock.get('/npm').times(PACOTE_MOCK_REQ_COUNT).reply(200, pacumentResponse) |
There was a problem hiding this comment.
This is a great iteration. Eventually we'd want this in the mock registry (this request AND the packument it generates) but I don't want to block the PR on that.
milaninfy
force-pushed
the
mm/fix-update-notification
branch
from
af46071 to
ee70630
Compare
milaninfy
force-pushed
the
mm/fix-update-notification
branch
from
ee70630 to
a1f0923
Compare
When doing manifest call for fetching npm manifest it was using latest every-time, however fetching
npm@*will still givelatestif it's satisfies the engine range otherwise it will give highest matching version for the current engine range.Pacote.manifest calls internally uses pick-manifest logic to pick the appropriate version of the manifest for the engine range.