Skip to content
This repository has been archived by the owner on May 1, 2020. It is now read-only.

Commit

Permalink
fix(postprocess): fix and add tests for the logic surrounding purging…
Browse files Browse the repository at this point in the history 
… fonts
  • Loading branch information
@danbucholtz
danbucholtz committed Sep 7, 2017
1 parent fbad33f commit 0dd1b22
Show file tree
Hide file tree
Showing 8 changed files with 228 additions and 106 deletions.
2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@ionic/app-scripts",
"version": "2.1.4-201709061759",
"version": "2.1.4",
"description": "Scripts for Ionic Projects",
"homepage": "https://ionicframework.com/",
"author": "Ionic Team <[email protected]> (https://ionic.io)",
Expand Down
129 changes: 129 additions & 0 deletions src/optimization/remove-unused-fonts.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
import { join } from 'path';

import { removeUnusedFonts } from './remove-unused-fonts';
import * as helpers from '../util/helpers';

describe('Remove Fonts', () => {
describe('removeUnusedFonts', () => {
it('should not purge any fonts when target is not cordova', () => {
const fakeFontDirPath = join(process.cwd(), 'www', 'assets', 'fonts');
spyOn(helpers, helpers.getStringPropertyValue.name).and.returnValue(fakeFontDirPath);
spyOn(helpers, helpers.readDirAsync.name).and.returnValue(Promise.resolve(getMockFontDirData()));
spyOn(helpers, helpers.unlinkAsync.name).and.returnValue(Promise.resolve());

return removeUnusedFonts({ target: 'notCordova', platform: 'web' }).then(() => {
expect(helpers.getStringPropertyValue).toHaveBeenCalled();
expect(helpers.readDirAsync).toHaveBeenCalledWith(fakeFontDirPath);
expect(helpers.unlinkAsync).not.toHaveBeenCalled();
});
});

it('should purge all non-woffs for ionicons and roboto, and then all of noto-sans for ios', () => {
const fakeFontDirPath = join(process.cwd(), 'www', 'assets', 'fonts');
spyOn(helpers, helpers.getStringPropertyValue.name).and.returnValue(fakeFontDirPath);
spyOn(helpers, helpers.readDirAsync.name).and.returnValue(Promise.resolve(getMockFontDirData()));
const unlinkSpy = spyOn(helpers, helpers.unlinkAsync.name).and.returnValue(Promise.resolve());

return removeUnusedFonts({ target: 'cordova', platform: 'ios' }).then(() => {
expect(helpers.readDirAsync).toHaveBeenCalledWith(fakeFontDirPath);
expect(unlinkSpy.calls.all()[0].args[0]).toEqual(join(fakeFontDirPath, 'ionicons.eot'));
expect(unlinkSpy.calls.all()[1].args[0]).toEqual(join(fakeFontDirPath, 'ionicons.scss'));
expect(unlinkSpy.calls.all()[2].args[0]).toEqual(join(fakeFontDirPath, 'ionicons.svg'));
expect(unlinkSpy.calls.all()[3].args[0]).toEqual(join(fakeFontDirPath, 'ionicons.ttf'));
expect(unlinkSpy.calls.all()[4].args[0]).toEqual(join(fakeFontDirPath, 'noto-sans-bold.ttf'));
expect(unlinkSpy.calls.all()[5].args[0]).toEqual(join(fakeFontDirPath, 'noto-sans-bold.woff'));
expect(unlinkSpy.calls.all()[6].args[0]).toEqual(join(fakeFontDirPath, 'noto-sans-regular.ttf'));
expect(unlinkSpy.calls.all()[7].args[0]).toEqual(join(fakeFontDirPath, 'noto-sans-regular.woff'));
expect(unlinkSpy.calls.all()[8].args[0]).toEqual(join(fakeFontDirPath, 'noto-sans.scss'));

expect(unlinkSpy.calls.all()[9].args[0]).toEqual(join(fakeFontDirPath, 'roboto-bold.ttf'));
expect(unlinkSpy.calls.all()[10].args[0]).toEqual(join(fakeFontDirPath, 'roboto-light.ttf'));
expect(unlinkSpy.calls.all()[11].args[0]).toEqual(join(fakeFontDirPath, 'roboto-medium.ttf'));
expect(unlinkSpy.calls.all()[12].args[0]).toEqual(join(fakeFontDirPath, 'roboto-regular.ttf'));
expect(unlinkSpy.calls.all()[13].args[0]).toEqual(join(fakeFontDirPath, 'roboto.scss'));
});
});

it('should purge all non-woffs for ionicons, all of roboto and noto-sans for android', () => {
const fakeFontDirPath = join(process.cwd(), 'www', 'assets', 'fonts');
spyOn(helpers, helpers.getStringPropertyValue.name).and.returnValue(fakeFontDirPath);
spyOn(helpers, helpers.readDirAsync.name).and.returnValue(Promise.resolve(getMockFontDirData()));
const unlinkSpy = spyOn(helpers, helpers.unlinkAsync.name).and.returnValue(Promise.resolve());

return removeUnusedFonts({ target: 'cordova', platform: 'android' }).then(() => {
expect(helpers.readDirAsync).toHaveBeenCalledWith(fakeFontDirPath);
expect(unlinkSpy.calls.all()[0].args[0]).toEqual(join(fakeFontDirPath, 'ionicons.eot'));
expect(unlinkSpy.calls.all()[1].args[0]).toEqual(join(fakeFontDirPath, 'ionicons.scss'));
expect(unlinkSpy.calls.all()[2].args[0]).toEqual(join(fakeFontDirPath, 'ionicons.svg'));
expect(unlinkSpy.calls.all()[3].args[0]).toEqual(join(fakeFontDirPath, 'ionicons.ttf'));
expect(unlinkSpy.calls.all()[4].args[0]).toEqual(join(fakeFontDirPath, 'noto-sans-bold.ttf'));
expect(unlinkSpy.calls.all()[5].args[0]).toEqual(join(fakeFontDirPath, 'noto-sans-bold.woff'));
expect(unlinkSpy.calls.all()[6].args[0]).toEqual(join(fakeFontDirPath, 'noto-sans-regular.ttf'));
expect(unlinkSpy.calls.all()[7].args[0]).toEqual(join(fakeFontDirPath, 'noto-sans-regular.woff'));
expect(unlinkSpy.calls.all()[8].args[0]).toEqual(join(fakeFontDirPath, 'noto-sans.scss'));

expect(unlinkSpy.calls.all()[9].args[0]).toEqual(join(fakeFontDirPath, 'roboto-bold.ttf'));
expect(unlinkSpy.calls.all()[10].args[0]).toEqual(join(fakeFontDirPath, 'roboto-bold.woff'));
expect(unlinkSpy.calls.all()[11].args[0]).toEqual(join(fakeFontDirPath, 'roboto-bold.woff2'));
expect(unlinkSpy.calls.all()[12].args[0]).toEqual(join(fakeFontDirPath, 'roboto-light.ttf'));
expect(unlinkSpy.calls.all()[13].args[0]).toEqual(join(fakeFontDirPath, 'roboto-light.woff'));
expect(unlinkSpy.calls.all()[14].args[0]).toEqual(join(fakeFontDirPath, 'roboto-light.woff2'));
expect(unlinkSpy.calls.all()[15].args[0]).toEqual(join(fakeFontDirPath, 'roboto-medium.ttf'));
expect(unlinkSpy.calls.all()[16].args[0]).toEqual(join(fakeFontDirPath, 'roboto-medium.woff'));
expect(unlinkSpy.calls.all()[17].args[0]).toEqual(join(fakeFontDirPath, 'roboto-medium.woff2'));
expect(unlinkSpy.calls.all()[18].args[0]).toEqual(join(fakeFontDirPath, 'roboto-regular.ttf'));
expect(unlinkSpy.calls.all()[19].args[0]).toEqual(join(fakeFontDirPath, 'roboto-regular.woff'));
expect(unlinkSpy.calls.all()[20].args[0]).toEqual(join(fakeFontDirPath, 'roboto-regular.woff2'));
expect(unlinkSpy.calls.all()[21].args[0]).toEqual(join(fakeFontDirPath, 'roboto.scss'));

});
});

it('should purge all non-woffs for ionicons, all of roboto and noto-sans for windows', () => {
const fakeFontDirPath = join(process.cwd(), 'www', 'assets', 'fonts');
spyOn(helpers, helpers.getStringPropertyValue.name).and.returnValue(fakeFontDirPath);
spyOn(helpers, helpers.readDirAsync.name).and.returnValue(Promise.resolve(getMockFontDirData()));
const unlinkSpy = spyOn(helpers, helpers.unlinkAsync.name).and.returnValue(Promise.resolve());

return removeUnusedFonts({ target: 'cordova', platform: 'windows' }).then(() => {
expect(helpers.readDirAsync).toHaveBeenCalledWith(fakeFontDirPath);
expect(helpers.unlinkAsync).not.toHaveBeenCalled();
});
});
});
});

function getMockFontDirData() {
return [
'ionicons.eot',
'ionicons.scss',
'ionicons.svg',
'ionicons.ttf',
'ionicons.woff',
'ionicons.woff2',
'noto-sans-bold.ttf',
'noto-sans-bold.woff',
'noto-sans-regular.ttf',
'noto-sans-regular.woff',
'noto-sans.scss',
'roboto-bold.ttf',
'roboto-bold.woff',
'roboto-bold.woff2',
'roboto-light.ttf',
'roboto-light.woff',
'roboto-light.woff2',
'roboto-medium.ttf',
'roboto-medium.woff',
'roboto-medium.woff2',
'roboto-regular.ttf',
'roboto-regular.woff',
'roboto-regular.woff2',
'roboto.scss',
'my-custom-font.eot',
'my-custom-font.scss',
'my-custom-font.svg',
'my-custom-font.ttf',
'my-custom-font.woff',
'my-custom-font.woff2'
];
}
141 changes: 59 additions & 82 deletions src/optimization/remove-unused-fonts.ts
View file
Original file line number Diff line number Diff line change
@@ -1,91 +1,68 @@
import { BuildContext } from '../util/interfaces';
import { join } from 'path';
import { Logger } from '../logger/logger';
import { unlinkAsync } from '../util/helpers';
import * as glob from 'glob';


export function removeUnusedFonts(context: BuildContext) {
// For webapps, we pretty much need all fonts to be available because
// the web server deployment never knows which browser/platform is
// opening the app. Additionally, webapps will request fonts on-demand,
// so having them all sit in the www/assets/fonts directory doesn’t
// hurt anything if it’s never being requested.

// However, with Cordova, the entire directory gets bundled and
// shipped in the ipa/apk, but we also know exactly which platform
// is opening the webapp. For this reason we can safely delete font
// files we know would never be opened by the platform. So app-scripts
// will continue to copy all font files over, but the cordova build
// process would delete those we know are useless and just taking up
// space. End goal is that the Cordova ipa/apk filesize is smaller.
import { extname, join } from 'path';

// Font Format Support:
// ttf: http://caniuse.com/#feat=ttf
// woff: http://caniuse.com/#feat=woff
// woff2: http://caniuse.com/#feat=woff2

if (context.target === 'cordova') {
const fontsRemoved: string[] = [];
// all cordova builds should remove .eot, .svg, .ttf, and .scss files
fontsRemoved.push('*.eot');
fontsRemoved.push('*.ttf');
fontsRemoved.push('*.svg');
fontsRemoved.push('*.scss');
import { Logger } from '../logger/logger';
import * as Constants from '../util/constants';
import { getStringPropertyValue, readDirAsync, unlinkAsync } from '../util/helpers';
import { BuildContext } from '../util/interfaces';

// all cordova builds should remove Noto-Sans
// Only windows would use Noto-Sans, and it already comes with
// a system font so it wouldn't need our own copy.
fontsRemoved.push('noto-sans*');

if (context.platform === 'android') {
// Remove all Roboto fonts. Android already comes with Roboto system
// fonts so shipping our own is unnecessary. Including roboto fonts
// is only useful for PWAs and during development.
fontsRemoved.push('roboto*');
// For webapps, we pretty much need all fonts to be available because
// the web server deployment never knows which browser/platform is
// opening the app. Additionally, webapps will request fonts on-demand,
// so having them all sit in the www/assets/fonts directory doesn’t
// hurt anything if it’s never being requested.

// However, with Cordova, the entire directory gets bundled and
// shipped in the ipa/apk, but we also know exactly which platform
// is opening the webapp. For this reason we can safely delete font
// files we know would never be opened by the platform. So app-scripts
// will continue to copy all font files over, but the cordova build
// process would delete those we know are useless and just taking up
// space. End goal is that the Cordova ipa/apk filesize is smaller.

// Font Format Support:
// ttf: http://caniuse.com/#feat=ttf
// woff: http://caniuse.com/#feat=woff
// woff2: http://caniuse.com/#feat=woff2
export function removeUnusedFonts(context: BuildContext): Promise<any> {
const fontDir = getStringPropertyValue(Constants.ENV_VAR_FONTS_DIR);
return readDirAsync(fontDir).then((fileNames: string[]) => {
fileNames = fileNames.sort();
const toPurge = getFontFileNamesToPurge(context.target, context.platform, fileNames);
const fullPaths = toPurge.map(fileName => join(fontDir, fileName));
const promises = fullPaths.map(fullPath => unlinkAsync(fullPath));
return Promise.all(promises);
});
}

} else if (context.platform === 'ios') {
// Keep Roboto for now. Apps built for iOS may still use Material Design,
// so in that case Roboto should be available. Later we can improve the
// CLI to be smarter and read the user’s ionic config. Also, the roboto
// fonts themselves are pretty small.
export function getFontFileNamesToPurge(target: string, platform: string, fileNames: string[]): string[] {
if (target !== Constants.CORDOVA) {
return [];
}
const filesToDelete = new Set<string>();
for (const fileName of fileNames) {
if (platform === 'android') {
// remove noto-sans, roboto, and non-woff ionicons
if (fileName.startsWith('noto-sans') || fileName.startsWith('roboto') || (isIonicons(fileName) && !isWoof(fileName))) {
filesToDelete.add(fileName);
}
} else if (platform === 'ios') {
// remove noto-sans, non-woff ionicons
if (fileName.startsWith('noto-sans') || (fileName.startsWith('roboto') && !isWoof(fileName)) || (isIonicons(fileName) && !isWoof(fileName))) {
filesToDelete.add(fileName);
}
}
// for now don't bother deleting anything for windows, need to get some info first

let filesToDelete: string[] = [];

let promises = fontsRemoved.map(pattern => {
return new Promise(resolve => {
let searchPattern = join(context.wwwDir, 'assets', 'fonts', pattern);

glob(searchPattern, (err, files) => {
if (err) {
Logger.error(`removeUnusedFonts: ${err}`);

} else {
files.forEach(f => {
if (filesToDelete.indexOf(f) === -1) {
filesToDelete.push(f);
}
});
}

resolve();
});

});
});

return Promise.all(promises).then(() => {
return unlinkAsync(filesToDelete).then(() => {
if (filesToDelete.length) {
Logger.info(`removed unused font files`);
return true;
}
return false;
});
});
}
return Array.from(filesToDelete);
}

function isIonicons(fileName: string) {
return fileName.startsWith('ionicons');
}

// nothing to do here, carry on
return Promise.resolve();
// woof woof
function isWoof(fileName: string) {
return extname(fileName) === '.woff' || extname(fileName) === '.woff2';
}
43 changes: 24 additions & 19 deletions src/postprocess.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,27 +22,32 @@ export function postprocess(context: BuildContext) {


function postprocessWorker(context: BuildContext) {
return Promise.all([
purgeSourceMapsIfNeeded(context),
removeUnusedFonts(context),
updateIndexHtml(context),
writeFilesToDisk(context)
]);
}
const promises: Promise<any>[] = [];
promises.push(purgeSourceMapsIfNeeded(context));
promises.push(updateIndexHtml(context));

export function writeFilesToDisk(context: BuildContext) {
if (getBooleanPropertyValue(Constants.ENV_AOT_WRITE_TO_DISK)) {
emptyDirSync(context.tmpDir);
const files = context.fileCache.getAll();
files.forEach(file => {
const dirName = dirname(file.path);
const relativePath = relative(process.cwd(), dirName);
const tmpPath = join(context.tmpDir, relativePath);
const fileName = basename(file.path);
const fileToWrite = join(tmpPath, fileName);
mkdirpSync(tmpPath);
writeFileSync(fileToWrite, file.content);
});
promises.push(writeFilesToDisk(context));
}

if (context.optimizeJs && getBooleanPropertyValue(Constants.ENV_PURGE_UNUSED_FONTS)) {
promises.push(removeUnusedFonts(context));
}

return Promise.all(promises);
}

export function writeFilesToDisk(context: BuildContext) {
emptyDirSync(context.tmpDir);
const files = context.fileCache.getAll();
files.forEach(file => {
const dirName = dirname(file.path);
const relativePath = relative(process.cwd(), dirName);
const tmpPath = join(context.tmpDir, relativePath);
const fileName = basename(file.path);
const fileToWrite = join(tmpPath, fileName);
mkdirpSync(tmpPath);
writeFileSync(fileToWrite, file.content);
});
return Promise.resolve();
}
1 change: 1 addition & 0 deletions src/util/config.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ describe('config', () => {
expect(context.wwwDir).toEqual(join(process.cwd(), Constants.WWW_DIR));
expect(context.wwwIndex).toEqual('index.html');
expect(context.buildDir).toEqual(join(process.cwd(), Constants.WWW_DIR, Constants.BUILD_DIR));
expect(fakeConfig[Constants.ENV_VAR_FONTS_DIR]).toEqual(join(context.wwwDir, 'assets', 'fonts'));
expect(context.pagesDir).toEqual(join(context.srcDir, 'pages'));
expect(context.componentsDir).toEqual(join(context.srcDir, 'components'));
expect(context.directivesDir).toEqual(join(context.srcDir, 'directives'));
Expand Down
8 changes: 8 additions & 0 deletions src/util/config.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,10 @@ export function generateContext(context?: BuildContext): BuildContext {
setProcessEnvVar(Constants.ENV_VAR_BUILD_DIR, context.buildDir);
Logger.debug(`buildDir set to ${context.buildDir}`);

const fontsDir = resolve(getConfigValue(context, '--fontsDir', null, Constants.ENV_VAR_FONTS_DIR, Constants.ENV_VAR_FONTS_DIR.toLowerCase(), join(context.wwwDir, 'assets', 'fonts')));
setProcessEnvVar(Constants.ENV_VAR_FONTS_DIR, fontsDir);
Logger.debug(`fontsDir set to ${fontsDir}`);

context.sourcemapDir = resolve(context.sourcemapDir || getConfigValue(context, '--sourcemapDir', null, Constants.ENV_VAR_SOURCEMAP_DIR, Constants.ENV_VAR_SOURCEMAP_DIR.toLowerCase(), Constants.SOURCEMAP_DIR));
setProcessEnvVar(Constants.ENV_VAR_SOURCEMAP_DIR, context.sourcemapDir);
Logger.debug(`sourcemapDir set to ${context.sourcemapDir}`);
Expand Down Expand Up @@ -276,6 +280,10 @@ export function generateContext(context?: BuildContext): BuildContext {
setProcessEnvVar(Constants.ENV_POLYFILL_FILE_NAME, polyfillName);
Logger.debug(`polyfillName set to ${polyfillName}`);

const purgeUnusedFonts = getConfigValue(context, '--purgeUnusedFonts', null, Constants.ENV_PURGE_UNUSED_FONTS, Constants.ENV_PURGE_UNUSED_FONTS.toLowerCase(), 'true');
setProcessEnvVar(Constants.ENV_PURGE_UNUSED_FONTS, purgeUnusedFonts);
Logger.debug(`purgeUnusedFonts set to ${purgeUnusedFonts}`);

/* Provider Path Stuff */
setProcessEnvVar(Constants.ENV_ACTION_SHEET_CONTROLLER_CLASSNAME, 'ActionSheetController');
setProcessEnvVar(Constants.ENV_ACTION_SHEET_CONTROLLER_PATH, join(context.ionicAngularDir, 'components', 'action-sheet', 'action-sheet-controller.js'));
Expand Down
4 changes: 3 additions & 1 deletion src/util/constants.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export const PROVIDER = 'provider';
export const TABS = 'tabs';
export const AT_ANGULAR = '@angular';
export const RXJS = 'rxjs';
export const CORDOVA = 'cordova';

export const ENV_VAR_PROD = 'prod';
export const ENV_VAR_DEV = 'dev';
Expand All @@ -36,6 +37,7 @@ export const ENV_VAR_PIPES_DIR = 'IONIC_PIPES_DIR';
export const ENV_VAR_PROVIDERS_DIR = 'IONIC_PROVIDERS_DIR';
export const ENV_VAR_TMP_DIR = 'IONIC_TMP_DIR';
export const ENV_VAR_WWW_DIR = 'IONIC_WWW_DIR';
export const ENV_VAR_FONTS_DIR = 'IONIC_FONTS_DIR';
export const ENV_VAR_SOURCEMAP_DIR = 'IONIC_SOURCEMAP_DIR';
export const ENV_VAR_HTML_TO_SERVE = 'IONIC_HTML_TO_SERVE';
export const ENV_VAR_BUILD_DIR = 'IONIC_BUILD_DIR';
Expand Down Expand Up @@ -83,7 +85,7 @@ export const ENV_NG_MODULE_FILE_NAME_SUFFIX = 'IONIC_NG_MODULE_FILENAME_SUFFIX';
export const ENV_POLYFILL_FILE_NAME = 'IONIC_POLYFILL_FILE_NAME';
export const ENV_PRINT_WEBPACK_DEPENDENCY_TREE = 'IONIC_PRINT_WEBPACK_DEPENDENCY_TREE';
export const ENV_PARSE_DEEPLINKS = 'IONIC_PARSE_DEEPLINKS';

export const ENV_PURGE_UNUSED_FONTS = 'IONIC_PURGE_UNUSED_FONTS';


/* Providers */
Expand Down
Loading

0 comments on commit 0dd1b22

Please sign in to comment.