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

check yarn version and run correct install command for yarn 2 #40722

Closed
wants to merge 2 commits into from

Conversation

doits
Copy link
Contributor

@doits doits commented Dec 1, 2020

Summary

Yarn 2 has different command line flags when installing dependencies. Rails now checks which yarn version is installed and uses correct flags, so it should work with either version.

Other Information

  • The guide is updated to install Yarn 2 with pnp disabled, so for maximum compatibility the usual node_modules folder gets created. Afaik everything should work like before with Yarn 1 with this.
  • Do we need a test for this?

@@ -145,6 +145,28 @@ $ yarn --version

If it says something like "1.22.0", Yarn has been installed correctly.

##### Additional steps for Yarn 2
Copy link
Member

Choose a reason for hiding this comment

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

Is there any way for us to not require those steps? We can even force all new Rails apps to use yarn 2.

Copy link
Contributor Author

@doits doits Dec 2, 2020

Choose a reason for hiding this comment

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

Yeah, it could be streamlined so that a generator installs correct .yarnrc.yml (and .gitignore?) and runs yarn set version berry. Then only the global install of yarn must be done by a user (like before). It could even update to Yarn 2 when upgrading a Rails app with rails app:update.

So dropping support for Yarn 1 is the way to go? I can update it accordingly if you confirm it.

Btw I just noticed those additional steps are at the wrong location in the guide anyway, since the Rails project is not created there yet and it must be run inside the Rails project folder.

Copy link
Contributor Author

@doits doits Dec 2, 2020

Choose a reason for hiding this comment

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

One thing to add is that Webpacker must support Yarn 2 then, too, and both Rails and Webpacker must be upgraded at the same time. See rails/webpacker#2799

Copy link

Choose a reason for hiding this comment

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

I know Rails 6.1 is in its RC2 but it would be nice to embed this (very) small fix in Rails 6.1.
It would allow people to migrate to Yarn 2 when they want. Otherwise we'll have to wait for Rails 6.1.1...

Copy link

Choose a reason for hiding this comment

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

In the meantime people who will want to use Yarn 2 will have an error during assets compilation :

$ time bin/rake assets:precompile
=> Checking config
DEPRECATION WARNING: connection_config is deprecated and will be removed from Rails 6.2 (Use connection_db_config instead) (called from <main> at /builds/nicolas/concerto/config/environment.rb:7)
Unknown Syntax Error: Unsupported option name ("--no-progress").
$ yarn install [--json] [--immutable] [--immutable-cache] [--check-cache] [--inline-builds] [--skip-builds]
I, [2020-12-05T16:20:41.097633 #2654]  INFO -- : Writing /builds/nicolas/concerto/public/assets/pghero/favicon-d65c971fa589b54eff2986b2c76b538cb940d24c47527496f0406643274ddb62.png
I, [2020-12-05T16:20:41.099431 #2654]  INFO -- : Writing /builds/nicolas/concerto/public/assets/manifest-04024382391bb910584145d8113cf35ef376b55d125bb4516cebeb14ce788597.js
I, [2020-12-05T16:20:41.100391 #2654]  INFO -- : Writing /builds/nicolas/concerto/public/assets/manifest-04024382391bb910584145d8113cf35ef376b55d125bb4516cebeb14ce788597.js.gz
I, [2020-12-05T16:20:41.101345 #2654]  INFO -- : Writing /builds/nicolas/concerto/public/assets/pghero/application-cf3eb1374d53ab33bda11ce75b3fb24754f28c6b4f5d34011bcca4c6278809eb.css
I, [2020-12-05T16:20:41.102525 #2654]  INFO -- : Writing /builds/nicolas/concerto/public/assets/pghero/application-cf3eb1374d53ab33bda11ce75b3fb24754f28c6b4f5d34011bcca4c6278809eb.css.gz
I, [2020-12-05T16:20:41.104312 #2654]  INFO -- : Writing /builds/nicolas/concerto/public/assets/pghero/application-52fd530a2be30bc58d62c925ef759f9b41fcb58720aedc5a1225b2e5fa73a488.js
I, [2020-12-05T16:20:41.105402 #2654]  INFO -- : Writing /builds/nicolas/concerto/public/assets/pghero/application-52fd530a2be30bc58d62c925ef759f9b41fcb58720aedc5a1225b2e5fa73a488.js.gz
Compiling...

Here assets compilation work because I run bin/yarn install --immutable just before.

Copy link

Choose a reason for hiding this comment

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

FYI my config is :

and it's already in production 🤟

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated this PR so that only the correct Yarn 2 flags are used. No guide changes anymore, so nobody is confused.

👍 for getting this in 6.1, too.

In 6.2 it can be changed so that only Yarn 2 is supported. Users have some time to switch to Yarn 2 in 6.1 then.

Choose a reason for hiding this comment

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

In 6.2 it can be changed so that only Yarn 2 is supported.

👍

Choose a reason for hiding this comment

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

@mariuspopaadrian
Copy link

Can this be backported to rails 5.2.x branch ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants