-
-
Notifications
You must be signed in to change notification settings - Fork 84
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
Double prepend when using customHash: null #87
Comments
Can you please contribute a failing test case to illustrate your issue? Our existing test cases with prepend are not failing. |
@rickharrison I ran into this same case from ember-cli-deploy.
|
@2468ben confirmed |
@rickharrison I checked out a fresh clone and ran
Working on it now. Side note: feels like this should be caught by Travis CI? I see a file in the repo but no "failing" tag. Not sure how travis works entirely. |
I reverted the changes to asset-rewrite in 1.0.11. This now fixes it. |
@rickharrison cool, will let you know if it fixes it for us (a few different parties tracking it in the ember-cli-deploy slack channel). one thing you might consider is making sure that the test fixtures have multiple assets in them. For example, the referenced commit only replaces a single asset in each file. |
Sounds good. A big help would be a contributing test case that demonstrates the exact problem you had. Especially in the asset-rewrite repo |
@rickharrison the failing test I referenced above is the exact case. a prepended url with customHash set to null. |
Right, but the test suite was passing in broccoli-asset-rewrite so a failing case there would be very helpful. |
it's because of AssetRewrite.prototype.processString: AssetRewrite.prototype.rewriteAssetPath gets called twice, but the prepend logic doesn't check if the prepend has already been added. The changes in #26 / 1.0.10 didn't account for the second run. Changing https://github.com/rickharrison/broccoli-asset-rewrite/blob/master/index.js#L119 to
then prepending only happens once (and the changes in #26 work too). |
Sorry I only had time right now to write where the issue is, I'm too busy for a while to build up tests for this. |
Not sure if this is an issue with asset-rev or asset-rewrite, but posting here as well.
We're using a Rails-backend index loaded from redis as shown here in the ember-cli-deploy lightning method. https://github.com/ghedamat/ember-deploy-demo/blob/master/edd-cli/ember-cli-build.js#L15
Since upgrading to latest, all the asset urls are double prepended, we're ending up with
GET http://localhost:4200/http://localhost:4200/assets/vendor.css
The text was updated successfully, but these errors were encountered: