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

Allow compile option to be settable #2063

Closed

Conversation

maschwenk
Copy link
Contributor

@maschwenk maschwenk commented Apr 19, 2019

Similar to check_yarn_integrity=, the changes to RAILS_ENV/NODE_ENV setting has made it basically impossible to mix pre-compilation with on-the-fly compilation for a given RAILS_ENV.

In my setup, I precompile assets in CI, run them through tests (RAILS_ENV=test), and deploy them to production (RAILS_ENV=production) all based on the same precompiled assets.

The two options I have are:

  1. Create a separate RAILS_ENV=test_ci and change those settings in webpacker.yml to conform with that. Maintaining multiple RAILS_ENV's is confusing and not worth the effort in my opinion.
  2. Require local RAILS_ENV=test to use pre-compilation. That is not a good development experience.

I would use this setter to basically conditionally turn on the compile flag based on whether the ENV['CI'] flag was set (very common in pretty much every CI provider). I used to hack around this pre-3.5.x by just changing the NODE_ENV on the fly. This no longer works.

As a maintainer @gauravtiwari I can understand why you might not be interested in allowing another form of configuration, but I think maybe leaving it undocumented could be one way to provide sharp tools. Alternatively, my sharp tools can be directly altering the @data hash of Webpacker configuration. Maybe that is sufficient, but curious to hear your thoughts.

P.S. The other hack I'm doing is sym-linking packs to packs-test in CI, so that prod/test seamlessly use the same pre-compiled packs location

@jakeNiemiec
Copy link
Member

jakeNiemiec commented Apr 19, 2019

I would use this setter to basically conditionally turn on the compile flag based on whether the ENV['CI'] flag was set (very common in pretty much every CI provider). I used to hack around this pre-3.5.x by just changing the NODE_ENV on the fly. This no longer works.

I think maybe leaving it undocumented could be one way to provide sharp tools. Alternatively, my sharp tools can be directly altering the @DaTa hash of Webpacker configuration. Maybe that is sufficient, but curious to hear your thoughts.

This is a good case in favor of an eject feature (eg #1916). Some people know what they are doing and don't need the guard-rails trying to hamper their efforts.

@maschwenk
Copy link
Contributor Author

maschwenk commented Apr 19, 2019

my current sharp tools are:

if ENV['RAILS_ASSET_PRECOMPILE'] == 'true'  # this is custom ENV var we set ourselves
  new_config = Webpacker.config.instance_variable_get(:@data).merge(public_output_path: 'packs', compile: false)
  Webpacker.config.instance_variable_set(:@data, new_config)
end

which is perfectly sufficient but has obvious drawbacks

@dbalatero
Copy link
Contributor

@maschwenk I do:

require "webpacker/configuration"

if Rails.env.test? && ENV["CI"]
  module PreventCompilingPacksInCI
    def compile?
      false
    end
  end

  class Webpacker::Configuration
    prepend PreventCompilingPacksInCI
  end
end

@maschwenk
Copy link
Contributor Author

I'm leaning towards closing this. I've already worked around it and I think the valid approach would either be the Eject feature or just doing hacks at our own risk. If we go down this road we'll have 15 different setters and I think that's worse.

@maschwenk maschwenk closed this May 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants