patrickhulce / fontmin-webpack

Minifies icon fonts to just the used glyphs.
MIT License
137 stars 19 forks source link

Included icons does not affect font file hash #8

Open scriby opened 6 years ago

scriby commented 6 years ago

It appears that the hash of the font file is taken prior to fontmin running, so it only depends on the original source file. This means that the font file hash won't change if the app starts using more icons from the source font file.

If the project relies on the hash for caching / updating purposes this will create bugs, as the production app can reference icons which aren't in the version of the font file the user has cached.

On my project we ended up switching to a fontmin loader instead to work around the issue.

patrickhulce commented 6 years ago

Thanks for the report @scriby could you provide a bit more detail? By hash are you referring to the hash present in the font filename (and the request is for different computed glyph patterns to result in unique filenames)?

onpubcom commented 2 years ago

I'm running in to this same issue.

Because I'm using the default Font Awesome Sass mixins via their node module, there's no easy/practical way to append ?v= (i.e. cache busting query param) to the font file names like you have used in your test fixtures in this repo.

Ideally the hash of the minified output webfont files changes based on the contents of the file as opposed to the contents of the source files.

This way anytime our list of in-use icons changes, then the font file names change as a result, which gives us cache-busting logic here essentially for free.

Either this or perhaps consider providing a new config param that allows us to specify which webpack substitution macro (https://webpack.js.org/configuration/output/#template-strings) like [contenthash], for example, we can use as an alternate way to compute the name of the output file, or something along these lines.

Awesome plugin btw! Thanks so much for putting this together.

patrickhulce commented 2 years ago

consider providing a new config param that allows us to specify which webpack substitution macro (https://webpack.js.org/configuration/output/#template-strings) like [contenthash], for example, we can use as an alternate way to compute the name of the output file, or something along these lines.

Great idea for a workaround! A bit tight on bandwidth myself, but if anyone wants to put up a PR + test for this, that'd be awesome :)

onpubcom commented 2 years ago

@patrickhulce , I might try to contribute something here if I can decipher what's going on in the plugin code. Interestingly enough, when I use this plugin with webpack 5, then it works just fine in conjunction with css-loader but when I also try to add the file-loader in to the mix then I'm better able to control the output file names using that loader's options, but my compiled CSS still points to the original hashed output file names in my url() references (which were re-written by css-loader) and not to the new file names I specified via my file-loader options. Is there any way you're aware of to workaround this issue? I guess what I'm asking is how do you get css-loader and file-loader to play nicely together, because when I combine them it doesn't seem to work as expected.

patrickhulce commented 2 years ago

Honestly @onpubcom I've been out of webpack for a bit while using other bundlers, so I'm a tad rusty, but is mini-css-extract-plugin not the path you're supposed to use along with css-loader anymore?

That's the path we have tests for

https://github.com/patrickhulce/fontmin-webpack/blob/4c72d4264f825f019290bdd07b668355466a0428/test/fixtures/webpack.extract-text.config.js#L8-L20

I thought file-loader is what you used when you didn't need any sort of syntax handling.

onpubcom commented 2 years ago

Hi @patrickhulce , I am using MiniCssExtractPlugin but that only filters output of the .css files themselves and not any files that were re-written by css-loader as far as I know. I'll do a bit of debugging of this plugin sometime soon and perhaps submit a patch to help with some of the issues I'm seeing.

onpubcom commented 2 years ago

@patrickhulce We submitted a PR for you to review on a possible solution for the issues discussed above:

Let us know if you have any feedback, thanks!