From c411f588c6cfdaa4f0e962e6deef7b1a8a089d56 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Thu, 5 Mar 2020 13:02:22 -0800 Subject: [PATCH] Revert "[flutter_tools] supports tree-shake-icons for web builds (#51808)" (#52045) This reverts commit aed961993dd67489f544d4aad867ed18b49b829f. --- .../targets/icon_tree_shaker.dart | 4 +- .../lib/src/build_system/targets/web.dart | 45 +-- .../flutter_tools/lib/src/web/compile.dart | 4 +- .../build_system/targets/web_test.dart | 290 ++++++++---------- 4 files changed, 148 insertions(+), 195 deletions(-) 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 index 8e85481264..b401ec969d 100644 --- 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 @@ -289,7 +289,7 @@ class IconTreeShaker { for (final Map iconDataMap in consts.constantInstances) { if ((iconDataMap['fontPackage'] ?? '') is! String || // Null is ok here. iconDataMap['fontFamily'] is! String || - iconDataMap['codePoint'] is! num) { + iconDataMap['codePoint'] is! int) { throw IconTreeShakerException._( 'Invalid ConstFinder result. Expected "fontPackage" to be a String, ' '"fontFamily" to be a String, and "codePoint" to be an int, ' @@ -301,7 +301,7 @@ class IconTreeShaker { ? family : 'packages/$package/$family'; result[key] ??= []; - result[key].add((iconDataMap['codePoint'] as num).round()); + result[key].add(iconDataMap['codePoint'] as int); } return result; } diff --git a/packages/flutter_tools/lib/src/build_system/targets/web.dart b/packages/flutter_tools/lib/src/build_system/targets/web.dart index 92c3a5834a..79618de8e0 100644 --- a/packages/flutter_tools/lib/src/build_system/targets/web.dart +++ b/packages/flutter_tools/lib/src/build_system/targets/web.dart @@ -156,31 +156,9 @@ class Dart2JSTarget extends Target { final BuildMode buildMode = getBuildModeForName(environment.defines[kBuildMode]); final String specPath = globals.fs.path.join(globals.artifacts.getArtifactPath(Artifact.flutterWebSdk), 'libraries.json'); final String packageFile = PackageMap.globalPackagesPath; - final File outputKernel = environment.buildDir.childFile('app.dill'); final File outputFile = environment.buildDir.childFile('main.dart.js'); - // Run the dart2js compilation in two stages, so that icon tree shaking can - // parse the kernel file for web builds. - final ProcessResult kernelResult = await globals.processManager.run([ - globals.artifacts.getArtifactPath(Artifact.engineDartBinary), - globals.artifacts.getArtifactPath(Artifact.dart2jsSnapshot), - '--libraries-spec=$specPath', - '-o', - outputKernel.path, - '--packages=$packageFile', - if (buildMode == BuildMode.profile) - '-Ddart.vm.profile=true' - else - '-Ddart.vm.product=true', - for (final String dartDefine in parseDartDefines(environment)) - '-D$dartDefine', - '--cfe-only', - environment.buildDir.childFile('main.dart').path, - ]); - if (kernelResult.exitCode != 0) { - throw Exception(kernelResult.stdout + kernelResult.stderr); - } - final ProcessResult javaScriptResult = await globals.processManager.run([ + final ProcessResult result = await globals.processManager.run([ globals.artifacts.getArtifactPath(Artifact.engineDartBinary), globals.artifacts.getArtifactPath(Artifact.dart2jsSnapshot), '--libraries-spec=$specPath', @@ -190,17 +168,24 @@ class Dart2JSTarget extends Target { '-O4', if (buildMode == BuildMode.profile) '--no-minify', - if (csp) - '--csp', '-o', outputFile.path, - environment.buildDir.childFile('app.dill').path, + '--packages=$packageFile', + if (buildMode == BuildMode.profile) + '-Ddart.vm.profile=true' + else + '-Ddart.vm.product=true', + if (csp) + '--csp', + for (final String dartDefine in parseDartDefines(environment)) + '-D$dartDefine', + environment.buildDir.childFile('main.dart').path, ]); - if (javaScriptResult.exitCode != 0) { - throw Exception(javaScriptResult.stdout + javaScriptResult.stderr); + if (result.exitCode != 0) { + throw Exception(result.stdout + result.stderr); } final File dart2jsDeps = environment.buildDir - .childFile('app.dill.deps'); + .childFile('main.dart.js.deps'); if (!dart2jsDeps.existsSync()) { globals.printError('Warning: dart2js did not produced expected deps list at ' '${dart2jsDeps.path}'); @@ -212,7 +197,7 @@ class Dart2JSTarget extends Target { platform: globals.platform, ); final Depfile depfile = depfileService.parseDart2js( - environment.buildDir.childFile('app.dill.deps'), + environment.buildDir.childFile('main.dart.js.deps'), outputFile, ); depfileService.writeToFile( diff --git a/packages/flutter_tools/lib/src/web/compile.dart b/packages/flutter_tools/lib/src/web/compile.dart index 6d8acc5c0a..15a1f6e05f 100644 --- a/packages/flutter_tools/lib/src/web/compile.dart +++ b/packages/flutter_tools/lib/src/web/compile.dart @@ -11,7 +11,6 @@ import '../base/logger.dart'; import '../build_info.dart'; import '../build_system/build_system.dart'; import '../build_system/targets/dart.dart'; -import '../build_system/targets/icon_tree_shaker.dart'; import '../build_system/targets/web.dart'; import '../convert.dart'; import '../globals.dart' as globals; @@ -53,7 +52,8 @@ Future buildWeb( kHasWebPlugins: hasWebPlugins.toString(), kDartDefines: jsonEncode(dartDefines), kCspMode: csp.toString(), - kIconTreeShakerFlag: buildInfo.treeShakeIcons.toString(), + // TODO(dnfield): Enable font subset. We need to get a kernel file to do + // that. https://github.com/flutter/flutter/issues/49730 }, )); if (!result.success) { diff --git a/packages/flutter_tools/test/general.shard/build_system/targets/web_test.dart b/packages/flutter_tools/test/general.shard/build_system/targets/web_test.dart index 8e3161adf2..3e8002b7d3 100644 --- a/packages/flutter_tools/test/general.shard/build_system/targets/web_test.dart +++ b/packages/flutter_tools/test/general.shard/build_system/targets/web_test.dart @@ -16,38 +16,35 @@ import 'package:process/process.dart'; import 'package:platform/platform.dart'; import '../../../src/common.dart'; -import '../../../src/context.dart'; import '../../../src/mocks.dart'; import '../../../src/testbed.dart'; -const List kDart2jsLinuxArgs = [ - 'bin/cache/dart-sdk/bin/dart', - 'bin/cache/dart-sdk/bin/snapshots/dart2js.dart.snapshot', - '--libraries-spec=bin/cache/flutter_web_sdk/libraries.json', -]; - void main() { Testbed testbed; Environment environment; - FakeProcessManager processManager; - final Platform linux = FakePlatform( - operatingSystem: 'linux', - environment: {}, - ); - final Platform windows = FakePlatform( - operatingSystem: 'windows', - environment: {}, - ); + MockPlatform mockPlatform; + MockPlatform mockWindowsPlatform; DepfileService depfileService; setUp(() { + mockPlatform = MockPlatform(); + mockWindowsPlatform = MockPlatform(); + + when(mockPlatform.isWindows).thenReturn(false); + when(mockPlatform.isMacOS).thenReturn(true); + when(mockPlatform.isLinux).thenReturn(false); + when(mockPlatform.environment).thenReturn(const {}); + + when(mockWindowsPlatform.isWindows).thenReturn(true); + when(mockWindowsPlatform.isMacOS).thenReturn(false); + when(mockWindowsPlatform.isLinux).thenReturn(false); + testbed = Testbed(setup: () { final File packagesFile = globals.fs.file(globals.fs.path.join('foo', '.packages')) ..createSync(recursive: true) ..writeAsStringSync('foo:lib/\n'); PackageMap.globalPackagesPath = packagesFile.path; globals.fs.currentDirectory.childDirectory('bar').createSync(); - processManager = FakeProcessManager.list([]); environment = Environment.test( globals.fs.currentDirectory, @@ -64,7 +61,7 @@ void main() { ); environment.buildDir.createSync(recursive: true); }, overrides: { - Platform: () => linux, + Platform: () => mockPlatform, }); }); @@ -159,7 +156,7 @@ void main() { // Import. expect(generated, contains("import 'package:foo/main.dart' as entrypoint;")); }, overrides: { - Platform: () => windows, + Platform: () => mockWindowsPlatform, })); test('WebEntrypointTarget generates an entrypoint without plugins and init platform', () => testbed.run(() async { @@ -219,131 +216,111 @@ void main() { test('Dart2JSTarget calls dart2js with expected args with csp', () => testbed.run(() async { environment.defines[kBuildMode] = 'profile'; environment.defines[kCspMode] = 'true'; - processManager.addCommand(FakeCommand( - command: [ - ...kDart2jsLinuxArgs, - '-o', - environment.buildDir.childFile('app.dill').absolute.path, - '--packages=${globals.fs.path.join('foo', '.packages')}', - '-Ddart.vm.profile=true', - '--cfe-only', - environment.buildDir.childFile('main.dart').absolute.path, - ] - )); - processManager.addCommand(FakeCommand( - command: [ - ...kDart2jsLinuxArgs, - '-O4', - '--no-minify', - '--csp', - '-o', - environment.buildDir.childFile('main.dart.js').absolute.path, - environment.buildDir.childFile('app.dill').absolute.path, - ] - )); - + when(globals.processManager.run(any)).thenAnswer((Invocation invocation) async { + return FakeProcessResult(exitCode: 0); + }); await const Dart2JSTarget().build(environment); + + final List expected = [ + globals.fs.path.join('bin', 'cache', 'dart-sdk', 'bin', 'dart'), + globals.fs.path.join('bin', 'cache', 'dart-sdk', 'bin', 'snapshots', 'dart2js.dart.snapshot'), + '--libraries-spec=' + globals.fs.path.join('bin', 'cache', 'flutter_web_sdk', 'libraries.json'), + '-O4', // highest optimizations + '--no-minify', // but uses unminified names for debugging + '-o', + environment.buildDir.childFile('main.dart.js').absolute.path, + '--packages=${globals.fs.path.join('foo', '.packages')}', + '-Ddart.vm.profile=true', + '--csp', + environment.buildDir.childFile('main.dart').absolute.path, + ]; + verify(globals.processManager.run(expected)).called(1); }, overrides: { - ProcessManager: () => processManager, + ProcessManager: () => MockProcessManager(), })); test('Dart2JSTarget calls dart2js with expected args in profile mode', () => testbed.run(() async { environment.defines[kBuildMode] = 'profile'; - processManager.addCommand(FakeCommand( - command: [ - ...kDart2jsLinuxArgs, - '-o', - environment.buildDir.childFile('app.dill').absolute.path, - '--packages=${globals.fs.path.join('foo', '.packages')}', - '-Ddart.vm.profile=true', - '--cfe-only', - environment.buildDir.childFile('main.dart').absolute.path, - ] - )); - processManager.addCommand(FakeCommand( - command: [ - ...kDart2jsLinuxArgs, - '-O4', - '--no-minify', - '-o', - environment.buildDir.childFile('main.dart.js').absolute.path, - environment.buildDir.childFile('app.dill').absolute.path, - ] - )); - + when(globals.processManager.run(any)).thenAnswer((Invocation invocation) async { + return FakeProcessResult(exitCode: 0); + }); await const Dart2JSTarget().build(environment); + + final List expected = [ + globals.fs.path.join('bin', 'cache', 'dart-sdk', 'bin', 'dart'), + globals.fs.path.join('bin', 'cache', 'dart-sdk', 'bin', 'snapshots', 'dart2js.dart.snapshot'), + '--libraries-spec=' + globals.fs.path.join('bin', 'cache', 'flutter_web_sdk', 'libraries.json'), + '-O4', // highest optimizations + '--no-minify', // but uses unminified names for debugging + '-o', + environment.buildDir.childFile('main.dart.js').absolute.path, + '--packages=${globals.fs.path.join('foo', '.packages')}', + '-Ddart.vm.profile=true', + environment.buildDir.childFile('main.dart').absolute.path, + ]; + verify(globals.processManager.run(expected)).called(1); }, overrides: { - ProcessManager: () => processManager, + ProcessManager: () => MockProcessManager(), })); test('Dart2JSTarget calls dart2js with expected args in release mode', () => testbed.run(() async { environment.defines[kBuildMode] = 'release'; - processManager.addCommand(FakeCommand( - command: [ - ...kDart2jsLinuxArgs, - '-o', - environment.buildDir.childFile('app.dill').absolute.path, - '--packages=${globals.fs.path.join('foo', '.packages')}', - '-Ddart.vm.product=true', - '--cfe-only', - environment.buildDir.childFile('main.dart').absolute.path, - ] - )); - processManager.addCommand(FakeCommand( - command: [ - ...kDart2jsLinuxArgs, - '-O4', - '-o', - environment.buildDir.childFile('main.dart.js').absolute.path, - environment.buildDir.childFile('app.dill').absolute.path, - ] - )); - + when(globals.processManager.run(any)).thenAnswer((Invocation invocation) async { + return FakeProcessResult(exitCode: 0); + }); await const Dart2JSTarget().build(environment); + + final List expected = [ + globals.fs.path.join('bin', 'cache', 'dart-sdk', 'bin', 'dart'), + globals.fs.path.join('bin', 'cache', 'dart-sdk', 'bin', 'snapshots', 'dart2js.dart.snapshot'), + '--libraries-spec=' + globals.fs.path.join('bin', 'cache', 'flutter_web_sdk', 'libraries.json'), + '-O4', // highest optimizations. + '-o', + environment.buildDir.childFile('main.dart.js').absolute.path, + '--packages=${globals.fs.path.join('foo', '.packages')}', + '-Ddart.vm.product=true', + environment.buildDir.childFile('main.dart').absolute.path, + ]; + verify(globals.processManager.run(expected)).called(1); }, overrides: { - ProcessManager: () => processManager, + ProcessManager: () => MockProcessManager(), })); test('Dart2JSTarget calls dart2js with expected args in release with dart2js optimization override', () => testbed.run(() async { environment.defines[kBuildMode] = 'release'; environment.defines[kDart2jsOptimization] = 'O3'; - processManager.addCommand(FakeCommand( - command: [ - ...kDart2jsLinuxArgs, - '-o', - environment.buildDir.childFile('app.dill').absolute.path, - '--packages=${globals.fs.path.join('foo', '.packages')}', - '-Ddart.vm.product=true', - '--cfe-only', - environment.buildDir.childFile('main.dart').absolute.path, - ] - )); - processManager.addCommand(FakeCommand( - command: [ - ...kDart2jsLinuxArgs, - '-O3', - '-o', - environment.buildDir.childFile('main.dart.js').absolute.path, - environment.buildDir.childFile('app.dill').absolute.path, - ] - )); - + when(globals.processManager.run(any)).thenAnswer((Invocation invocation) async { + return FakeProcessResult(exitCode: 0); + }); await const Dart2JSTarget().build(environment); + + final List expected = [ + globals.fs.path.join('bin', 'cache', 'dart-sdk', 'bin', 'dart'), + globals.fs.path.join('bin', 'cache', 'dart-sdk', 'bin', 'snapshots', 'dart2js.dart.snapshot'), + '--libraries-spec=' + globals.fs.path.join('bin', 'cache', 'flutter_web_sdk', 'libraries.json'), + '-O3', // configured optimizations. + '-o', + environment.buildDir.childFile('main.dart.js').absolute.path, + '--packages=${globals.fs.path.join('foo', '.packages')}', + '-Ddart.vm.product=true', + environment.buildDir.childFile('main.dart').absolute.path, + ]; + verify(globals.processManager.run(expected)).called(1); }, overrides: { - ProcessManager: () => processManager, + ProcessManager: () => MockProcessManager(), })); test('Dart2JSTarget produces expected depfile', () => testbed.run(() async { environment.defines[kBuildMode] = 'release'; when(globals.processManager.run(any)).thenAnswer((Invocation invocation) async { - environment.buildDir.childFile('app.dill.deps') + environment.buildDir.childFile('main.dart.js.deps') .writeAsStringSync('file:///a.dart'); return FakeProcessResult(exitCode: 0); }); await const Dart2JSTarget().build(environment); - expect(environment.buildDir.childFile('dart2js.d'), exists); + expect(environment.buildDir.childFile('dart2js.d').existsSync(), true); final Depfile depfile = depfileService.parse(environment.buildDir.childFile('dart2js.d')); expect(depfile.inputs.single.path, globals.fs.path.absolute('a.dart')); @@ -356,64 +333,54 @@ void main() { test('Dart2JSTarget calls dart2js with Dart defines in release mode', () => testbed.run(() async { environment.defines[kBuildMode] = 'release'; environment.defines[kDartDefines] = '["FOO=bar","BAZ=qux"]'; - processManager.addCommand(FakeCommand( - command: [ - ...kDart2jsLinuxArgs, - '-o', - environment.buildDir.childFile('app.dill').absolute.path, - '--packages=${globals.fs.path.join('foo', '.packages')}', - '-Ddart.vm.product=true', - '-DFOO=bar', - '-DBAZ=qux', - '--cfe-only', - environment.buildDir.childFile('main.dart').absolute.path, - ] - )); - processManager.addCommand(FakeCommand( - command: [ - ...kDart2jsLinuxArgs, - '-O4', - '-o', - environment.buildDir.childFile('main.dart.js').absolute.path, - environment.buildDir.childFile('app.dill').absolute.path, - ] - )); - + when(globals.processManager.run(any)).thenAnswer((Invocation invocation) async { + return FakeProcessResult(exitCode: 0); + }); await const Dart2JSTarget().build(environment); + + final List expected = [ + globals.fs.path.join('bin', 'cache', 'dart-sdk', 'bin', 'dart'), + globals.fs.path.join('bin', 'cache', 'dart-sdk', 'bin', 'snapshots', 'dart2js.dart.snapshot'), + '--libraries-spec=' + globals.fs.path.join('bin', 'cache', 'flutter_web_sdk', 'libraries.json'), + '-O4', + '-o', + environment.buildDir.childFile('main.dart.js').absolute.path, + '--packages=${globals.fs.path.join('foo', '.packages')}', + '-Ddart.vm.product=true', + '-DFOO=bar', + '-DBAZ=qux', + environment.buildDir.childFile('main.dart').absolute.path, + ]; + verify(globals.processManager.run(expected)).called(1); }, overrides: { - ProcessManager: () => processManager, + ProcessManager: () => MockProcessManager(), })); test('Dart2JSTarget calls dart2js with Dart defines in profile mode', () => testbed.run(() async { environment.defines[kBuildMode] = 'profile'; environment.defines[kDartDefines] = '["FOO=bar","BAZ=qux"]'; - processManager.addCommand(FakeCommand( - command: [ - ...kDart2jsLinuxArgs, - '-o', - environment.buildDir.childFile('app.dill').absolute.path, - '--packages=${globals.fs.path.join('foo', '.packages')}', - '-Ddart.vm.profile=true', - '-DFOO=bar', - '-DBAZ=qux', - '--cfe-only', - environment.buildDir.childFile('main.dart').absolute.path, - ] - )); - processManager.addCommand(FakeCommand( - command: [ - ...kDart2jsLinuxArgs, - '-O4', - '--no-minify', - '-o', - environment.buildDir.childFile('main.dart.js').absolute.path, - environment.buildDir.childFile('app.dill').absolute.path, - ] - )); - + when(globals.processManager.run(any)).thenAnswer((Invocation invocation) async { + return FakeProcessResult(exitCode: 0); + }); await const Dart2JSTarget().build(environment); + + final List expected = [ + globals.fs.path.join('bin', 'cache', 'dart-sdk', 'bin', 'dart'), + globals.fs.path.join('bin', 'cache', 'dart-sdk', 'bin', 'snapshots', 'dart2js.dart.snapshot'), + '--libraries-spec=' + globals.fs.path.join('bin', 'cache', 'flutter_web_sdk', 'libraries.json'), + '-O4', + '--no-minify', + '-o', + environment.buildDir.childFile('main.dart.js').absolute.path, + '--packages=${globals.fs.path.join('foo', '.packages')}', + '-Ddart.vm.profile=true', + '-DFOO=bar', + '-DBAZ=qux', + environment.buildDir.childFile('main.dart').absolute.path, + ]; + verify(globals.processManager.run(expected)).called(1); }, overrides: { - ProcessManager: () => processManager, + ProcessManager: () => MockProcessManager(), })); test('Dart2JSTarget throws developer-friendly exception on misformatted DartDefines', () => testbed.run(() async { @@ -460,3 +427,4 @@ void main() { } class MockProcessManager extends Mock implements ProcessManager {} +class MockPlatform extends Mock implements Platform {}