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

include_ember_* does not appear to honor asset prefix/fingerprint? #298

Closed
thejchap opened this issue Nov 22, 2015 · 40 comments
Closed

include_ember_* does not appear to honor asset prefix/fingerprint? #298

thejchap opened this issue Nov 22, 2015 · 40 comments
Labels

Comments

@thejchap
Copy link

might be missing something...during heroku deploy...does this have to do with the custom RAILS_ENV? Still investigating

In view

include_ember_stylesheet_tags :dashboard
include_ember_script_tags :dashboard

In application config (RAILS_ENV='staging'):

config.action_controller.asset_host = 'https://d3dncadikrv0jv.cloudfront.net'
config.assets.prefix = '/staging/assets'
config.assets.js_compressor = :uglifier # minifyJS set to false in Brocfile

The rendered script tags/style tags look like this (notice asset prefix and fingerprint are not present)....they are for all the rest of the rails assets (application.js/css, etc):

<script src="https://d3dncadikrv0jv.cloudfront.net/javascripts/dashboard/assets/vendor.js"></script>
<script src="https://d3dncadikrv0jv.cloudfront.net/javascripts/dashboard/assets/dashboard.js"></script>
<link rel="stylesheet" media="screen" href="https://d3dncadikrv0jv.cloudfront.net/stylesheets/dashboard/assets/vendor.css" />
<link rel="stylesheet" media="screen" href="https://d3dncadikrv0jv.cloudfront.net/stylesheets/dashboard/assets/dashboard.css" />
@thejchap
Copy link
Author

Appears to be the case in RAILS_ENV=production as well

@seanpdoyle
Copy link
Contributor

@thejchap thanks for opening this issue.

There seems to be a few things in play here.

For starters, I would expect the asset URLs to start with /assets instead of /javascripts and /stylesheets. That fact leads me to believe that your URLs point to missing assets.

Second, if you're going to be serving the assets from a CDN, and/or are using a Rails-side prefix, you need to configure the addon accordingly:

https://github.com/rondale-sc/ember-cli-rails-addon/#configuration

Try something like this:

// dashboard/ember-cli-build.js

function assetHostAndPrefix(environment) {
  if (environment.match(/staging|production/i)) {
    return 'https://d3dncadikrv0jv.cloudfront.net/' + environment + 'assets/';
  }
}

module.exports = function(defaults) {
  var app = new EmberApp(defaults, {
    'ember-cli-rails': {
      prepend: assetHostAndPrefix(EmberApp.env()),
    }
  });
};

Could you give that a shot and report back with your results?

@thejchap
Copy link
Author

@seanpdoyle Thanks for the reply

No luck....it appears as if all the assets are being fingerprinted properly and put in the right place during rake assets:precompile....however Rails.application.assets doesn't include anything that looks like the vendor.js/css/etc from the ember app. It does include /app/tmp/ember-cli-913b6133-b6dd-4325-9ae3-050706759883/assets though

And thanks for the link to the config...I'm not using the index.html file from the ember app...the JS and CSS are just embedded in a rails view via the above helpers and the app's rootElement is set to an element in the view. I did update this anyways though

Very strange, because the Rails assets in app/assets are all referenced/included properly

@seanpdoyle
Copy link
Contributor

@thejchap thanks for the additional information. I'll investigate further.

As an alternative suggestion:

Have you considered not setting config.assets.prefix = '/staging/assets'?

Given that you're fingerprinting your assets, there won't ever be name collisions between staging and production assets, since the fingerprints take into account the contents of the files. The only way names would collide is if their contents were equivalent (in that case, they'd be identical and it wouldn't really be a "collision").

TL;DR you can safely serve / store / cache fingerprinted files from the same bucket

@thejchap
Copy link
Author

@seanpdoyle thanks I'll let you know if I come across anything else.

Re: not separating the assets into different folders - yeah, theoretically should be fine, and I will try this to see if it changes anything, but not a huge fan of the idea of having production and staging pointing to the same files....even given the fact that the file and its contents are the same

@seanpdoyle
Copy link
Contributor

@thejchap could you try pulling down

rondale-sc/ember-cli-rails-addon#22

// dashboard/package.json

{
  "devDependencies": {
    "ember-cli-rails-addon": "rondale-sc/ember-cli-rails-addon#sd-expose-prefix"
  }
}

and configuring the addon with your prefix:

// dashboard/ember-cli-build.js

function emberCliRailsConfiguration(environment) {
  if (environment.match(/staging|production/i)) {
    return {
      prepend: 'https://d3dncadikrv0jv.cloudfront.net/',
      prefix: environment + 'assets/',
    }
  } else {
    return {};
  }
}

module.exports = function(defaults) {
  var app = new EmberApp(defaults, {
    'ember-cli-rails': emberCliRailsConfiguration(EmberApp.env()),
  });
};

If that works, I'll publish it as part of 0.5.2 and close this issue.

@thejchap
Copy link
Author

@seanpdoyle thanks! no luck unfortunately...the index.html in the ember app still gets the asset urls prefixed correctly (as it did with the earlier snippet), however the include_ember_*_tags helpers still point to https://d3dncadikrv0jv.cloudfront.net/javascripts/dashboard/assets/vendor.js without the fingerprint/prefix.

Also FYI, as far as I know, EmberApp.env() only returns development or production...in order to access the correct value for which env we're in in ember-cli-build.js I've been referencing process.env.RAILS_ENV

I am going to try wiping out all CDN configuration/prefix/asset_host/etc and see if they get linked to properly

@seanpdoyle
Copy link
Contributor

Also FYI, as far as I know, EmberApp.env() only returns development or production...in order to access > the correct value for which env we're in in ember-cli-build.js I've been referencing
process.env.RAILS_ENV

That might be part of the problem.

Unless you're taking additional steps, Ember won't know how to treat RAILS_ENV=staging.

"production" in this sense represents a production-like build.

To that end, the addon doesn't currently fingerprint non-production-like builds (for the sake of non-fingerprinted URLs in development):

https://github.com/rondale-sc/ember-cli-rails-addon/blob/7745bddf33bd14a007edea654f63ad774dd94155/index.js#L50-L52

Could you try not passing process.env.RAILS_ENV?

@seanpdoyle
Copy link
Contributor

helpers still point to https://d3dncadikrv0jv.cloudfront.net/javascripts/dashboard/assets/vendor.js

I'm very surprised by this.

This line:

prefix: environment + 'assets/',

should ensure the prefix makes it's way into the URL.

@seanpdoyle
Copy link
Contributor

I have a hunch this is our culprit:

def assets
["#{app.name}/assets/vendor", "#{app.name}/assets/#{ember_app_name}"]
end

def assets
  ["#{app.name}/assets/vendor", "#{app.name}/assets/#{ember_app_name}"]
end

I'll investigate and push up a branch with a fix

@thejchap
Copy link
Author

Unless you're taking additional steps, Ember won't know how to treat RAILS_ENV=staging.
"production" in this sense represents a production-like build.

Right, however the behavior is still the same even when RAILS_ENV=production. Just confirmed in production that the helpers still point to the URLs mentioned above

Thanks for all your help on this! This is an upgrade from 0.3.0 FWIW

seanpdoyle added a commit to seanpdoyle/ember-cli-rails-heroku-example that referenced this issue Nov 23, 2015
@seanpdoyle
Copy link
Contributor

@thejchap I've tried to reproduce this bug here:

https://github.com/seanpdoyle/ember-cli-rails-heroku-example/compare/sd-add-asset-tag-example

Could you pull that down, make it work more like your production app in whatever way you need to?

If you're able to reproduce the error, could you fork that repo and make a PR against that branch emphasizing the diff necessary to reproduce this bug?

@thejchap
Copy link
Author

@seanpdoyle Here you go:
seanpdoyle/ember-cli-rails-heroku-example#6

Here is the app deployed on Heroku:
https://asset-tag-example-repro.herokuapp.com/

Here are the include helpers failing
https://asset-tag-example-repro.herokuapp.com/helpers

@botandrose
Copy link
Contributor

FWIW, I'm seeing this too, and I'm not doing anything unusual with asset hosts, prefixes, etc. Everything fine on dev and test, but no fingerprint on production.

@botandrose
Copy link
Contributor

Huh, now also seeing it after a delay on development. Assets come through at first, several minutes later they are 404. A rails server restart fixes this. May or may not be related to other issue. I'll investigate further.

@botandrose
Copy link
Contributor

Looks like its consistently ~5 minutes after Rails server restart. I currently suspect this is unrelated, so splitting off into a new issue.

@seanpdoyle
Copy link
Contributor

Disregarding @botandrose's comments (that conversation is ongoing in #304), I think I know what's going on here.

Rails isn't responsible for generating the fingerprints (EmberCLI handles that).

