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

Re-compile file when environment variable changed #366

Closed
schneems opened this Issue Aug 18, 2016 · 19 comments

Comments

Projects
None yet
10 participants
@schneems
Member

schneems commented Aug 18, 2016

We need some kind of way for sprockets to evaluate an environment variable so that it knows when it needs to recompile files.

Problem: Currently if you have an asset that looks like this:

background-image: <%= ENV["HOME_LOGO"] %>

And you modify HOME_LOGO then sprockets won't see a change on disk so it won't know to re-compile the asset.

Suggested solution:

We could add a directive, something like

depends_on_value = <%= ENV["HOME_LOGO"] %>

Or maybe depends_on_env_var HOME_LOGO so that sprockets knows to check that value when computing if the cache is correct.

Ideally we would do this automagically kind of like we auto add dependencies in sprocket-rails. Not sure about it though. Might be too magical. In the intermediate we need a way to manually solve the problem instead of rev-ing the cache version for ALL assets.

@rafaelfranca

This comment has been minimized.

Show comment
Hide comment
@rafaelfranca

rafaelfranca Aug 18, 2016

Member

Haha. @bouk just implement this in sprockets-commoner. I think he will send a PR later.

Member

rafaelfranca commented Aug 18, 2016

Haha. @bouk just implement this in sprockets-commoner. I think he will send a PR later.

@rafaelfranca

This comment has been minimized.

Show comment
Hide comment
@rafaelfranca

rafaelfranca Aug 18, 2016

Member

Wait the PR is already here #365. @schneems is there something that it doesn't solve that you have in mind?

Member

rafaelfranca commented Aug 18, 2016

Wait the PR is already here #365. @schneems is there something that it doesn't solve that you have in mind?

@matthewd

This comment has been minimized.

Show comment
Hide comment
@matthewd

matthewd Aug 19, 2016

Member

We could have a method that means "depend on the value of this parameter"...

background-image: <%= depend_on_value(ENV["HOME_LOGO"]) %>

Then if any such call is encountered, we evaluate the erb (only) on this particular asset whenever we need to check its dependencies. That's about the only workable approach I can think of that matches the power of ERB with something that can actually invalidate it.

It seems to work conceptually -- erb is always evaluated first, and unlike the other processors, shouldn't add enormous overhead to re-run... but I've not looked at how architecturally feasible it would be.


#365's ENV-specific thing feels like a hack to me (sprockets giving ENV special treatment; the implementation-aware way it gets registered), but does seem like a serviceable stop-gap for a common case.

Member

matthewd commented Aug 19, 2016

We could have a method that means "depend on the value of this parameter"...

background-image: <%= depend_on_value(ENV["HOME_LOGO"]) %>

Then if any such call is encountered, we evaluate the erb (only) on this particular asset whenever we need to check its dependencies. That's about the only workable approach I can think of that matches the power of ERB with something that can actually invalidate it.

It seems to work conceptually -- erb is always evaluated first, and unlike the other processors, shouldn't add enormous overhead to re-run... but I've not looked at how architecturally feasible it would be.


#365's ENV-specific thing feels like a hack to me (sprockets giving ENV special treatment; the implementation-aware way it gets registered), but does seem like a serviceable stop-gap for a common case.

@schneems

This comment has been minimized.

Show comment
Hide comment
@schneems

schneems Aug 25, 2016

Member

I think i'm leaning towards having ENV['some_value'] auto add an env dependency to an asset for this case. While it's special casing ENV, i don't really see many people manipulating assets via other means (globals, constants, thread locals). I think I want to solve this case out of the box i.e. someone who has never used sprockets before doesn't know they have to add a magic comment to get it to re-compile.

Member

schneems commented Aug 25, 2016

I think i'm leaning towards having ENV['some_value'] auto add an env dependency to an asset for this case. While it's special casing ENV, i don't really see many people manipulating assets via other means (globals, constants, thread locals). I think I want to solve this case out of the box i.e. someone who has never used sprockets before doesn't know they have to add a magic comment to get it to re-compile.

@ysyyork

This comment has been minimized.

