broccoli-asset-rev

broccoli-asset-rev copied to clipboard

Hi. I'm adding a loading image, to index.html, in an Ember.JS project. The problem is that, after fingerprinting this:

        <img
             src="images/myimage.png"
             srcset="images/[email protected] 2x, images/myimage.png 1x"
             sizes="200px"
             alt="Loading" />

        <img
             src="//mycdn.cloudfront.net/images/myimage-515d3d97735aae0a950e74c8ad32ddbf.png"
             srcset="//mycdn.cloudfront.net/images/[email protected]"
             sizes="200px"
             alt="Loading" />

The problem is that, it removes the 1x and 2x directives, and this makes the desktop browsers load the 2x image by default. I don't really know why, but I guess is something to do with srcset compatibility. Any way we can preserve both the 1x and 2x directives?

Thanks for reporting this. Just to be upfront, this might be a difficult problem to solve. I don't really have an ETA at this time of when I will be able to tackle this.

I'm hitting this too. Is there any solution you've in mind? The current approach of one-regex-to-rule-them-all doesn't seem like it can last much longer.

Out of curiosity, why does the regex explicitly try to match on string delimiters (e.g. " or ')? Matching on the bare filename would solve this problem, although I have to assume that breaks everything. :stuck_out_tongue_closed_eyes:

Maybe it would be best to switch to an Esprima-based parser for JS files and only pattern match inside string literals?

I'm not really sure what the best strategy to solve this use case is. It matches one of the delimiters so that it knows where to stick the prepend in (i.e. cloudfront domain).

I'm all ears on other solutions, but implementing an esprima-based parser is probably over my head.

Same problem here. srcset it's not handled properly.

Using ember-cli:

// No fingerprint

var el6 = dom.createElement("img");
dom.setAttribute(el6,"src","/images/product/[email protected]");
dom.setAttribute(el6,"srcset","/images/product/calender_framed-be441c32691eb35d8a0e0542462b81d1.jpg 1x, /images/product/[email protected] 2x");
dom.setAttribute(el6,"class","wow fadeIn img-responsive");

// Fingerprint
// fingerprint: {
//   prepend: '//s3-eu-west-1.amazonaws.com/smhw-frontend-prod/';
// },
var el6 = dom.createElement("img");
dom.setAttribute(el6,"src","//s3-eu-west-1.amazonaws.com/smhw-frontend-prod/images/product/[email protected]");
dom.setAttribute(el6,"srcset","//s3-eu-west-1.amazonaws.com/smhw-frontend-prod/images/product/calender_framed-be441c32691eb35d8a0e0542462b81d1.jpg");
dom.setAttribute(el6,"class","wow fadeIn img-responsive");

Yea the only thing I'm concerned with is breaking existing functionality. A lot of the PRs thus far have tended to break behavior to fix edge cases.

I myself conceded defeat a couple times. The regexp approach is unbeliable hard to modify without breaking some edge case. I tried for a while to have different parsers for different files (hbs, js...) but that is not a simple task to do.

I cannot reproduce this issue (here is my try https://serabe.github.io/check-asset-rev/ ). Can anyone send me one?

Thanks!

Ok, the problem was with the prepend option. rickharrison/broccoli-asset-rewrite#61 fixes it.