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

`yarn install`, webpacker, and `assets:precompile` #744

Closed
rebeccacremona opened this issue Apr 12, 2019 · 6 comments
Closed

`yarn install`, webpacker, and `assets:precompile` #744

rebeccacremona opened this issue Apr 12, 2019 · 6 comments

Comments

@rebeccacremona
Copy link
Contributor

@rebeccacremona rebeccacremona commented Apr 12, 2019

On every call to assets:precompile, yarn install is getting run twice, both times without --frozen-lockfile. It can (and does) install/uninstall packages.

We certainly don't want it running twice. We might not want it running at all: it's not clear to me that asset compilation is the best moment to update node packages, in whatever environment.

Even more fun: the runs can (and do) install different packages, under the right circumstances. For example, if you run RAILS_ENV=development bin/bundle exec rake assets:precompile locally, the first installation move will make sure devDependencies are installed, and the second installation move will make sure devDependencies are removed.

Here's why yarn install is being run at all.

Answer 1: Rails always runs yarn install before assets:precompile https://github.com/rails/rails/blob/v5.2.2.1/railties/lib/rails/tasks/yarn.rake. In Rails 6, it will include the --frozen-lockfile flag, which is great! If we decide we want it to run at all, we should probably override to include that flag.

Answer 2: there is a problem with webpacker. They don't intend for it to redundantly run yarn install. But it is. There are two possible culprits:

  1. rails/webpacker@4d06333#diff-e36986293b3ec8ccf91a5c317454efcf, not yet released, changes the logic for detecting whether or not webpacker:yarn_install needs to run or not. Hopefully that change will work in our situation.

  2. There's also this line: https://github.com/rails/webpacker/blob/master/lib/tasks/webpacker/compile.rake#L49. I've had timing issues when running things like Rake::Task.task_defined?("assets:precompile").... In my experience today, the task isn't always defined at the moment you are querying, but may be defined later!

I think it's just 1)... but it could be that 1) and 2) are both sufficient :-)

The webpacker:yarn_install task is run before the Rails yarn:install task.

Here's why they can install different packages.

Webpacker's compile command will always run in "production" mode, regardless of how NODE_ENV is set.
rails/webpacker#1395
rails/webpacker#1558

NODE_ENV is not explicitly set anywhere in the H2O ecosystem, so far as I know. Instead, it defaults to different values, depending.

Somehow or the other, this can result in the first yarn install running with NODE_ENV=development and the second with NODE_ENV=production. I'll admit I don't totally get it, even though I can watch it happen repeatedly.

What do we want to do?

Dunno.

By adding this to our Rakefile, I was able to override both installation tasks:

# Add your own tasks in files placed in lib/tasks ending in .rake,
# for example lib/tasks/capistrano.rake, and they will automatically be available to Rake.

require File.expand_path('../config/application', __FILE__)

H2o::Application.load_tasks

Rake::Task['yarn:install'].clear
namespace :yarn do
  desc "Override yarn:install. Don't do anything!"
  task :install => [:environment] do
    puts "Cremona is in control of rake."
  end
end

Rake::Task['webpacker:yarn_install'].clear
namespace :webpacker do
  desc "Overridden webpacker:yarn_install. Don't do anything!"
  task :yarn_install => [:environment] do
    puts "Cremona is in control of webpacker."
  end
end

with the results:

$ RAILS_ENV=development NODE_ENV=development bin/bundle exec rake assets:precompile
Cremona is in control of webpacker.
Cremona is in control of rake.
Compiling…
Compiled all packs in /h2o/public/packs

So, we can do whatever we want LOL.

I think we should at least make wepacker:yarn_install a no-op. It IS redundant, whyever it's running.

I think we should probably explicitly set NODE_ENV to production in production. If not in the actual environment (I am in NO position to hazard whether that might break everything), then at least in the asset compilation command, so it becomes RAILS_ENV=production NODE_ENV=production bin/bundle exec rake assets:precompile. It would be nice if we could do it in the actual shell environment, so that ?? yarn commands work as expected consistently, like yarn check ?? (truth?)

I think we should consider not having the asset compilation step do any installation. Or at least add in the frozen lockfile flag.

@rebeccacremona

This comment has been minimized.

Copy link
Contributor Author

@rebeccacremona rebeccacremona commented Apr 12, 2019

Oh, and I'm pretty sure any changes here won't affect the Guard setup, which is running the webpack-dev-server as a separate process... but double-checking is definitely in order! FWIW with the above hack in place, and a round or two of on-the-fly recompilation via Guard, none of my debug messages appeared.

@cgruppioni

This comment has been minimized.

Copy link
Contributor

@cgruppioni cgruppioni commented Apr 15, 2019

Very thorough @rebeccacremona !

I don't know how the deploy process calls yarn install... If it isn't dependent on rails or webpacker in these tasks, if it's not then not running it at all seems ok. @bensteinberg would know the actual answer to this.

Setting NODE_ENV seems like an easy win. However, it is called in the Vue setup https://github.com/harvard-lil/h2o/blob/master/app/webpacker/packs/vue_app.js#L2 - I'm not sure why but I can ask Greg tomorrow.

@bensteinberg

This comment has been minimized.

Copy link
Contributor

@bensteinberg bensteinberg commented Apr 15, 2019

I would love to override the behavior of assets:precompile and call yarn install only when we mean it. @rebeccacremona, if you file a PR, I'll change the salt config as necessary.

@rebeccacremona

This comment has been minimized.

Copy link
Contributor Author

@rebeccacremona rebeccacremona commented Apr 15, 2019

re: Vue's production mode: https://vuejs.org/v2/guide/deployment.html

I think in our case, it's primarily about stripping/silencing warnings and not about anything particularly meaningful. I'll look more thoroughly soon, both to see whether webpacker's override of NODE_ENV kicks in here https://github.com/harvard-lil/h2o/blob/master/app/webpacker/packs/vue_app.js#L2. I suspect it does, during asset precompilation, but have not yet looked.

re: deployment + PR, sounds good to me! PR forthcoming some time this week.

@rebeccacremona

This comment has been minimized.

Copy link
Contributor Author

@rebeccacremona rebeccacremona commented Apr 15, 2019

@cgruppioni It's as I suspected: webpacker is arranging for NODE_ENV to be equal to production inside the pre-compiled assets, no matter what we do. So, explicitly setting NODE_ENV=production on the production and staging servers should be very low risk. (She says, boldly....)

The correct value for Travis is less clear to me. Since Travis is also pre-compiling assets, it should probably be 'production' there too, for clarity.... but

# webpacker.yml
test:
  <<: *default
  compile: true

  # Compile test packs to a separate directory
  public_output_path: packs-test

implies that it's not using the pre-compiled javascript. Do you have an opinion about whether Travis should be using pre-compiled assets, like prod, or on-the-fly assets, like your dev instance does?

Demo that NODE_ENV is production in pre-compiled assets as expected

Add a logging line to vue_app.js like this:

import Vue from "vue/dist/vue.esm";
Vue.config.productionTip = process.env.NODE_ENV == "development";
console.error("I'm the vue script! The NODE_ENV of the process is:", process.env.NODE_ENV)

and then compile assets RAILS_ENV=production bin/bundle exec rake assets:precompile (or RAILS_ENV=development, or NODE_ENV=development... anything, and then run the rails dev server (directly, not through guard) with webpacker.yml tweaked to use pre-compiled assets:

development:
  <<: *default
  compile: false

Load up a casebook in editing mode, and you'll see the console message:

I'm the vue script! The NODE_ENV of the process is: production

Repeat as desired by deleting public packs and then re-compiling assets with RAILS_ENV=development and NODE_ENV=development and any other combinations: wepacker's compile command override is clearly in effect.

@vvo

This comment has been minimized.

Copy link

@vvo vvo commented Jan 7, 2020

@rebeccacremona hi! Anything specific you did to fix that aside from your initial comment? I posted your solution to double heroku yarn installs here (with credits): https://dev.to/vvo/a-rails-6-setup-guide-for-2019-and-2020-hf5#heroku-and-double-yarn-installs and the heroku maintainer said it should not happen here: https://dev.to/vvo/a-rails-6-setup-guide-for-2019-and-2020-hf5#heroku-and-double-yarn-installs but I never got back again. Let me know!

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

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.