Show comment
Hide comment
@ysyyork

ysyyork Sep 3, 2016

I agree with @matthewd . my use case is that I'm using non-env variables in my js, so adding env dependency is not a good solution for me. Adding a syntax to explicitly declare the dependency seems much more flexible.

ysyyork commented Sep 3, 2016

I agree with @matthewd . my use case is that I'm using non-env variables in my js, so adding env dependency is not a good solution for me. Adding a syntax to explicitly declare the dependency seems much more flexible.

@schneems

This comment has been minimized.

Show comment
Hide comment
@schneems

schneems Sep 6, 2016

Member

@ysyyork can you give me some example code that shows the exact problem you're hitting? There may already be an existing API to handle your case.

Member

schneems commented Sep 6, 2016

@ysyyork can you give me some example code that shows the exact problem you're hitting? There may already be an existing API to handle your case.

@ysyyork

This comment has been minimized.

Show comment
Hide comment
@ysyyork

ysyyork Sep 7, 2016

Hi @schneems, thanks. Sure. So here is a snippet of my code. In a js file with path app/assets/javascripts/config.js.erb like this

var aSettingsVariableThatShouldBeUsedLater = <%= Settings.variable %>

var useThisSettingsVariable = aSettingsVariableThatShouldBeUsedLater + "/test"

