Add cache buster config option #42
Conversation
|
Looks good! |
|
Any chance of this being merged? |
|
@backflip Please contact me, I want to help with reviewing the PR's if you want to |
|
Hey @jefsnare, sounds awesome! I have added you as a collaborator. |
|
@backflip Cool, only write access should be nice, I can't merge or add changes. Can you provide this permission? |
|
Not sure, but you'll probably have to accept the invite first. I might be able to fine-tune permissions afterwards:
|
|
@backflip Done, missed that email for some reason. Thank you! |
|
I can add you as an npm owner, too, if you provide me with your username. This would allow you to publish releases. |
|
@backflip my username is jefsnare on NPM |
| url('<%= fontPath %><%= fontName %>.ttf') format('truetype'), | ||
| url('<%= fontPath %><%= fontName %>.svg#<%= fontName %>') format('svg'); | ||
| src: url('<%= fontPath %><%= fontName %>.eot<%= cacheBusterQueryString %>'); | ||
| src: url('<%= fontPath %><%= fontName %>.eot?<%= cacheBuster %>#iefix') format('eot'), |
There was a problem hiding this comment.
Why are you using assigned var cacheBuster instead of also using cacheBusterQueryString ?
There was a problem hiding this comment.
This is because for EOT format, the IE fix needs to always have the ? of the querystring whether or not a cache buster is used. When it is not, for other formats the ? is ommitted but not for EOT.
This is the only difference between the two vaiables cacheBusterQueryString and cacheBuster: the former include the ?, the latter doesn't. I used to variables for this because having ifs in the template made the whole thing less readable (too much clutter).
(Sorry for the late response!)
This allows users to give a version tag or timestamp that is automatically included in the font files URLs as a query string parameter. This is useful to bust HTTP caches when deploying new versions of icon fonts.
|
I just rebased this PR on master, and added a fixup to add documentation (and simplify code a little bit). Left the fixup so that you can see the changes but it should be squashed before merge. However tests don't pass anymore on my computer; don't know why. The test this PR adds is still OK but the ones that was there before are not OK anymore for some reason. It seems to be a difference in generated font vs expected. I'm guessing a dependency changed the way it generates font files a bit. Are you able to reproduce this issue? Test output: |
|
@mrtbld, I have updated the test files in master, I guess the woff packages were updated upstream. |
This allows users to give a version tag or timestamp that is automatically included in the font files URLs as a query string parameter. This is useful to bust HTTP caches when deploying new versions of icon fonts.
This is different approach than @lzl124631x's P/R #39. It doesn't force any tagging scheme (timestamp, version number, whatever) since it is provided by the user. It works out of the box: no need to use a custom CSS template, just set
cacheBusterto something (of course you can use it in custom templates too).The default behavior is the same as before.
Review appreciated. :-)