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

Adding an option to append a content hash to the end of font files for cache-busting on glyph changes #51

Merged
merged 10 commits into from
Nov 23, 2021

Conversation

john-positano
Copy link
Contributor

No description provided.

Positano and others added 7 commits November 4, 2021 15:01
and defeats the purpose of caching. This time, a simple MD5 hash is made
in order to change the file suffix only when the contents have changed
…ngth > file.minified.length) {` block so we only execute this new logic now if there's a minified file that needs to be output.
Copy link
Owner

@patrickhulce patrickhulce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

impl looks great! could we get a quick test that ensures the filename is updated in the CSS? :) should be able to reuse the same existing webpack configs for the most part, just utilize appendHash option

@john-positano
Copy link
Contributor Author

@patrickhulce Did you mean "check that on my end" or did you want a test module written to go along with these changes?

@patrickhulce
Copy link
Owner

a test module written to go along with these changes

This one :)

@john-positano
Copy link
Contributor Author

@patrickhulce I wrote a test to make sure that the hashes appended to the font filenames are referenced in the final assets, that's pushed

@patrickhulce
Copy link
Owner

thanks very much @john-positano!

a quick patch to fix the lint failures and we're on our way 🎉

git am <<-'EOF'
From cc7ad0fb5e48aa151821d95d9b4797a47b6ad3c2 Mon Sep 17 00:00:00 2001
From: Patrick Hulce <patrick.hulce@compass.com>
Date: Tue, 16 Nov 2021 22:40:14 -0600
Subject: [PATCH] lint fixes

---
 lib/index.js       | 18 +++++++++---------
 test/index.test.js |  6 +++---
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/lib/index.js b/lib/index.js
index 283f8e0..c3703e2 100644
--- a/lib/index.js
+++ b/lib/index.js
@@ -1,7 +1,7 @@
 const fs = require('fs')
 const path = require('path')
-const log = require('debug')('fontmin-webpack')
 const crypto = require('crypto')
+const log = require('debug')('fontmin-webpack')
 
 const _ = require('lodash')
 const ttf2woff2 = require('ttf2woff2')
@@ -28,7 +28,7 @@ class FontminPlugin {
         autodetect: true,
         allowedFilesRegex: null,
         skippedFilesRegex: null,
-        appendHash: false
+        appendHash: false,
       },
       options,
     )
@@ -230,25 +230,25 @@ class FontminPlugin {
               } else {
                 compilation.assets[file.asset] = new RawSource(file.minified)
               }
-            }          
+            }
           })
         })
     }, Promise.resolve())
   }
 
   appendMinifiedFileHash(file) {
-    const fileHash = crypto.createHash( 'md5' ).update( file.minified ).digest( 'hex' )
+    const fileHash = crypto.createHash('md5').update(file.minified).digest('hex')
     return file.asset.split('.').join(`-${fileHash}.`)
   }
 
   hashifyFontReferences(oldAssetName, newAssetName, assets) {
     Object.keys(assets).forEach(
-      ( asset, assetIndex ) => {
-        let oldAssetNameRegex = new RegExp( oldAssetName.replace( '.', '\\.' ), 'g' )
-        let assetSource = assets[ asset ].source().toString()
+      asset => {
+        const oldAssetNameRegex = new RegExp(oldAssetName.replace('.', '\\.'), 'g')
+        const assetSource = assets[asset].source().toString()
 
-        if ( assetSource.match( oldAssetNameRegex ) ) {
-          assets[ asset ] = new RawSource( assetSource.replace( oldAssetNameRegex, newAssetName ) )
+        if (assetSource.match(oldAssetNameRegex)) {
+          assets[asset] = new RawSource(assetSource.replace(oldAssetNameRegex, newAssetName))
         }
       }
     )
diff --git a/test/index.test.js b/test/index.test.js
index c74a251..90de651 100644
--- a/test/index.test.js
+++ b/test/index.test.js
@@ -249,9 +249,9 @@ describe('FontminPlugin', () => {
     })
 
     it('should append the hash to the ends of all refrences in all assets', () => {
-      let out = fs.readFileSync(DIST_FOLDER + '/out.js').toString()
-      fontStats.forEach( file => expect( out.match( file.filename ) ).to.be.ok )
-      fontStats.forEach( file => expect( out.match( file.filename.replace( /\-([a-z]|[0-9])+\./, '.') ) ).to.not.be.ok )
+      const out = fs.readFileSync(DIST_FOLDER + '/out.js').toString()
+      fontStats.forEach(file => expect(out.match(file.filename)).to.be.ok)
+      fontStats.forEach(file => expect(out.match(file.filename.replace(/-([a-z0-9]+)\./, '.'))).to.not.be.ok)
     })
   })
 })
-- 
2.33.1


EOF

@john-positano
Copy link
Contributor Author

@patrickhulce I weeded out lint errors. CircleCI can be reran.

@patrickhulce patrickhulce merged commit fcfcad4 into patrickhulce:master Nov 23, 2021
@patrickhulce
Copy link
Owner

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

Successfully merging this pull request may close these issues.

3 participants