ember-cli / broccoli-asset-rev

Broccoli plugin to add fingerprint checksums and CDN URLs to your assets
MIT License
87 stars 84 forks source link

Erroneously fingerprinting file references #53

Closed paulelliott closed 9 years ago

paulelliott commented 9 years ago

We discovered an issue where strings matching app.js within js content would be fingerprinted, even though they weren't referring to the actual app.js generated by ember cli. Case in point:

Our code makes a request to look for a public file on GitHub. In one of our js files is a string containing a URL like this:

https://api.github.com/repos/{{repo stuff etc}}/contents/app.json?ref=master

After the broccoli build the string is transformed into this:

https://api.github.com/repos/{{repo stuff etc}}/contents/app-{{big-ol-fingerprint}}.json?ref=master

It appears as though it is finding it because it matches app.js, even though that is a subset of the actual string.

rickharrison commented 9 years ago

This is a pretty difficult problem to solve due to the nature of the regex used to fingerprint and replace all the asset paths.

Could you try using the ignore option? Are there other paths in that file that need to be re-written? You can see the ignore option here: https://github.com/rickharrison/broccoli-asset-rewrite

paulelliott commented 9 years ago

We tried using the ignore option, but that tells it not to fingerprint app.js at all. We do want it fingerprinted, we just don't want it to alter URLs of other files. I realize this is an extremely difficult thing to address as there is no reliable terminator for asset paths.

rickharrison commented 9 years ago

Are you sure you used ignore and not exclude ?

paulelliott commented 9 years ago

You are correct. We used exclude when we tried it earlier. From the docs (and code) in ember-cli, it looks like ignore isn't passed down through ember-cli's fingerprint options. Is there another way we can set it?

rickharrison commented 9 years ago

Can you try setting ignore and see if it works? I haven't worked with ember-cli in quite some time so I'm not that familiar with the current state of their build process.

Dhaulagiri commented 9 years ago

ignore didn't work because it is not passed through by this library to broccoli-asset-rewrite. I'm not sure that it would have helped us if it were passed through, though. In the end we renamed our app.js build generated by ember-cli to something else so that broccoli-asset-rev wouldn't try to fingerprint instances of app.js in our code.

rickharrison commented 9 years ago

Thanks for that catch. I will try and get an update out to fix that issue soon.

rickharrison commented 9 years ago

This is fixed in 2.0.6. Please let me know if everything works for you.