roadie-rails icon indicating copy to clipboard operation
roadie-rails copied to clipboard

Fully support asset_host

Open Mange opened this issue 9 years ago • 11 comments

Here are my notes about this feature so people can contribute with ideas and feedback.

Current issues:

  1. Asset host is not applied to assets that get full URLs
  2. When asset host is set, stylesheet_link_tag becomes an absolute URL and inlining will not happen

1. Asset host is not applied

  • This is out of scope / wontfix
    • After all, why don't you just use image_tag, etc.
    • Can I find any examples in the wild where it makes sense?

2. Does not inline stylesheet_link_tags with asset_host

  • Not trivial to fix. asset_host can be String, Array or callable.
    • String is easy to fix; see if full URL starts with this string and strip it.
    • Array is also pretty easy. Do the String check for each item in the array.
    • callable is much harder. It could be doing anything, really.
      • User should provide a callable that reverses asset_host?
      • Take a guess (=~ %r{/assets/})? No. Error prone and too magic. Will break in all kinds of ways.
      • Take basename of file and look for it anyway? Will break for some people that want to be able to actually link to external stylesheets that happens to have the same name as a local asset too. ("Hey, this is why I used a full URL!")
      • Don't support this? How to communicate it? It will be seen as a "silent error" by most, and callable asset_host is a very common pattern.
        • If asset_host is set to a callable, require a reversing proc or fail with an error message.
          • But what about those who don't want it? They might just be after inline <style> elements or something. This requires them to work more.
          • This error will mostly only be detected in production. That's a very bad experience.
  • Will require changes to roadie
    • Roadie does not even ask asset providers on absolute URLs. AssetProviders need to provide a new API that can resolve absolute URLs.
      • asset_provider.find_using_url(url) if asset_provider.respond_to?(:find_using_url) perhaps? This does not require a major version bump; old implementations work like before.
  • Alternative solution (by Aidan Feldman) #30: Mutate asset_host to not do anything when encountering stylesheets.
    • This will use the normal asset host on anything that Roadie doesn't need
    • Stylesheets will still get relative URL so Roadie can pick them up.

Mange avatar Jul 08 '14 12:07 Mange

The fact that I have to set asset_host to nil is definitely annoying when you expect "image_tag", etc to work the way you expect as well :(

nateberkopec avatar Jan 14 '15 14:01 nateberkopec

If I'm not mistaken, I don't think issue 1) is fixed by using image_tag.

image_tag "[email protected]"
http:///assets/[email protected]"

nateberkopec avatar Jan 14 '15 14:01 nateberkopec

I'm expressing myself really bad up there. The point is that I don't want to build in support for asset_host-like behavior inside roadie, e.g. that the hostname could be semi-random. I'd like to keep url_options as a static hash for simplicity. If someone would want semi-random hostnames on images, they could generate the email as such. That was the point. (It's not really possible right now due to this bug, though. But when it's fixed, I won't add similar semantics.)

Is that clearer?

Mange avatar Jan 15 '15 07:01 Mange

One thing I am a bit confused by... will Roadie options for host take the place of the asset host? Or can I only serve the inlined CSS directly from my application server? I guess it is inlined, so it would just be read off of disk?

Thanks for all your hard work here, I really appreciate Roadie!

maxwell avatar Jan 21 '15 21:01 maxwell

One thing I am a bit confused by... will Roadie options for host take the place of the asset host?

Roadie's URL options is only applied to relative paths. The reason this could be useful is when you're generating emails outside of a Rails request cycle (crontab, background job, etc.) and Rails does not know which hostname the user visited and cannot generate urls with the foo_url helpers without you supplying a hostname manually.

Roadie aims to make this easier to the user. Just configure the hostname that should be used in one place, and all the URLs will be changed. Keep on using foo_path, but get a foo_url "for free".

Since "Read more" links shouldn't go to asset hosts, the configured asset host is different from the Roadie URL options. Sure, relative paths inside CSS is also treated with the Roadie URL options to save you from trouble, but if you are among the asset host users, you already have a centralized config for where assets can be served from, and the full URL can be used in the email in the first place. Roadie isn't needed in that situation, so again, it does not need to use asset host.

tl;dr: If you have an asset host, Roadie does not need it. If you don't have an asset host, Roadie does not need it. :-)

Bullet 1 of this issue ("Asset host is not used") is out of scope / wontfix. Bullet 2 ("Cannot inline with an asset host") is the actual bug, being worked on in #30. I should make this clearer.

Or can I only serve the inlined CSS directly from my application server? I guess it is inlined, so it would just be read off of disk?

Yes, Roadie will not download stylesheets from the internet. You could write your own asset provider that does that, however[1]. You could even attach some caching layer if you wanted to.

If you have the stylesheets on the application server it will inline the contents, so you don't need to serve the assets anywhere.

Thanks for all your hard work here, I really appreciate Roadie!

That warms my heart. Thank you!

[1]: The inliner will only ask the provider about stylesheets with relative paths right now (someone needs to tell me if they need this).

Mange avatar Jan 28 '15 08:01 Mange

Yes, Roadie will not download stylesheets from the internet. You could write your own asset provider that does that, however[1]. You could even attach some caching layer if you wanted to.

Version 3.1 of Roadie can do this. It's still just a RC, but roadie-rails 1.1.0.rc2 onwards have support for it as well.

1.1 of roadie-rails will have better support, utilizing Rails cache and maybe even automatically handles the asset host differently.

Mange avatar Sep 14 '15 18:09 Mange

Hi @Mange , thanks for all your hard work!

Could you provide an update on the asset_host issue?

Cheers!

jufemaiz avatar Jun 22 '16 23:06 jufemaiz

I've recently started working on a Rails 3 app with an asset_host, so I might be able to get some insight on this now.

I still feel the best way out of this is to manually build the URL for your stylesheet in the email, or to add external providers that rewrites your CDN resources to look like local ones. It's a bit more configuration than getting it out of the box, but then again, this is a very tricky problem that only applies to people working with non-default configs so some configuration might be necessary.

I'm still extremely open to good suggestions on how to handle this automatically.

Den tor 23 juni 2016 01:15Joel Courtney [email protected] skrev:

Hi @Mange https://github.com/Mange , thanks for all your hard work!

Could you provide an update on the asset_host issue?

Cheers!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Mange/roadie-rails/issues/11#issuecomment-227905307, or mute the thread https://github.com/notifications/unsubscribe/AAAGP6a2IHqGFzdsLndHiBQgc2jR515wks5qOcIIgaJpZM4CLIsL .

Mange avatar Jun 29 '16 16:06 Mange

Hi @Mange, thanks for the feedback! Good to know what the opinion is of the author :)

I've managed to get it working for the time being using a workaround posted at https://github.com/Mange/roadie-rails/issues/10#issuecomment-143759231

I think that you're almost there. I disagree that hand crafting is a good thing – but I'm also understanding that as the author you have valid reasons for your opinion, particularly with the underlying Roadie Gem being a non-rails implementation.

Let me do some more thinking and document some more use cases where I believe there is a need (predominantly around image versus stylesheet collision issues).

Cheers for all your hard work – it's very much appreciated!

jufemaiz avatar Jun 29 '16 22:06 jufemaiz

Thank you for your kind words. I look forward to hearing about your findings. ☺️

Den tor 30 juni 2016 00:13Joel Courtney [email protected] skrev:

Hi @Mange https://github.com/Mange, thanks for the feedback! Good to know what the opinion is of the author :)

I've managed to get it working for the time being using a workaround posted at #10 (comment) https://github.com/Mange/roadie-rails/issues/10#issuecomment-143759231

I think that you're almost there. I disagree that hand crafting is a good thing – but I'm also understanding that as the author you have valid reasons for your opinion, particularly with the underlying Roadie Gem being a non-rails implementation.

Let me do some more thinking and document some more use cases where I believe there is a need (predominantly around image versus stylesheet collision issues).

Cheers for all your hard work – it's very much appreciated!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Mange/roadie-rails/issues/11#issuecomment-229505277, or mute the thread https://github.com/notifications/unsubscribe/AAAGP9OVeXJApVAACkdHrnAbamW0GjQbks5qQu4HgaJpZM4CLIsL .

Mange avatar Jun 30 '16 15:06 Mange

When asset_host is used for images, the stylesheet URL can still be generated without the host like this:

<%# Ensure the stylesheet is referenced without a host so that the local
    version is read by Roadie even if asset_host is set. %>
<link rel="stylesheet" href="<%= stylesheet_path('email', host: '') %>" />

This seems like a better workaround than setting asset_host to nil as the readme suggests.

glebm avatar Mar 26 '17 17:03 glebm