Merge e047196 into 59d8649

Munter · Jun 7, 2020 · e996721 · e996721
2 parents 59d8649 + e047196
commit e996721
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 7 deletions.
diff --git a/lib/subsetFonts.js b/lib/subsetFonts.js
@@ -1150,22 +1150,26 @@ These glyphs are used on your site, but they don't exist in the font you applied
             const fontRelations = cssAsset.outgoingRelations.filter(
               (relation) => relation.node === rule
             );
-            const urlStrings = value
-              .split(/,\s*/)
-              .filter((entry) => entry.startsWith('url('));
+            const urlStrings = value.split(/,\s*/).filter(
+              (entry) => entry.startsWith('url(') && entry.includes('/subfont/') // Avoid preloading unused variants
+            );
             const urlValues = urlStrings.map((urlString, idx) =>
               urlString.replace(
                 fontRelations[idx].href,
                 '" + "/__subfont__".toString("url") + "'
               )
             );
-            url = `"${urlValues.join(', ')}"`;
+            if (urlValues.length > 0) {
+              url = `"${urlValues.join(', ')}"`;
+            }
           }
         });
 
-        fontFaceContructorCalls.push(
-          `new FontFace("${name}", ${url}, ${JSON.stringify(props)}).load();`
-        );
+        if (url) {
+          fontFaceContructorCalls.push(
+            `new FontFace("${name}", ${url}, ${JSON.stringify(props)}).load();`
+          );
+        }
       });
 
       const jsPreloadRelation = htmlAsset.addRelation(

diff --git a/test/subsetFonts.js b/test/subsetFonts.js
@@ -3563,6 +3563,39 @@ describe('subsetFonts', function () {
           url: `${assetGraph.root}IBMPlexSans-Italic.woff`,
         });
       });
+
+      it('should not preload the unused variants', async function () {
+        const assetGraph = new AssetGraph({
+          root: pathModule.resolve(
+            __dirname,
+            '../testdata/subsetFonts/unused-variant-preload/'
+          ),
+        });
+        const [htmlAsset] = await assetGraph.loadAssets('index.html');
+        await assetGraph.populate({
+          followRelations: {
+            crossorigin: false,
+          },
+        });
+        await subsetFonts(assetGraph, {
+          inlineFonts: false,
+        });
+        const [preloadPolyfill] = assetGraph.findAssets({
+          type: 'JavaScript',
+          text: { $regex: /new FontFace/ },
+        });
+        expect(preloadPolyfill.text, 'to contain', ".woff2'")
+          .and('to contain', ".woff'")
+          .and('not to contain', ".ttf'")
+          .and('not to contain', 'fonts.gstatic.com');
+        const preloadLinks = assetGraph.findRelations({
+          from: htmlAsset,
+          type: 'HtmlPreloadLink',
+        });
+        expect(preloadLinks, 'to satisfy', [
+          { href: /^\/subfont\/Noto_Serif-400-[a-f0-9]{10}\.woff2$/ },
+        ]);
+      });
     });
 
     it('should return a fontInfo object with defaulted/normalized props', async function () {

diff --git a/testdata/subsetFonts/unused-variant-preload/index.html b/testdata/subsetFonts/unused-variant-preload/index.html
@@ -0,0 +1,9 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <link rel="stylesheet" type="text/css" href="https://fonts.googleapis.com/css?family=Noto+Serif:400,700,400i|Open+Sans:700,400">
+  </head>
+  <body>
+    <div style="font-family: Noto Serif">Hello</div>
+  </body>
+</html>