Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

Commit

Permalink
[flutter_tools] tree shake icons from web builds (#115886)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
christopherfujino committed Dec 15, 2022
1 parent ada4460 commit 1eaf5c0
Show file tree
Hide file tree
Showing 8 changed files with 83 additions and 42 deletions.
1 change: 1 addition & 0 deletions packages/flutter/lib/src/cupertino/icons.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions packages/flutter/lib/src/material/icons.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
11 changes: 11 additions & 0 deletions packages/flutter/lib/src/widgets/icon_data.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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].
Expand All @@ -161,6 +160,7 @@ class IconTreeShaker {
required String outputPath,
required String relativePath,
}) async {

if (!enabled) {
return false;
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter_tools/lib/src/commands/build_web.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ void main() {
'DartDefines': 'Zm9vPWE=,RkxVVFRFUl9XRUJfQVVUT19ERVRFQ1Q9dHJ1ZQ==',
'DartObfuscation': 'false',
'TrackWidgetCreation': 'false',
'TreeShakeIcons': 'false',
'TreeShakeIcons': 'true',
});
}),
});
Expand Down Expand Up @@ -187,7 +187,7 @@ void main() {
'DartDefines': 'RkxVVFRFUl9XRUJfQVVUT19ERVRFQ1Q9dHJ1ZQ==',
'DartObfuscation': 'false',
'TrackWidgetCreation': 'false',
'TreeShakeIcons': 'false',
'TreeShakeIcons': 'true',
});
}),
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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,
);
Expand All @@ -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 {
Expand Down Expand Up @@ -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',
Expand Down

0 comments on commit 1eaf5c0

Please sign in to comment.