search

ember-cli / broccoli-asset-rewrite

Broccoli plugin to rewrite a source tree from an asset map.
MIT License
10 stars 53 forks source link

Add support for rewriting multiple references to the same asset #62

Closed epetrsn closed 3 years ago

epetrsn commented 6 years ago

Fixes #39 and #59.

Previously, when a matching asset path was found, the matched path was replaced globally with replaceString. When a file contains multiple references to the same (or similar) asset paths, the matching part of the path may be replaced globally multiple times. In the case where there is a prepend string and no fingerprint, the resulting path may contain duplicate segments.

This PR uses String.prototype.replace to allow for local replacements of each matched asset path. This approach removes the possibility that a path could be replaced multiple times. It also ensures that each matched path is replaced with the correct replaceString.

Additionally, new tests have been added for duplicate assets paths and escaped quotation marks (e.g., compiled Glimmer templates)

rwjblue commented 6 years ago

This seems like a nice fix (and I love seeing more test cases!), thank you for putting it together.

I'll leave this open for a short while to allow others to review, but please ping me if I don't get back to it in a couple days...

epetrsn commented 6 years ago

@rwjblue Any update on the status of this PR?

TRMW commented 6 years ago

@rwjblue (and anyone else with approve privileges) I'd love to see this get merged!

apellerano-pw commented 6 years ago

@rwjblue any word on when this can get added in?

epetrsn commented 6 years ago

@rwjblue Pinging again. Any objection to merging this?