Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(npm): use Yarn 3 mode to skip install or build #11012

Merged
merged 18 commits into from Aug 29, 2021

Conversation

ylemkimon
Copy link
Contributor

@ylemkimon ylemkimon commented Jul 29, 2021

Changes:

  • Set skipInstalls = false if zero-installs is used.
  • Use new Yarn 3 --mode=update-lockfile flag to not run the link step and only fetch what's necessary to compute an updated lockfile if config.skipInstalls !== false.
  • Use --mode=skip-build to not run the build scripts instead of skipping the whole build step (enableScripts: false) if !adminConfig.allowScripts || config.ignoreScripts.

Context:

This PR fixes #10061 and reduces network bandwidth by fetching only necessary packages. This PR also fixes #7584 by running the build step that unplugs packages.

Caveat

Yarn built between yarnpkg/berry@9bcd27a and yarnpkg/berry@d132132 would cause an invalid option error. This would affect:

  • Yarn 3 release candidates (3.0.0-rc.1 to 3.0.0-rc.9)
  • yarn set version from sources run between two commits

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please tick one)

I have verified these changes via:

Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should also be tested on exiting yarn v1 and yarn v2 projects to be safe?

lib/manager/npm/post-update/yarn.spec.ts Show resolved Hide resolved
@rarkins
Copy link
Collaborator

rarkins commented Jul 29, 2021

Where would be the best place to check this?

skipInstalls logic is spread a little between npm/extract and npm/post-update today, so I think you can put it in either of those where it's useful. I guess it needs to be after we detect yarn version

@ylemkimon
Copy link
Contributor Author

ylemkimon commented Aug 7, 2021

Sorry for the delay. It now sets skipInstalls = false if zero-installs is used, i.e., .yarn/cache and related files are detected in extractPackageFile(). This PR is ready for review.

@ylemkimon ylemkimon closed this Aug 7, 2021
@ylemkimon ylemkimon reopened this Aug 7, 2021
@sburba sburba mentioned this pull request Aug 17, 2021
3 tasks
Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some small changes

lib/manager/npm/extract/index.ts Outdated Show resolved Hide resolved
lib/manager/npm/post-update/yarn.ts Outdated Show resolved Hide resolved
lib/manager/npm/post-update/yarn.ts Outdated Show resolved Hide resolved
lib/manager/npm/post-update/yarn.ts Outdated Show resolved Hide resolved
lib/manager/npm/post-update/yarn.ts Outdated Show resolved Hide resolved
@viceice
Copy link
Member

viceice commented Aug 28, 2021

Please use re-request review buttons 🙃
image

@ylemkimon ylemkimon requested a review from viceice August 28, 2021 22:31
@ylemkimon
Copy link
Contributor Author

I didn't know a non-member could do that 😄

@viceice
Copy link
Member

viceice commented Aug 29, 2021

Also the author of the PR is allowed to use that. 🤗

@rarkins rarkins merged commit 62a540c into renovatebot:main Aug 29, 2021
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 26.12.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@ylemkimon
Copy link
Contributor Author

Thank you for the reviews!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants