Fixes unwanted yarn.lock optimization issue #3490

[arcanis]

Summary

This PR aims to prevent the lockfile from being optimized when reading it. If a package isn't "fresh" (ie if it's present in the lockfile), we resolve it to the exact version registered in the lockfile.

Test plan

I haven't been able to come up with a minimal testcase that could be used with the integration tests :/ I used the following repository to test my changes, but it brings a lot of dependencies and some of them don't seem to work in the tests (got this apparently unrelated error: error "neutrino#optional" is wrong version: expected "0.1.3", got "v0.1.3").

$> git clone https://github.com/timkelty/yarn-order-bug
$> yarn
$> git diff yarn.lock

The last line should print nothing, but before this patch it was optimizing the diff.

[BYK]

Why not make --frozen-lockfile default to true instead?

[arcanis]

I thought about this, but:

• Frozen lockfiles will throw an error if their lockfiles need to be updated (ie if a resolved package is "fresh"). We don't want this behavior. Furthermore, if there's no lockfile at all, we still want to write it (at least for now, otherwise we would be breaking peoples' workflow in a minor).

• Pure lockfiles will not write their lockfiles either, even if there's none.

This PR also ships with a fix (the resolvedRange thing) to instruct Yarn to use the same references when resolving the same version. Without it, it yells a lot of warnings (thankfully nothing really break) because it thinks that multiple packages are trying to unpack into the same directory.

[BYK]

Awesome!

[BYK]

Please use a more descriptive commit title before merging.

[bestander]

+1 about descriptive title, I'll help you with this one.
Also I think you need to add "fixes #3490" to the PR description so that Github closes the associated issue automatically and people were able to navigate to the issue with a click (the #3490 in title is not clickable)

[bestander]

We need tests for this, @arcanis.
I understand that it will be a waste of time to hand-craft the test, so just add an integration test from https://github.com/timkelty/yarn-order-bug.
We'd better pay the price of extra 20s on test runtime than have this issue return back.

[bestander]

Ideas for tests:

1. https://github.com/timkelty/yarn-order-bug indeed looks rathe large, if you can express it in install operations then good.

2. There is a test case in this branch https://github.com/yarnpkg/yarn/pull/3563/files, just cherry-pick it and verify that your PR fixes it

[bestander]

@BYK, regarding --frozen-lockfile being default, there is an early discussion here #570.

I raised the issue but was convinced that we can drop lockfile strictness when package.json is manually updated.
In all other cases Yarn should definitely not update lockfile without a warning.

[bestander]

This looks promising, @arcanis.
How is it related to #3673, can we return concurrent resolution?

[arcanis]

@bestander I'm not completely sure regarding #3673 - I think it should work, but I'll have to test it manually later. Once this patch is ok'd I'll publish the 0.27.1 - David is needing the link: feature, so I'd like not to block him too long. After that, I'll make some tests with adding back the async behavior :)

[arcanis]

Working on the tests, btw 👍

[arcanis]

Should be ok 👍