Skip to content
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

Using template literals in assets path are not replaced with the finger printed checksum. #66

Open
sarathkumarbaskaran opened this issue May 16, 2018 · 4 comments

Comments

@sarathkumarbaskaran
Copy link

Using string literals in asset path is not replacing the finger printed value because string literal expressions are not captured in the regex.

Example -

   `${this.get('assetPath')}/images/asset.png`  // this has no effect
   this.get('assetPath') + '/images/asset.png'  // this works
@dnahodil
Copy link

I am getting this too, (I've worked around it the same way that you have).

I was wondering if it's because this library is only intended for HTML/CSS/HBS and not JS?
e.g. This comment doesn't mention JS files: https://github.com/rickharrison/broccoli-asset-rewrite/blob/8cb5062189939d03b1f238eb95e61e7dbd45e9fe/index.js#L94

@rickharrison Is this library intended to replace paths in JS files, too?

Also, is this library actively taking changes at the moment? (I've seen some other comments about trying to have maintenance moved to the Ember team). If so I might try to put up a PR for the change + tests. (Time permitting)

@rickharrison
Copy link
Collaborator

Yes, it was intended to replace most paths in js files as well.

Most of the ember team already had commit/publish access to the repos. Let me know if I need to add anyone else!

@dnahodil
Copy link

Thanks for the quick response, Rick!

Okay, thanks. Well I'll have a dig in and see what I can come up with.

Cheers,

@patocallaghan
Copy link

I got hit by this today trying to drop support for IE11 😅 I just stumbled upon this open issue after opening a similar one over at broccoli-asset-rev.

@rwjblue @dnahodil is there any chance we could get the PR in #73 merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants