Modernizes installs' output #5509

arcanis · 2023-06-19T10:27:54Z

What's the problem this PR addresses?

I wasn't entirely happy with the Yarn output:

When running yarn add, we have no way to know what packages are actually added to the lockfile. The cache messages sometimes help, but with zero-installs now being opt-in, in many cases the packages would already be in the cache and thus wouldn't be displayed at all.
The "can't be found in the cache and will be fetched from the remote registry" messages were incredibly slow to print - the same install would take 28s install for Gatsby on a local iTerm 2 without those logs, vs almost 2 minutes with. They also weren't very useful: we were only showing the last 5 of them, and with zero-installs being opt-in in many cases they wouldn't be shown at all.
The node-gyp warnings were for the most part unactionable (at best the user would pin a fixed version in their packageExtensions field, which I suspect no one ever did).
The deprecated warnings were for the most part unactionable, and only printed in very specific cases (the first time they were added to the project).
The "missing / invalid peer dependency" warnings were actionable, but in practice no one really look at them except when something breaks - and even then, it becomes unreadable when there are too many of them.
The skipped build warnings were printed every single time you ran yarn install. It's nice to know the first time, then it quickly becomes redundant.

How did you fix it?

I went a little overboard and did everything in the same PR ... at first I thought I wouldn't have to change unrelated parts, but then I finished implementing the skipped build warnings duplicate removal and oh no 🙈

Only peer dependency warnings caused by workspaces are now reporter. They have also been moved inside the post-resolution validation step. The resolution step is now expected to only do one of two things: either report an error when the resolution fails, or report the packages which got added / removed from the lockfile.
The node-gyp warnings have been removed.
The deprecated warnings have been removed from the install. The yarn npm audit command now reports deprecated packages, although this can be disabled using --no-deprecations (or any of the audit filtering settings).
The "can't be found in the cache" messages have been removed. Instead, the fetch step will report the number of cache hits / cache misses once it's finished (same behaviour as preferAggregateCacheInfo). The size delta will also be reported.
Packages whose build was skipped are now stored within Project#skippedBuilds, which is stored within the install state file. Warnings are only emitted if the install was skipped for the first time. To see the messages again, users can run yarn rebuild.
Added the Yarn version on installs. A bit because of branding when screenshot are taken, but mostly so we easily know what version are people using when they share screenshots to us. In a follow-up PR I'd like to try to retrieve the latest version from the website, to let them know once one is available.

Checklist

I have read the Contributing Guide.

I have set the packages that need to be released for my changes to be effective.

I will check that all automated PR checks pass before the PR gets reviewed.

arcanis · 2023-06-19T10:29:39Z

Perhaps we can also consider removing the "forgettable messages" mechanism, since they are complex and not that useful (the only other forgettable message is UNUSED_CACHE_ENTRY, which we could easily aggregate into a single line, like we do here).

merceyz · 2023-06-19T16:52:52Z

There are some issues that this probably fixes (for example #4165), could you check and link to them so they're closed?

paul-soporan

Nice work! Left an in-depth review, but overall I really like the changes 👍

Note: We shouldn't forget to update the CHANGELOG, either now or later, especially with the breaking changes.

The resolution step is now expected to only do one of two things: either report an error when the resolution fails, or report the packages which got added / removed from the lockfile.

Note that this is expected, not enforced, as proven by the fact that nothing prevented you from forgetting to remove the deprecation warnings during installs.

Perhaps we should do something about it 🤔

packages/yarnpkg-core/sources/YarnVersion.ts

packages/yarnpkg-core/sources/miscUtils.ts

packages/yarnpkg-core/sources/formatUtils.ts

packages/yarnpkg-core/sources/Project.ts

packages/plugin-npm-cli/sources/commands/npm/audit.ts

packages/yarnpkg-core/sources/Project.ts

arcanis · 2023-06-22T09:49:43Z

I think the PR is ready, I'll land it this afternoon unless someone sees a significant blocker.

paul-soporan · 2023-06-22T19:14:49Z

Looks good 👍

**What's the problem this PR addresses?**   Follow-up to #5509 (comment). Changed package detection can be optimized because of an important property of the resolution process: `startPackageResolution` is guaranteed to be run a single time for a given package. **How did you fix it?**  Because of this we can: - start with a list of the initial packages - for each package that we resolve, we delete it from the list - if the deletion was successful, it means that the package has existed before and exists now too - otherwise, it means the package didn't exist before and we can add it to `addedPackages` - in the end, `initialPackages` will only contain the removed packages **Checklist**    - [X] I have read the [Contributing Guide](https://yarnpkg.com/advanced/contributing).   - [X] I have set the packages that need to be released for my changes to be effective.   - [X] I will check that all automated PR checks pass before the PR gets reviewed. Co-authored-by: Maël Nison <[email protected]>

**What's the problem this PR addresses?**   Follow-up to yarnpkg/berry#5509 (comment). Changed package detection can be optimized because of an important property of the resolution process: `startPackageResolution` is guaranteed to be run a single time for a given package. **How did you fix it?**  Because of this we can: - start with a list of the initial packages - for each package that we resolve, we delete it from the list - if the deletion was successful, it means that the package has existed before and exists now too - otherwise, it means the package didn't exist before and we can add it to `addedPackages` - in the end, `initialPackages` will only contain the removed packages **Checklist**    - [X] I have read the [Contributing Guide](https://yarnpkg.com/advanced/contributing).   - [X] I have set the packages that need to be released for my changes to be effective.   - [X] I will check that all automated PR checks pass before the PR gets reviewed. Co-authored-by: Maël Nison <[email protected]>

**What's the problem this PR addresses?**   While reviewing #5509, I noticed that the `--mode` flag wasn't passed to `yarn up --recursive`. The issue started in #2913, when the flag was originally introduced. **How did you fix it?**  Made it pass `--mode` to `yarn up --recursive` and added tests. **Checklist**    - [X] I have read the [Contributing Guide](https://yarnpkg.com/advanced/contributing).   - [X] I have set the packages that need to be released for my changes to be effective.   - [X] I will check that all automated PR checks pass before the PR gets reviewed. --------- Co-authored-by: Maël Nison <[email protected]>

**What's the problem this PR addresses?**   The refactoring in #5509 accidentally dropped the `includeLogs: true` from the `yarn install` implementation. Fixes #5866 **How did you fix it?** I considered extending `installWithNewReport` to have a new `includeLogs` property, but it felt like this behaviour was unneeded for other use cases than `yarn install`, so I simply switched the `yarn install` implementation so that it manually creates the report stream rather than delegate to `installWithNewReport`. **Checklist**    - [x] I have read the [Contributing Guide](https://yarnpkg.com/advanced/contributing).   - [x] I have set the packages that need to be released for my changes to be effective.   - [x] I will check that all automated PR checks pass before the PR gets reviewed.

Deprecates the cache messages

bec58e0

Further logging changes

90077c0

arcanis changed the title ~~Makes preferAggregateCacheInfo default to true~~ Modernizes installs' output Jun 19, 2023

paul-soporan self-requested a review June 19, 2023 14:29

arcanis added 5 commits June 19, 2023 16:43

Resets the skippedBuilds set before each install

ad75b41

Experiments: add the Yarn version on installs

8aa954f

Removes the dot outside of installs

f308558

Removes forgettable mechanism

56aba6b

Adds trailing dot

865f1ea

arcanis added 9 commits June 19, 2023 20:24

Adds an indirection to avoid repeating the StreamReport options too much

292c3bb

Versions

5543444

Fixes lint / ts

5807d62

Tweaks the fetch output

d9e2b65

File size tweaks

c430fc7

Updates snapshots

d713b54

Fixes tests

216e800

Fixes tests

72a493b

Prints build logs on CI after installs failed

da567c1

paul-soporan mentioned this pull request Jun 20, 2023

feat: cache npm metadata #5491

Merged

7 tasks

Moves the file logs right after the line that reference them

be0d679

paul-soporan requested changes Jun 21, 2023

View reviewed changes

arcanis added 7 commits June 21, 2023 08:37

Feedback

d7ad8a9

Fedback

568d860

Improves package order

c286570

Fixes typecheck

e3841c3

Updates the changelog

cdebdb0

Fixes test

3f110e7

Removes obsolete test

9da1956

Fixes linting

769562b

arcanis requested a review from paul-soporan June 21, 2023 10:53

arcanis added 2 commits June 22, 2023 10:31

Merge remote-tracking branch 'origin/master' into mael/no-cache-messages

fbf5aae

Fixes snapshots after merge

fddd625

arcanis merged commit ec7f37a into master Jun 22, 2023

arcanis deleted the mael/no-cache-messages branch June 22, 2023 13:00

This was referenced Jun 22, 2023

perf(core): optimize changed package detection #5515

Merged

fix: pass mode to yarn up --recursive #5517

Merged

arcanis mentioned this pull request Nov 3, 2023

Fixes missing --json output on yarn install #5933

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Modernizes installs' output #5509

Modernizes installs' output #5509

Uh oh!

arcanis commented Jun 19, 2023 •

edited

Loading

Uh oh!

arcanis commented Jun 19, 2023

Uh oh!

merceyz commented Jun 19, 2023 •

edited

Loading

Uh oh!

paul-soporan left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

arcanis commented Jun 22, 2023

Uh oh!

paul-soporan commented Jun 22, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Modernizes installs' output #5509

Modernizes installs' output #5509

Uh oh!

Conversation

arcanis commented Jun 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arcanis commented Jun 19, 2023

Uh oh!

merceyz commented Jun 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

paul-soporan left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

arcanis commented Jun 22, 2023

Uh oh!

paul-soporan commented Jun 22, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

arcanis commented Jun 19, 2023 •

edited

Loading

merceyz commented Jun 19, 2023 •

edited

Loading

paul-soporan left a comment •

edited

Loading