Skip to content

Commit

Permalink
Only warn about missing fonttools install if we are actually trying t…
Browse files Browse the repository at this point in the history
…o subset local fonts
  • Loading branch information
Munter committed Nov 2, 2019
1 parent 8bb72e1 commit 3167d02
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 8 deletions.
24 changes: 17 additions & 7 deletions lib/subsetFonts.js
Original file line number Diff line number Diff line change
Expand Up @@ -287,13 +287,7 @@ async function getSubsetsForFontUsage(

try {
subsetLocalFont = require('./subsetLocalFont');
} catch (err) {
assetGraph.info(
new Error(
'Local subsetting is not possible because fonttools are not installed. Falling back to only subsetting Google Fonts. Run `pip install fonttools brotli zopfli` to enable local font subsetting'
)
);
}
} catch (err) {}

const allFonts = [];

Expand Down Expand Up @@ -375,6 +369,7 @@ async function getSubsetsForFontUsage(

await Promise.all(Object.values(subsetPromiseMap));
} else {
const localFonts = [];
const fontCssUrlMap = {};

for (const item of htmlAssetTextsWithProps) {
Expand All @@ -386,6 +381,7 @@ async function getSubsetsForFontUsage(
const fontAsset = assetGraph.findAssets({ url: fontUsage.fontUrl })[0];

if (fontAsset.hostname !== 'fonts.gstatic.com') {
localFonts.push(fontAsset);
continue;
}

Expand All @@ -402,6 +398,20 @@ async function getSubsetsForFontUsage(
}
}

if (localFonts.length > 0) {
const localFontDescriptions = localFonts
.map(a => ` - ${a.urlOrDescription}`)
.join('\n');
assetGraph.info(
new Error(
`Local subsetting is not possible because fonttools are not installed. Falling back to only subsetting Google Fonts. Run \`pip install fonttools brotli zopfli\` to enable local font subsetting`
)
);
assetGraph.info(
new Error(`Unoptimised fonts:\n${localFontDescriptions}`)
);
}

const assetGraphForLoadingFonts = new AssetGraph();

for (const format of formats) {
Expand Down
13 changes: 12 additions & 1 deletion test/subsetFonts.js
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,14 @@ describe('subsetFonts', function() {
});

expect(infos, 'to satisfy', [
expect.it('to be an', Error) // Can't get the right type of error due to limited mocking abilities
expect.it(
'to have message',
'Local subsetting is not possible because fonttools are not installed. Falling back to only subsetting Google Fonts. Run `pip install fonttools brotli zopfli` to enable local font subsetting'
),
expect.it(
'to have message',
'Unoptimised fonts:\n - testdata/subsetFonts/local-single/OpenSans.ttf'
)
]);
});

Expand Down Expand Up @@ -237,6 +244,10 @@ describe('subsetFonts', function() {
message:
'Local subsetting is not possible because fonttools are not installed. Falling back to only subsetting Google Fonts. Run `pip install fonttools brotli zopfli` to enable local font subsetting'
},
{
message:
'Unoptimised fonts:\n - testdata/subsetFonts/existing-prefetch/OpenSans.ttf'
},
{
message:
'Detached <link rel="prefetch" as="font" type="application/x-font-ttf" href="OpenSans.ttf">. Will be replaced with preload with JS fallback.\nIf you feel this is wrong, open an issue at https://github.com/assetgraph/assetgraph/issues',
Expand Down

0 comments on commit 3167d02

Please sign in to comment.