magnars / optimus

A Ring middleware for frontend performance optimization.
364 stars 23 forks source link

Paths to assets used in Sass are not rewritten correctly #51

Open ianjenkinsii opened 8 years ago

ianjenkinsii commented 8 years ago

Assets like images that are used in the sass files such as background-image: url('../img/this-asset.png');do not load properly when a contextual path is in use. For example if my site renders two versions of its content at http://my-example-site.com and at http://my-example-site.com/preview/ the former would get the assets correctly but the site with the preview will not. A 404 is generated when attempting to retrieve this-asset.png In the compiled CSS the path is rewritten to /img/this-asset-png as in background-image: url('/img/this-asset.png'); For the non-contextual instance this OK but for the /preview site it is not

magnars commented 8 years ago

So the preview site serves its assets under /preview too? Adding a :base-url to the assets should fix that.

ianjenkinsii commented 8 years ago
(defn get-assets []
  (concat
    (asset/load-assets "public"
      [#"/img/.*\.(png|gif)"])
.............

Sorry, but could I bother you for an example. Say with the code snipped above, how does one introduce a :base-url? I could not find an example of its use.

magnars commented 8 years ago

Search for :base-url in the readme. There's an example of adding a custom middleware. Let me know if that needs any clarifications. ons. 6. apr. 2016 kl. 15.54 skrev Ian Jenks notifications@github.com:

(defn get-assets [](concat %28asset/load-assets "public" [#"/img/.*.%28png|gif)"]) .............

Sorry, but could I bother you for an example. Say with the code snipped above, how does one introduce a :base-url? I could not find an example of its use.

— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/magnars/optimus/issues/51#issuecomment-206382788

ianjenkinsii commented 8 years ago

Thanks for the pointer. However, even with :base-url the problem persists. The resource path is rewritten so that it points to http://my-example-site.com/img/this-asset.png instead of at http://my-example-site.com/preview/img/this-asset.png. I think I must be misunderstanding the application of the the :base-url

magnars commented 8 years ago

Can you paste in how you use it? ons. 6. apr. 2016 kl. 18.48 skrev Ian Jenks notifications@github.com:

Thanks for the pointer. However, even with :base-url the problem persists. The resource path is rewritten so that it points to http://my-example-site.com/img/this-asset.png instead of at http://my-example-site.com/preview/img/this-asset.png. I think I must be misunderstanding the application of the the :base-url

— You are receiving this because you commented.

Reply to this email directly or view it on GitHub https://github.com/magnars/optimus/issues/51#issuecomment-206460428

ianjenkinsii commented 8 years ago
(defn add-context-url-to-assets [assets]
  (map #(assoc % :base-url "http://my-example-site.com/preview/") assets))

(defn my-dev-optimize
[assets options]
-> assets
     (optimizations/none options) 
      (add-context-url-to-assets))
magnars commented 8 years ago

Looks right to me. I'll check what base-url is doing. ons. 6. apr. 2016 kl. 22.54 skrev Ian Jenks notifications@github.com:

(defn add-context-url-to-assets [assets](map #%28assoc % :base-url) assets))

(defn my-dev-optimize [assets options] -> assets (optimizations/none options) (add-context-url-to-assets))

— You are receiving this because you commented.

Reply to this email directly or view it on GitHub https://github.com/magnars/optimus/issues/51#issuecomment-206563004

magnars commented 8 years ago

Yep, looks like a bug. I'll have a fix tomorrow, I hope. ons. 6. apr. 2016 kl. 23.26 skrev Magnar Sveen magnars@gmail.com:

Looks right to me. I'll check what base-url is doing. ons. 6. apr. 2016 kl. 22.54 skrev Ian Jenks notifications@github.com:

(defn add-context-url-to-assets [assets](map #%28assoc % :base-url) assets))

(defn my-dev-optimize [assets options] -> assets (optimizations/none options) (add-context-url-to-assets))

— You are receiving this because you commented.

Reply to this email directly or view it on GitHub https://github.com/magnars/optimus/issues/51#issuecomment-206563004

magnars commented 8 years ago

After looking into this issue, I think the proper fix is to have a middleware that rewrites the CSS-urls to relative paths. Just forcing the base-url into the css-files would be quite bulky, and possibly wrong (for instance, it would rely on the base-url middleware to be placed just so-and-so in the stack).

I don't have the time to do that just now. Would you be interested in creating such a middleware? It would look at the path of the css-asset, and rewrite all referenced paths inside it relative to itself.