When serving the EmberCLI-generated index.html, everything works fine, since the URLs are fingerprinted (in production, development deliberately disables fingerprinting -- maybe it shouldn't) and injected into the HTML without any asset pipeline involvement.

However, Rails still thinks the assets are named frontend/assets/frontend.js, and frontend/assets/vendor.js when they're actually frontend/assets/frontend-deadbeefabc123 and frontend/assets/vendor-abc123deadbeef (or another actually hashed-URL).

We currently handle this disconnect by using https://github.com/alexspeller/non-stupid-digest-assets to allow Rails' references to frontend/assets/frontend.js survive the asset pre-compilation step. Unfortunately, this doesn't work once we introduce EmberCLI's fingerprinting into the process.

I'll do my best to add a failing test case to the test suite that covers this functionality. Once we have that, we can start working towards a solution.

One possible solution is to force EmberCLI to generate a Rails-compatible manifest file, which we can monkey with and merge into Sprocket's version.

Thanks for your patience in this.

@seanpdoyle seanpdoyle added the bug label Nov 24, 2015
@thejchap
Copy link
Author

@seanpdoyle ah! got it. makes a lot of sense. i am happy to work on a PR if you'd like...if not lmk if there's anything else I can do to help. thanks for all your hard work on this....great tool

@seanpdoyle
Copy link
Contributor

@thejchap @botandrose could you two please try with the following:

# Gemfile

gem "ember-cli-rails", github: "thoughtbot/ember-cli-rails", branch: "sd-manifest"
// frontend/package.json

{
  "devDependencies": {
    "ember-cli-rails-addon": "rondale-sc/ember-cli-rails-addon#sd-manifest"
  }
}

Both files are experiments and won't land until we confirm they work.

@botandrose
Copy link
Contributor

@seanpdoyle Booted my app in production, and the ember assets are coming through! Great work!

@thejchap
Copy link
Author

@seanpdoyle nice! confirmed for me too

@thejchap
Copy link
Author

@seanpdoyle FYI getting this locally in RAILS_ENV=development. Looks like its coming from here:

https://github.com/thoughtbot/ember-cli-rails/compare/sd-manifest?expand=1#diff-c366a180fec0d75b75125290586270b1R82

TypeError - no implicit conversion of nil into String:
  /opt/rubies/2.2.2/lib/ruby/gems/2.2.0/bundler/gems/ember-cli-rails-2418cd64e045/lib/ember_cli/path_set.rb:82:in `manifest'
  /opt/rubies/2.2.2/lib/ruby/gems/2.2.0/bundler/gems/ember-cli-rails-2418cd64e045/lib/ember_cli/sprockets.rb:60:in `ember_manifest'
  /opt/rubies/2.2.2/lib/ruby/gems/2.2.0/bundler/gems/ember-cli-rails-2418cd64e045/lib/ember_cli/sprockets.rb:42:in `update!'
  /opt/rubies/2.2.2/lib/ruby/gems/2.2.0/bundler/gems/ember-cli-rails-2418cd64e045/lib/ember_cli/app.rb:100:in `copy_index_html_file'
  /opt/rubies/2.2.2/lib/ruby/gems/2.2.0/bundler/gems/ember-cli-rails-2418cd64e045/lib/ember_cli/app.rb:84:in `build_and_watch'
  /opt/rubies/2.2.2/lib/ruby/gems/2.2.0/bundler/gems/ember-cli-rails-2418cd64e045/lib/ember_cli/app.rb:43:in `build'

@thejchap
Copy link
Author

@seanpdoyle yeah, looks like in development mode the generated script tags are empty

seanpdoyle added a commit to rondale-sc/ember-cli-rails-addon that referenced this issue Dec 1, 2015
Closes [thoughtbot/ember-cli-rails#298][#298].

For Rails to integrate Ember into the asset pipeline, EmberCLI must
generate a Sprockets-compatible manifest file, which Rails will merge
into its own Asset manifest.

[#298]: thoughtbot/ember-cli-rails#298
seanpdoyle added a commit that referenced this issue Dec 1, 2015
Closes [#298]

Expose Ember's fingerprinted assets to Sprockets.

[#298]: #298
seanpdoyle added a commit that referenced this issue Dec 1, 2015
Closes [#298]

Expose Ember's fingerprinted assets to Sprockets.

[#298]: #298
seanpdoyle added a commit to rondale-sc/ember-cli-rails-addon that referenced this issue Dec 1, 2015
Closes [thoughtbot/ember-cli-rails#298][#298].

For Rails to integrate Ember into the asset pipeline, EmberCLI must
generate a Sprockets-compatible manifest file, which Rails will merge
into its own Asset manifest.

[#298]: thoughtbot/ember-cli-rails#298
seanpdoyle added a commit that referenced this issue Dec 1, 2015
Closes [#298]

Expose Ember's fingerprinted assets to Sprockets.

[#298]: #298
seanpdoyle added a commit that referenced this issue Dec 1, 2015
Closes [#298]

Expose Ember's fingerprinted assets to Sprockets.

[#298]: #298
@thejchap
Copy link
Author

thejchap commented Dec 1, 2015

@seanpdoyle which branch should i be on for the addon?

@seanpdoyle
Copy link
Contributor

@thejchap sd-manifest (rondale-sc/ember-cli-rails-addon#23)

seanpdoyle added a commit that referenced this issue Dec 2, 2015
Closes [#298]

Expose Ember's fingerprinted assets to Sprockets.

No longer support Sprockets-based helpers for Rails 3.2, given the
difference in how the Asset Pipeline works post-4.0.

[#298]: #298
seanpdoyle added a commit that referenced this issue Dec 2, 2015
Closes [#298]

Expose Ember's fingerprinted assets to Sprockets.

No longer support Sprockets-based helpers for Rails 3.2, given the
difference in how the Asset Pipeline works post-4.0.

[#298]: #298
seanpdoyle added a commit that referenced this issue Dec 2, 2015
Closes [#298]

Expose Ember's fingerprinted assets to Sprockets.

No longer support Sprockets-based helpers for Rails 3.2, given the
difference in how the Asset Pipeline works post-4.0.

[#298]: #298
seanpdoyle added a commit that referenced this issue Dec 2, 2015
Closes [#298]

Expose Ember's fingerprinted assets to Sprockets.

No longer support Sprockets-based helpers for Rails 3.2, given the
difference in how the Asset Pipeline works post-4.0.

[#298]: #298
seanpdoyle added a commit that referenced this issue Dec 2, 2015
Closes [#298]

Expose Ember's fingerprinted assets to Sprockets.

No longer support Sprockets-based helpers for Rails 3.2, given the
difference in how the Asset Pipeline works post-4.0.

[#298]: #298
seanpdoyle added a commit that referenced this issue Dec 2, 2015
Closes [#298]

Expose Ember's fingerprinted assets to Sprockets.

No longer support Sprockets-based helpers for Rails 3.2, given the
difference in how the Asset Pipeline works post-4.0.

[#298]: #298
seanpdoyle added a commit that referenced this issue Dec 2, 2015
Closes [#298]

Expose Ember's fingerprinted assets to Sprockets.

No longer support Sprockets-based helpers for Rails 3.2, given the
difference in how the Asset Pipeline works post-4.0.

[#298]: #298
seanpdoyle added a commit that referenced this issue Dec 2, 2015
Closes [#298]

Expose Ember's fingerprinted assets to Sprockets.

No longer support Sprockets-based helpers for Rails 3.2, given the
difference in how the Asset Pipeline works post-4.0.

[#298]: #298
seanpdoyle added a commit that referenced this issue Dec 2, 2015
Closes [#298]

Expose Ember's fingerprinted assets to Sprockets.

No longer support Sprockets-based helpers for Rails 3.2, given the
difference in how the Asset Pipeline works post-4.0.

[#298]: #298
seanpdoyle added a commit that referenced this issue Dec 2, 2015
Closes [#298]

Expose Ember's fingerprinted assets to Sprockets.

No longer support Sprockets-based helpers for Rails 3.2, given the
difference in how the Asset Pipeline works post-4.0.

[#298]: #298
seanpdoyle added a commit that referenced this issue Dec 2, 2015
Closes [#298]

Use Sprockets-generated manifest to detect which files are the most up
to date versions of EmberCLI-generated assets.

No longer support Sprockets-based helpers for Rails 3.2, given the
difference in how the Asset Pipeline works post-4.0.

[#298]: #298
seanpdoyle added a commit that referenced this issue Dec 3, 2015
Closes [#298]

Use Sprockets-generated manifest to detect which files are the most up
to date versions of EmberCLI-generated assets.

No longer support Sprockets-based helpers for Rails 3.2, given the
difference in how the Asset Pipeline works post-4.0.

[#298]: #298
seanpdoyle added a commit that referenced this issue Dec 3, 2015
Closes [#298]

Use Sprockets-generated manifest to detect which files are the most up
to date versions of EmberCLI-generated assets.

No longer support Sprockets-based helpers for Rails 3.2, given the
difference in how the Asset Pipeline works post-4.0.

[#298]: #298
seanpdoyle added a commit that referenced this issue Dec 3, 2015
Closes [#298]

Use Sprockets-generated manifest to detect which files are the most up
to date versions of EmberCLI-generated assets.

No longer support Sprockets-based helpers for Rails 3.2, given the
difference in how the Asset Pipeline works post-4.0.

[#298]: #298
@seanpdoyle seanpdoyle reopened this Dec 3, 2015
@seanpdoyle
Copy link
Contributor

@thejchap this has been merged into master.

Could you please try this out?

@patcoll
Copy link

patcoll commented Dec 5, 2015

FWIW @seanpdoyle master seems to have fixed my botched upgrade to 0.5.6 that had the same symptoms mentioned by @thejchap

@seanpdoyle
Copy link
Contributor

Closing based on @patcoll feedback.

This is a part of the 0.5.7 release.

@thejchap please let me know if this didn't resolve your issues.

@thejchap
Copy link
Author

thejchap commented Dec 5, 2015

@seanpdoyle thanks sounds good. It did not resolve my issues, but I haven't yet had time to investigate/look into it in more detail. If I find out what's going on I'll open a new issue or reply here. Thanks for all your hard work!

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

No branches or pull requests

5 participants