Remove support for inlineFonts

Just keep the subsets inline when only one format is to be supported. #10 (comment)
Munter · Aug 8, 2020 · bd85efb · bd85efb
1 parent 90f109c
commit bd85efb
Show file tree

Hide file tree

Showing 7 changed files with 92 additions and 163 deletions.
diff --git a/README.md b/README.md
@@ -85,8 +85,6 @@ Options:
                                      JavaScript enabled                 [boolean] [default: false]
   --in-place, -i                     Modify HTML-files in-place. Only use on build artifacts
                                                                         [boolean] [default: false]
-  --inline-fonts                     Inline fonts as data-URIs inside the @font-face declaration
-                                                                        [boolean] [default: false]
   --inline-css                       Inline CSS that declares the @font-face for the subset fonts
                                                                         [boolean] [default: false]
   --font-display                     Injects a font-display value into the @font-face CSS.

diff --git a/lib/parseCommandLineOptions.js b/lib/parseCommandLineOptions.js
@@ -56,11 +56,6 @@ module.exports = function parseCommandLineOptions(argv) {
       type: 'boolean',
       default: false,
     })
-    .options('inline-fonts', {
-      describe: 'Inline fonts as data-URIs inside the @font-face declaration',
-      type: 'boolean',
-      default: false,
-    })
     .options('inline-css', {
       describe: 'Inline CSS that declares the @font-face for the subset fonts',
       type: 'boolean',
@@ -103,13 +98,16 @@ module.exports = function parseCommandLineOptions(argv) {
       type: 'boolean',
       default: false,
     })
-    .check(({ harfbuzz, subsetPerPage }) => {
+    .check(({ harfbuzz, subsetPerPage, inlineFonts }) => {
       // Fail instead of silently ignoring legacy switches.
       // This prevents --subset-per-page etc. from implicitly being interpreted as 'string',
       // which means that it would consume a following non-option cli param
       if (subsetPerPage !== undefined) {
         return '--[no-]subset-per-page is no longer supported as of subfont 6.0.0';
       }
+      if (inlineFonts !== undefined) {
+        return '--[no-]inline-fonts is no longer supported as of subfont 6.0.0';
+      }
       if (harfbuzz && parseInt(process.versions.node) < 10) {
         return 'The --harfbuzz option requires node.js 10 or above';
       } else {

diff --git a/lib/subfont.js b/lib/subfont.js
@@ -14,7 +14,6 @@ module.exports = async function subfont(
     debug = false,
     dryRun = false,
     silent = false,
-    inlineFonts = false,
     inlineCss = false,
     fontDisplay = 'swap',
     formats,
@@ -215,7 +214,6 @@ module.exports = async function subfont(
   }
 
   const { fontInfo } = await subsetFonts(assetGraph, {
-    inlineFonts,
     inlineCss,
     fontDisplay,
     formats,

diff --git a/lib/subsetFonts.js b/lib/subsetFonts.js
@@ -649,7 +649,6 @@ async function subsetFonts(
     subsetPath = 'subfont/',
     jsPreload = true,
     omitFallbacks = false,
-    inlineFonts,
     inlineCss,
     fontDisplay,
     onlyInfo,
@@ -1127,7 +1126,9 @@ These glyphs are used on your site, but they don't exist in the font you applied
 
     subsetFontsToBeMinified.add(cssAsset);
 
-    if (!inlineFonts) {
+    // Keep the subsets inline unless we've injected more than one format:
+    const keepFontsInline = formats.length === 1;
+    if (!keepFontsInline) {
       for (const fontRelation of cssAsset.outgoingRelations) {
         const fontAsset = fontRelation.to;
         if (!fontAsset.isLoaded) {
@@ -1187,7 +1188,7 @@ These glyphs are used on your site, but they don't exist in the font you applied
       cssAsset.url = cssAssetUrl;
     }
 
-    if (!inlineFonts) {
+    if (!keepFontsInline) {
       for (const fontRelation of cssAsset.outgoingRelations) {
         const fontAsset = fontRelation.to;
 
@@ -1240,7 +1241,7 @@ These glyphs are used on your site, but they don't exist in the font you applied
     );
 
     let cssAssetInsertion = cssRelation;
-    if (jsPreload && !inlineFonts) {
+    if (jsPreload && !keepFontsInline) {
       // JS-based font preloading for browsers without <link rel="preload"> support
       const fontFaceContructorCalls = [];
 

diff --git a/test/parseCommandLineOptions.js b/test/parseCommandLineOptions.js
@@ -4,12 +4,7 @@ const parseCommandLineOptions = require('../lib/parseCommandLineOptions');
 describe('parseCommandLineOptions', function () {
   it('should return an object with the parsed options', function () {
     expect(
-      parseCommandLineOptions([
-        '--dryrun',
-        '--inline-fonts',
-        '--no-fallbacks',
-        '--recursive',
-      ]),
+      parseCommandLineOptions(['--dryrun', '--no-fallbacks', '--recursive']),
       'to satisfy',
       {
         root: undefined,
@@ -18,7 +13,6 @@ describe('parseCommandLineOptions', function () {
         debug: false,
         dryRun: true,
         silent: false,
-        inlineFonts: true,
         inlineCss: false,
         fontDisplay: 'swap',
         inPlace: false,

diff --git a/test/referenceImages.js b/test/referenceImages.js
@@ -4,7 +4,6 @@ const combos = require('combos');
 describe('reference images', function () {
   for (const options of combos({
     inlineCss: [false, true],
-    inlineFonts: [false, true],
     omitFallbacks: [false, true],
     dynamic: [false, true],
     harfbuzz: [false, true],