Here I use the Config gem (https://github.com/railsconfig/config) to set environment variables. Then when I do precompile for the first time, this code works well. But if I change settings variable in the .yml file, I precompile again, the digest won't change and browser will just use the out-dated version of js file.

ysyyork commented Sep 7, 2016

Hi @schneems, thanks. Sure. So here is a snippet of my code. In a js file with path app/assets/javascripts/config.js.erb like this

var aSettingsVariableThatShouldBeUsedLater = <%= Settings.variable %>

var useThisSettingsVariable = aSettingsVariableThatShouldBeUsedLater + "/test"

Here I use the Config gem (https://github.com/railsconfig/config) to set environment variables. Then when I do precompile for the first time, this code works well. But if I change settings variable in the .yml file, I precompile again, the digest won't change and browser will just use the out-dated version of js file.

@matthewd

This comment has been minimized.

Show comment
Hide comment
@matthewd

matthewd Sep 7, 2016

Member

Yeah, while I'm now convinced it's worthwhile to special-case ENV so that it completely Just Works, I still think we should do the thing I described to support arbitrary expressions. There's a reason we boot the whole application when compiling assets, after all.


i don't really see many people manipulating assets via other means

I suspect there might be some selection bias at play there.

I think I want to solve this case out of the box i.e. someone who has never used sprockets before doesn't know they have to add a magic comment to get it to re-compile.

For ENV-users, we can meet that goal with #365. But given we're exposing a full ruby environment, it seems reasonable to seek better than "bump the global asset version" for people doing other things. (Did we restore the global asset version behaviour, or is that still pending?)


In short: I think there's room for a generic thing [which people would unfortunately have to opt in to] as well as the ENV-aware approach. But that's room in the "PRs welcome" sense; the specific need described in this issue is addressed by the more focused solution in #365.

Member

matthewd commented Sep 7, 2016

Yeah, while I'm now convinced it's worthwhile to special-case ENV so that it completely Just Works, I still think we should do the thing I described to support arbitrary expressions. There's a reason we boot the whole application when compiling assets, after all.


i don't really see many people manipulating assets via other means

I suspect there might be some selection bias at play there.

I think I want to solve this case out of the box i.e. someone who has never used sprockets before doesn't know they have to add a magic comment to get it to re-compile.

For ENV-users, we can meet that goal with #365. But given we're exposing a full ruby environment, it seems reasonable to seek better than "bump the global asset version" for people doing other things. (Did we restore the global asset version behaviour, or is that still pending?)


In short: I think there's room for a generic thing [which people would unfortunately have to opt in to] as well as the ENV-aware approach. But that's room in the "PRs welcome" sense; the specific need described in this issue is addressed by the more focused solution in #365.

@schneems

This comment has been minimized.

Show comment
Hide comment
@schneems

schneems Sep 7, 2016

Member

I agree we should have a generic API for supporting this case.

Member

schneems commented Sep 7, 2016

I agree we should have a generic API for supporting this case.

@ysyyork

This comment has been minimized.

Show comment
Hide comment
@ysyyork

ysyyork Sep 7, 2016

Great to hear. But for current usage, is there any workaround? Now every time I change ENV variables, I have to manually increment the version number in assets.rb so that all the assets can be re-precompiled. But this is really time consuming cus all the assets will be re-precompiled.

ysyyork commented Sep 7, 2016

Great to hear. But for current usage, is there any workaround? Now every time I change ENV variables, I have to manually increment the version number in assets.rb so that all the assets can be re-precompiled. But this is really time consuming cus all the assets will be re-precompiled.

@bouk

This comment has been minimized.

Show comment
Hide comment
@bouk

bouk Sep 8, 2016

Member

What we've been doing at shopify is extending DirectiveProcessor:

require 'sprockets'

class DependOnFileDirectiveProcessor < Sprockets::DirectiveProcessor
  def process_depend_on_file_directive(path)
    uri, deps = @environment.resolve!(path, load_paths: [::Rails.root.to_s])
    @dependencies.merge(deps)
    uri
  end
end

If you then have a file that reads some sort of config, you can do //= depend_on_file config/settings.yml and the file would be recalculated if the file changes (although the way the settings are loaded does need to support reloading.

ENV is a special case, because its values aren't dependant on a certain file, which is why I made #365

Member

bouk commented Sep 8, 2016

What we've been doing at shopify is extending DirectiveProcessor:

require 'sprockets'

class DependOnFileDirectiveProcessor < Sprockets::DirectiveProcessor
  def process_depend_on_file_directive(path)
    uri, deps = @environment.resolve!(path, load_paths: [::Rails.root.to_s])
    @dependencies.merge(deps)
    uri
  end
end

If you then have a file that reads some sort of config, you can do //= depend_on_file config/settings.yml and the file would be recalculated if the file changes (although the way the settings are loaded does need to support reloading.

ENV is a special case, because its values aren't dependant on a certain file, which is why I made #365

@schneems

This comment has been minimized.

Show comment
Hide comment
@schneems

schneems Sep 8, 2016

Member

Great call on

//= depend_on_file config/settings.yml

Between that and env var detection I can't think of a case where we can't detect the variable change.

There are edge cases where the env var may be coming from a file in a gem where sprockets might not know how to easily find it. But it would still technically be possible.

Member

schneems commented Sep 8, 2016

Great call on

//= depend_on_file config/settings.yml

Between that and env var detection I can't think of a case where we can't detect the variable change.

There are edge cases where the env var may be coming from a file in a gem where sprockets might not know how to easily find it. But it would still technically be possible.

@vincentwoo

This comment has been minimized.

Show comment
Hide comment
@vincentwoo

vincentwoo Sep 14, 2016

@bouk I basically use the same depend on file config in my app, but I don't have the last mile to auto-update the in-memory object when the YAML changes. Do you have a recommendation for that?

vincentwoo commented Sep 14, 2016

@bouk I basically use the same depend on file config in my app, but I don't have the last mile to auto-update the in-memory object when the YAML changes. Do you have a recommendation for that?

@pujiaxun

This comment has been minimized.

Show comment
Hide comment
@pujiaxun

pujiaxun Oct 7, 2016

I am meeting the same issue. I use a variable of model attribute from database, in a file whose name ends with '.scss.erb'.
So the method 'depend_on' seems not fit me. What I want is an API for triggering a recompiling by some ruby codes in Models or Controllers.
Anyone can help me? Thanks.

EDIT:
I found a way to hack it and wrote a passage, which may help.
http://www.jasonsi.com/2016/10/08/25/

pujiaxun commented Oct 7, 2016

I am meeting the same issue. I use a variable of model attribute from database, in a file whose name ends with '.scss.erb'.
So the method 'depend_on' seems not fit me. What I want is an API for triggering a recompiling by some ruby codes in Models or Controllers.
Anyone can help me? Thanks.

EDIT:
I found a way to hack it and wrote a passage, which may help.
http://www.jasonsi.com/2016/10/08/25/

@yivo

This comment has been minimized.

Show comment
Hide comment
@yivo

yivo Oct 7, 2016

I have the same issue.
I use sprockets ERB processing to embed momentJS locales.
moment.js.erb:

<% # English is included in momentJS itself %>
<% I18n.available_locales.select { |x| x != :en }.each do |locale| %>
  <% require_asset "bower_components/moment/locale/#{locale}" %>
<% end %>

When I change I18n.available_locales this will not be recompiled.
It will be very cool feature if we'll able to depend asset on ruby code not just on other assets, files, env...
//= depend_on_value moment_locales_state

class Sprockets::Context
  def moment_locales_state
    I18n.available_locales
  end
end

yivo commented Oct 7, 2016

I have the same issue.
I use sprockets ERB processing to embed momentJS locales.
moment.js.erb:

<% # English is included in momentJS itself %>
<% I18n.available_locales.select { |x| x != :en }.each do |locale| %>
  <% require_asset "bower_components/moment/locale/#{locale}" %>
<% end %>

When I change I18n.available_locales this will not be recompiled.
It will be very cool feature if we'll able to depend asset on ruby code not just on other assets, files, env...
//= depend_on_value moment_locales_state

class Sprockets::Context
  def moment_locales_state
    I18n.available_locales
  end
end
@schneems

This comment has been minimized.

Show comment
Hide comment
@schneems

schneems Oct 13, 2016

Member

Are you keeping your locales in a yml file or in a database? If it's a file you can depend on the file.

I'm interested to get use cases for non-file based values in assets. I18n stored in a database seems like a valid use case, however then you have the problem of if you change a value, it won't be updated in your assets until the next time you deploy.

Member

schneems commented Oct 13, 2016

Are you keeping your locales in a yml file or in a database? If it's a file you can depend on the file.

I'm interested to get use cases for non-file based values in assets. I18n stored in a database seems like a valid use case, however then you have the problem of if you change a value, it won't be updated in your assets until the next time you deploy.

@reidcooper

This comment has been minimized.

Show comment
Hide comment
@reidcooper

reidcooper Apr 26, 2017

Cause I don't know how to correctly word my problem but it is the same problem going on in this ticket, @bouk mentioned a great solution to my problem (which is for YML files). However, I cannot see it working.

app/assets/javascripts/platform/backbone/modules/experience_manager.js.erb

//= depend_on_file 'config/browser_experiences.yml'

MW.ExperienceManager = function(ExperienceManager, mw_app, Backbone, Marionette, $, _){
  var _experience_restrictions = <%= BrowserExperience::Orchestrator.append_restrictions(:to_json) %>;
}

app/contexts/browser_experience/depend_on_file_directive_processor.rb

require 'sprockets'

class DependOnFileDirectiveProcessor < Sprockets::DirectiveProcessor
  def process_depend_on_file_directive(path)
    uri, deps = @environment.resolve!(path, load_paths: [::Rails.root.to_s])
    @dependencies.merge(deps)
    uri
  end
end

application.rb

config.assets.configure do |env|
      env.unregister_processor('text/js', Sprockets::DirectiveProcessor)
      env.register_processor('text/js', DependOnFileDirectiveProcessor)
    end

When I change one of the integer values within my YML file, it does not seem to want to update the digest value when I deploy to heroku.

========== UPDATE, Current Implemenation

I am now trying a class decorator on Sprockets::DirectiveProcessor. I have removed the work I had in application.rb above.

app/contexts/sprockets/process_depend_on_config_directive.rb

# https://gist.github.com/dylanjha/11233766
# hints from https://github.com/rails/sprockets/issues/366

Sprockets::DirectiveProcessor.class_eval do
  def process_depend_on_config_directive(file)
    path = File.expand_path(file, "#{Rails.root}/config")
    context.depend_on(path)
  end
end

app/assets/javascripts/platform/backbone/modules/experience_manager.js.erb

//= depend_on_config 'browser_experiences.yml'

MW.ExperienceManager = function(ExperienceManager, mw_app, Backbone, Marionette, $, _){
  var _experience_restrictions = <%= BrowserExperience::Orchestrator.append_restrictions(:to_json) %>;
}

This still will not bust the cache even though I change a value in my yml file.

========== UPDATE 2:

I believe I am just crazy haha, but I believe I got this working. It was due to me caching the yml file separately which is being used for another purpose. Once I temporarily removed the caching block, it does properly update the javascript digest once deployed.

Sorry for the wildfire, if anything I hope my work above gives some insight.

reidcooper commented Apr 26, 2017

Cause I don't know how to correctly word my problem but it is the same problem going on in this ticket, @bouk mentioned a great solution to my problem (which is for YML files). However, I cannot see it working.

app/assets/javascripts/platform/backbone/modules/experience_manager.js.erb

//= depend_on_file 'config/browser_experiences.yml'

MW.ExperienceManager = function(ExperienceManager, mw_app, Backbone, Marionette, $, _){
  var _experience_restrictions = <%= BrowserExperience::Orchestrator.append_restrictions(:to_json) %>;
}

app/contexts/browser_experience/depend_on_file_directive_processor.rb

require 'sprockets'

class DependOnFileDirectiveProcessor < Sprockets::DirectiveProcessor
  def process_depend_on_file_directive(path)
    uri, deps = @environment.resolve!(path, load_paths: [::Rails.root.to_s])
    @dependencies.merge(deps)
    uri
  end
end

application.rb

config.assets.configure do |env|
      env.unregister_processor('text/js', Sprockets::DirectiveProcessor)
      env.register_processor('text/js', DependOnFileDirectiveProcessor)
    end

When I change one of the integer values within my YML file, it does not seem to want to update the digest value when I deploy to heroku.

========== UPDATE, Current Implemenation

I am now trying a class decorator on Sprockets::DirectiveProcessor. I have removed the work I had in application.rb above.

app/contexts/sprockets/process_depend_on_config_directive.rb

# https://gist.github.com/dylanjha/11233766
# hints from https://github.com/rails/sprockets/issues/366

Sprockets::DirectiveProcessor.class_eval do
  def process_depend_on_config_directive(file)
    path = File.expand_path(file, "#{Rails.root}/config")
    context.depend_on(path)
  end
end

app/assets/javascripts/platform/backbone/modules/experience_manager.js.erb

//= depend_on_config 'browser_experiences.yml'

MW.ExperienceManager = function(ExperienceManager, mw_app, Backbone, Marionette, $, _){
  var _experience_restrictions = <%= BrowserExperience::Orchestrator.append_restrictions(:to_json) %>;
}

This still will not bust the cache even though I change a value in my yml file.

========== UPDATE 2:

I believe I am just crazy haha, but I believe I got this working. It was due to me caching the yml file separately which is being used for another purpose. Once I temporarily removed the caching block, it does properly update the javascript digest once deployed.

Sorry for the wildfire, if anything I hope my work above gives some insight.

@stepheneb

This comment has been minimized.

Show comment
Hide comment
@stepheneb

stepheneb Sep 14, 2017

Am also looking to get asset cache marked dirty and rebuilt when styles change because of change in application setting used in ERB block in application.scss.erb.

I just spent a number of hours with this problem and assumed at first I had a problem with Capistrano: capistrano/rails#208.

I then made what I thought was a workaround using //= depend_on but getting changes noticed was problematic. See new issue #500

stepheneb commented Sep 14, 2017

Am also looking to get asset cache marked dirty and rebuilt when styles change because of change in application setting used in ERB block in application.scss.erb.

I just spent a number of hours with this problem and assumed at first I had a problem with Capistrano: capistrano/rails#208.

I then made what I thought was a workaround using //= depend_on but getting changes noticed was problematic. See new issue #500

@bouk

This comment has been minimized.

Show comment
Hide comment
@bouk

bouk Sep 14, 2017

Member

Closing this as the original issue has been resolved

Member

bouk commented Sep 14, 2017

Closing this as the original issue has been resolved

@bouk bouk closed this Sep 14, 2017

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