Unverified Commit 1eaf5c0f authored by Christopher Fujino's avatar Christopher Fujino Committed by GitHub

[flutter_tools] tree shake icons from web builds (#115886)

* wip

* remove temp text file

* fix tests

* add test

* default to off

* restore gitignore

* update

* apply annotation to cupertino icons as well

* update reference to library in icon_tree_shaker.dart

* update tests

* fix tests

* remove hack to skip non-const check on web

* add hint about how much reduction and test
parent ada44605
......@@ -59,6 +59,7 @@ import 'package:flutter/widgets.dart';
/// See also:
///
/// * [Icon], used to show these icons.
@staticIconProvider
class CupertinoIcons {
// This class is not meant to be instantiated or extended; this constructor
// prevents instantiation and extension.
......
......@@ -149,6 +149,7 @@ class PlatformAdaptiveIcons implements Icons {
/// * [IconButton]
/// * <https://material.io/resources/icons>
/// * [AnimatedIcons], for the list of available animated Material Icons.
@staticIconProvider
class Icons {
// This class is not meant to be instantiated or extended; this constructor
// prevents instantiation and extension.
......@@ -93,3 +93,14 @@ class IconDataProperty extends DiagnosticsProperty<IconData> {
return json;
}
}
class _StaticIconProvider {
const _StaticIconProvider();
}
/// Annotation for classes that only provide static const [IconData] instances.
///
/// This is a hint to the font tree shaker to ignore the constant instances
/// of [IconData] appearing in the class when tracking which code points
/// should be retained in the bundled font.
const Object staticIconProvider = _StaticIconProvider();
......@@ -190,7 +190,7 @@ class KernelSnapshot extends Target {
// Force linking of the platform for desktop embedder targets since these
// do not correctly load the core snapshots in debug mode.
// See https://github.com/flutter/flutter/issues/44724
bool forceLinkPlatform;
final bool forceLinkPlatform;
switch (targetPlatform) {
case TargetPlatform.darwin:
case TargetPlatform.windows_x64:
......
......@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
import 'package:meta/meta.dart';
import 'package:mime/mime.dart' as mime;
import 'package:process/process.dart';
......@@ -150,8 +151,6 @@ class IconTreeShaker {
/// Calls font-subset, which transforms the [input] font file to a
/// subsetted version at [outputPath].
///
/// All parameters are required.
///
/// If [enabled] is false, or the relative path is not recognized as an icon
/// font used in the Flutter application, this returns false.
/// If the font-subset subprocess fails, it will [throwToolExit].
......@@ -161,6 +160,7 @@ class IconTreeShaker {
required String outputPath,
required String relativePath,
}) async {
if (!enabled) {
return false;
}
......@@ -212,9 +212,23 @@ class IconTreeShaker {
_logger.printError(await utf8.decodeStream(fontSubsetProcess.stderr));
throw IconTreeShakerException._('Font subsetting failed with exit code $code.');
}
_logger.printStatus(getSubsetSummaryMessage(input, _fs.file(outputPath)));
return true;
}
@visibleForTesting
String getSubsetSummaryMessage(File inputFont, File outputFont) {
final String fontName = inputFont.basename;
final double inputSize = inputFont.lengthSync().toDouble();
final double outputSize = outputFont.lengthSync().toDouble();
final double reductionBytes = inputSize - outputSize;
final String reductionPercentage = (reductionBytes / inputSize * 100).toStringAsFixed(1);
return 'Font asset "$fontName" was tree-shaken, reducing it from '
'${inputSize.ceil()} to ${outputSize.ceil()} bytes '
'($reductionPercentage% reduction). Tree-shaking can be disabled '
'by providing the --no-tree-shake-icons flag when building your app.';
}
/// Returns a map of { fontFamily: relativePath } pairs.
Future<Map<String, String>> _parseFontJson(
String fontManifestData,
......@@ -268,6 +282,8 @@ class IconTreeShaker {
'--kernel-file', appDill.path,
'--class-library-uri', 'package:flutter/src/widgets/icon_data.dart',
'--class-name', 'IconData',
'--annotation-class-name', '_StaticIconProvider',
'--annotation-class-library-uri', 'package:flutter/src/widgets/icon_data.dart',
];
_logger.printTrace('Running command: ${cmd.join(' ')}');
final ProcessResult constFinderProcessResult = await _processManager.run(cmd);
......
......@@ -19,7 +19,7 @@ class BuildWebCommand extends BuildSubCommand {
required FileSystem fileSystem,
required bool verboseHelp,
}) : _fileSystem = fileSystem, super(verboseHelp: verboseHelp) {
addTreeShakeIconsFlag(enabledByDefault: false);
addTreeShakeIconsFlag();
usesTargetOption();
usesOutputDir();
usesPubOption();
......
......@@ -137,7 +137,7 @@ void main() {
'DartDefines': 'Zm9vPWE=,RkxVVFRFUl9XRUJfQVVUT19ERVRFQ1Q9dHJ1ZQ==',
'DartObfuscation': 'false',
'TrackWidgetCreation': 'false',
'TreeShakeIcons': 'false',
'TreeShakeIcons': 'true',
});
}),
});
......@@ -187,7 +187,7 @@ void main() {
'DartDefines': 'RkxVVFRFUl9XRUJfQVVUT19ERVRFQ1Q9dHJ1ZQ==',
'DartObfuscation': 'false',
'TrackWidgetCreation': 'false',
'TreeShakeIcons': 'false',
'TreeShakeIcons': 'true',
});
}),
});
......
......@@ -40,6 +40,8 @@ void main() {
'--kernel-file', appDillPath,
'--class-library-uri', 'package:flutter/src/widgets/icon_data.dart',
'--class-name', 'IconData',
'--annotation-class-name', '_StaticIconProvider',
'--annotation-class-library-uri', 'package:flutter/src/widgets/icon_data.dart',
];
void addConstFinderInvocation(
......@@ -227,13 +229,18 @@ void main() {
fileSystem: fileSystem,
artifacts: artifacts,
);
final CompleterIOSink stdinSink = CompleterIOSink();
addConstFinderInvocation(appDill.path, stdout: validConstFinderResult);
resetFontSubsetInvocation(stdinSink: stdinSink);
// Font starts out 2500 bytes long
final File inputFont = fileSystem.file(inputPath)
..writeAsBytesSync(List<int>.filled(2500, 0));
// after subsetting, font is 1200 bytes long
fileSystem.file(outputPath)
..createSync(recursive: true)
..writeAsBytesSync(List<int>.filled(1200, 0));
bool subsetted = await iconTreeShaker.subsetFont(
input: fileSystem.file(inputPath),
input: inputFont,
outputPath: outputPath,
relativePath: relativePath,
);
......@@ -249,6 +256,10 @@ void main() {
expect(subsetted, true);
expect(stdinSink.getAndClear(), '59470\n');
expect(processManager, hasNoRemainingExpectations);
expect(
logger.statusText,
contains('Font asset "MaterialIcons-Regular.otf" was tree-shaken, reducing it from 2500 to 1200 bytes (52.0% reduction). Tree-shaking can be disabled by providing the --no-tree-shake-icons flag when building your app.'),
);
});
testWithoutContext('Does not subset a non-supported font', () async {
......@@ -315,40 +326,41 @@ void main() {
expect(subsetted, false);
});
testWithoutContext('Non-constant instances', () async {
final Environment environment = createEnvironment(<String, String>{
kIconTreeShakerFlag: 'true',
kBuildMode: 'release',
for (final TargetPlatform platform in <TargetPlatform>[TargetPlatform.android_arm, TargetPlatform.web_javascript]) {
testWithoutContext('Non-constant instances $platform', () async {
final Environment environment = createEnvironment(<String, String>{
kIconTreeShakerFlag: 'true',
kBuildMode: 'release',
});
final File appDill = environment.buildDir.childFile('app.dill')
..createSync(recursive: true);
final IconTreeShaker iconTreeShaker = IconTreeShaker(
environment,
fontManifestContent,
logger: logger,
processManager: processManager,
fileSystem: fileSystem,
artifacts: artifacts,
);
addConstFinderInvocation(appDill.path, stdout: constFinderResultWithInvalid);
await expectLater(
() => iconTreeShaker.subsetFont(
input: fileSystem.file(inputPath),
outputPath: outputPath,
relativePath: relativePath,
),
throwsToolExit(
message:
'Avoid non-constant invocations of IconData or try to build'
' again with --no-tree-shake-icons.',
),
);
expect(processManager, hasNoRemainingExpectations);
});
final File appDill = environment.buildDir.childFile('app.dill')
..createSync(recursive: true);
final IconTreeShaker iconTreeShaker = IconTreeShaker(
environment,
fontManifestContent,
logger: logger,
processManager: processManager,
fileSystem: fileSystem,
artifacts: artifacts,
);
addConstFinderInvocation(appDill.path, stdout: constFinderResultWithInvalid);
await expectLater(
() => iconTreeShaker.subsetFont(
input: fileSystem.file(inputPath),
outputPath: outputPath,
relativePath: relativePath,
),
throwsToolExit(
message:
'Avoid non-constant invocations of IconData or try to build'
' again with --no-tree-shake-icons.',
),
);
expect(processManager, hasNoRemainingExpectations);
});
}
testWithoutContext('Non-zero font-subset exit code', () async {
final Environment environment = createEnvironment(<String, String>{
kIconTreeShakerFlag: 'true',
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment