[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
flutter · Dec 15, 2022 · 1eaf5c0 · 1eaf5c0
1 parent ada4460
commit 1eaf5c0
Show file tree

Hide file tree

Showing 8 changed files with 83 additions and 42 deletions.
diff --git a/packages/flutter/lib/src/cupertino/icons.dart b/packages/flutter/lib/src/cupertino/icons.dart
@@ -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.

diff --git a/packages/flutter/lib/src/material/icons.dart b/packages/flutter/lib/src/material/icons.dart
@@ -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.

diff --git a/packages/flutter/lib/src/widgets/icon_data.dart b/packages/flutter/lib/src/widgets/icon_data.dart
@@ -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();
diff --git a/packages/flutter_tools/lib/src/build_system/targets/common.dart b/packages/flutter_tools/lib/src/build_system/targets/common.dart
@@ -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:

diff --git a/packages/flutter_tools/lib/src/build_system/targets/icon_tree_shaker.dart b/packages/flutter_tools/lib/src/build_system/targets/icon_tree_shaker.dart
@@ -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);

diff --git a/packages/flutter_tools/lib/src/commands/build_web.dart b/packages/flutter_tools/lib/src/commands/build_web.dart
@@ -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();

diff --git a/packages/flutter_tools/test/commands.shard/hermetic/build_web_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/build_web_test.dart
@@ -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',
       });
     }),
   });

diff --git a/packages/flutter_tools/test/general.shard/build_system/targets/icon_tree_shaker_test.dart b/packages/flutter_tools/test/general.shard/build_system/targets/icon_tree_shaker_test.dart
@@ -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',