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

removes deprecated yarn check in bin/setup #50753

Merged
merged 1 commit into from
Jan 15, 2024

Conversation

adrienpoly
Copy link
Contributor

Motivation / Background

While running a fresh Rails install I got a bunch of warnings from the command yarn check --check-files and dig a bit into the documentation

This Pull Request has been created because it appears the check command is deprecated and will be removed in Yarn 2.0

Detail

This Pull Request changes the generator to replace

system("yarn check --check-files") || system!("yarn install") 

by the recommended command

system("yarn install--check-files")

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

@rails-bot rails-bot bot added the railties label Jan 14, 2024
@adrienpoly adrienpoly changed the title removes deprecated yarn check removes deprecated yarn check in bin/setup Jan 14, 2024
@zzak
Copy link
Member

zzak commented Jan 15, 2024

I don't even think we've tried upgrading to Yarn 2, we're even using 1.x on CI afaik.

I've heard ideas floating around of us dropping yarn all together and use npm in places that still use node, that might be a concern of jsbundling; that is to say I'm not sure but if there are other places that are not Yarn 2.0 compatible they might be investigated.

Noticed that debian testing/unstable provide Yarn Modern, so that's an interesting data point to consider.

@adrienpoly
Copy link
Contributor Author

Thanks for your review, here is a bit more information about why I came to investigate this.

  • Clone a Rails project with a package.json
  • run bin/setup

CleanShot 2024-01-15 at 07 02 23@2x

notice the 6 red error lines. This gave me a little WTF moment and I thought my setup went wrong. Those errors are the output of the current yarn check --check-files.

Another way to reproduce it is to run those commands in a project

rm -rf node_modules
bin/setup

This is the output with my proposed change
CleanShot 2024-01-15 at 07 38 01@2x

I think this change is beneficial as it drops a deprecated command from Yarn and it removes miss leading messages from bin/setup

@rafaelfranca rafaelfranca merged commit afc7d03 into rails:main Jan 15, 2024
4 checks passed
@adrienpoly adrienpoly deleted the remove-deprecated-yarn-check branch January 15, 2024 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants