Double prepend when using customHash: null

[typeoneerror]

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

[rickharrison]

Can you please contribute a failing test case to illustrate your issue? Our existing test cases with prepend are not failing.

[2468ben]

@rickharrison I ran into this same case from ember-cli-deploy. This following config is what does it for me, specifically customHash: null (with that line, prepending runs twice, without it only once):

appConfig.fingerprint = {
  enabled: true,
  prepend: '//localhost:4200',
  customHash: null,
  replaceExtensions: ['html']
};

[typeoneerror]

@2468ben confirmed customHash: null in my development setup as well. likely culprit?

[typeoneerror]

@rickharrison I checked out a fresh clone and ran npm test and the tests are already failing in this exact location:

1) skips fingerprint and prepends when set and customHash is null

-  <script type="text/javascript" src="https://foobar.cloudfront.net/https://foobar.cloudfront.net/https://foobar.cloudfront.net/https://foobar.cloudfront.net/assets/application.js"></script>
-  <script type="text/javascript" src=https://foobar.cloudfront.net/https://foobar.cloudfront.net/https://foobar.cloudfront.net/https://foobar.cloudfront.net/assets/application.js></script>

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.

[rickharrison]

I reverted the changes to asset-rewrite in 1.0.11. This now fixes it.

[typeoneerror]

@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.

[rickharrison]

Sounds good. A big help would be a contributing test case that demonstrates the exact problem you had. Especially in the asset-rewrite repo

[typeoneerror]

@rickharrison the failing test I referenced above is the exact case. a prepended url with customHash set to null.

[rickharrison]

Right, but the test suite was passing in broccoli-asset-rewrite so a failing case there would be very helpful.

[2468ben]

it's because of AssetRewrite.prototype.processString: https://github.com/rickharrison/broccoli-asset-rewrite/blob/master/index.js#L148 and https://github.com/rickharrison/broccoli-asset-rewrite/blob/master/index.js#L161

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

if (this.prepend && this.prepend !== '' && replaceString.indexOf(this.prepend) !== 0) {

then prepending only happens once (and the changes in #26 work too).

[2468ben]

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.