clubstudio / craft-asset-rev

A Craft CMS plugin to help with cache busting
MIT License
109 stars 7 forks source link

Support for Laravel Mix manifest #25

Closed benface closed 4 years ago

benface commented 4 years ago

It seems Laravel Mix's mix-manifest.json paths have a leading slash, so we need to include it in the value passed to rev() for it to be picked up, e.g.:

<link rel="stylesheet" type="text/css" href="{{ rev('/css/app.css') }}">

But then the URL output in the HTML has a double slash (because the base URL has a trailing slash):

<link rel="stylesheet" type="text/css" href="http://local.craft-project.com//css/app.css?id=ae58b3e665d52686119e">

Would it make sense to search the manifest for the path with a leading slash if the regular path isn't found, and if it finds that, automatically remove the slash from the resulting path? So we could do:

<link rel="stylesheet" type="text/css" href="{{ rev('css/app.css') }}">

and it would find this entry in mix-manifest.json:

"/css/app.css": "/css/app.css?id=ae58b3e665d52686119e"

and the output would be:

<link rel="stylesheet" type="text/css" href="http://local.craft-project.com/css/app.css?id=ae58b3e665d52686119e">

Maybe that's too opinionated, but otherwise I'm not sure how to deal with Laravel Mix. Thank you!

scottwakefield commented 4 years ago

Hi @benface,

I use the plugin exclusively with Laravel Mix and usually just leave the trailing slash off the URL prefix. Is that possible with your setup? How/where are you defining the URL?

Either way, I'm sure there's something we can do to help protect against the double slashes within the plugin codebase.

benface commented 4 years ago

@scottwakefield I'm not sure I understand. Are you saying that you don't have trailing slashes in your mix-manifest.json? Or that you are able to omit trailing slashes in rev(), e.g. {{ rev('css/app.css') }} and it works? The latter doesn't work for me, it just returns css/app.css.

scottwakefield commented 4 years ago

@benface, I do have leading slashes in my mix-manifest.json file but I don't have a trailing slash on my assetPrefixUrl.

What is your assetUrlPrefix set to? http://local.craft-project.com/?

benface commented 4 years ago

Thank you, yes I meant leading, not trailing, sorry. My assetUrlPrefix is set to {baseUrl} which resolves to http://local.craft-project.com/, with the trailing slash, yes. But the issue is that it doesn't pick up the asset from the manifest because rev() expects /css/app.css, not css/app.css, no?

scottwakefield commented 4 years ago

Ah, is this a Craft 2 project? It may be that the older version of the plugin is handling the links a little differently.

However, if you can trim the trailing slash from your {baseUrl} then it shouldn't matter that what the string you're passing into rev() starts with a / as the plugin should just concatenate the two values. Although, I can understand that might have unintended consequences if you're already referencing {baseUrl} elsewhere on the site.

With that in mind, I will take a look at making the plugin handle this scenario better asap :+1:

benface commented 4 years ago

Ah so sorry, I forgot to mention it, but yes, it is a Craft 2 project. Although I have been using the plugin on Craft 3 projects as well, and always found it a bit awkward to have to include the leading slash in rev(), since Craft's own url() and siteUrl() functions don't need it (in fact, if you include it in url(), it returns a root-relative URL instead of an absolute one). Although it wasn't really an issue for those projects because it seems that Craft 3 removes any trailing slash in aliases: https://craftcms.stackexchange.com/questions/29414/baseurl-and-trailing-slash-craft-3 (or is it Yii 2 doing that?)

Although, I can understand that might have unintended consequences if you're already referencing {baseUrl} elsewhere on the site.

Yeah, it is used in a few places unfortunately (including in the database).

Thank you for your help :)

benface commented 4 years ago

I'll close this issue as Laravel Mix manifests are clearly supported. Thanks again for your help!