From 64fd68ed422eb3053ce20df64db64a28dcec7460 Mon Sep 17 00:00:00 2001 From: stuartmorgan Date: Tue, 5 Oct 2021 15:27:53 -0400 Subject: [PATCH] Don't generate plugin registry in ResidentWebRunner (#91281) * Don't generate plugin registry in ResidentWebRunner generateDartPluginRegistry was being set to true unconditionally in ResidentRunner, bypassing the primary check in DartPluginRegistrantTarget, and the targetPlatform was not set in that codepath, bypassing the second after the changes in https://github.com/flutter/flutter/pull/87991. This caused web hot restarts to be slower due to doing unnecessary work. This ensures that generateDartPluginRegistry is false in the ResidentWebRunner to skip that unnecessary step. Fixes https://github.com/flutter/flutter/issues/91262 * Formatting Co-authored-by: Zachary Anderson Co-authored-by: Zachary Anderson --- .../targets/dart_plugin_registrant.dart | 2 + .../lib/src/isolated/resident_web_runner.dart | 4 ++ .../lib/src/resident_runner.dart | 6 ++- .../resident_web_runner_test.dart | 41 +++++++++++++++++++ 4 files changed, 52 insertions(+), 1 deletion(-) diff --git a/packages/flutter_tools/lib/src/build_system/targets/dart_plugin_registrant.dart b/packages/flutter_tools/lib/src/build_system/targets/dart_plugin_registrant.dart index 497b4c0303a..d5f2fb0e869 100644 --- a/packages/flutter_tools/lib/src/build_system/targets/dart_plugin_registrant.dart +++ b/packages/flutter_tools/lib/src/build_system/targets/dart_plugin_registrant.dart @@ -66,6 +66,8 @@ class DartPluginRegistrantTarget extends Target { // TODO(stuartmorgan): Investigate removing this check entirely; ideally the // source generation step shouldn't be platform dependent, and the generated // code should just do the right thing on every platform. + // Failing that, consider throwing if `targetPlatform` isn't set and finding + // all violations, as it's not consistently set here. return targetPlatform == TargetPlatform.fuchsia_arm64 || targetPlatform == TargetPlatform.fuchsia_x64 || targetPlatform == TargetPlatform.web_javascript; diff --git a/packages/flutter_tools/lib/src/isolated/resident_web_runner.dart b/packages/flutter_tools/lib/src/isolated/resident_web_runner.dart index c80a58d2a8d..41d5f6266d9 100644 --- a/packages/flutter_tools/lib/src/isolated/resident_web_runner.dart +++ b/packages/flutter_tools/lib/src/isolated/resident_web_runner.dart @@ -145,6 +145,10 @@ class ResidentWebRunner extends ResidentRunner { @override bool get supportsWriteSkSL => false; + @override + // Web uses a different plugin registry. + bool get generateDartPluginRegistry => false; + bool get _enableDwds => debuggingEnabled; ConnectionResult _connectionResult; diff --git a/packages/flutter_tools/lib/src/resident_runner.dart b/packages/flutter_tools/lib/src/resident_runner.dart index bbe3ecc0994..e59cd5527db 100644 --- a/packages/flutter_tools/lib/src/resident_runner.dart +++ b/packages/flutter_tools/lib/src/resident_runner.dart @@ -1101,6 +1101,10 @@ abstract class ResidentRunner extends ResidentHandlers { bool get trackWidgetCreation => debuggingOptions.buildInfo.trackWidgetCreation; + /// True if the shared Dart plugin registry (which is different than the one + /// used for web) should be generated during source generation. + bool get generateDartPluginRegistry => true; + // Returns the Uri of the first connected device for mobile, // and only connected device for web. // @@ -1152,7 +1156,7 @@ abstract class ResidentRunner extends ResidentHandlers { processManager: globals.processManager, platform: globals.platform, projectDir: globals.fs.currentDirectory, - generateDartPluginRegistry: true, + generateDartPluginRegistry: generateDartPluginRegistry, ); final CompositeTarget compositeTarget = CompositeTarget([ diff --git a/packages/flutter_tools/test/general.shard/resident_web_runner_test.dart b/packages/flutter_tools/test/general.shard/resident_web_runner_test.dart index 483ea0ae18b..ddb81efa4e7 100644 --- a/packages/flutter_tools/test/general.shard/resident_web_runner_test.dart +++ b/packages/flutter_tools/test/general.shard/resident_web_runner_test.dart @@ -941,6 +941,47 @@ void main() { ProcessManager: () => processManager, }); + // While this file should be ignored on web, generating it here will cause a + // perf regression in hot restart. + testUsingContext('Does not generate generated_main.dart', () async { + // Create necessary files for [DartPluginRegistrantTarget] + final File packageConfig = globals.fs.directory('.dart_tool') + .childFile('package_config.json'); + packageConfig.createSync(recursive: true); + packageConfig.writeAsStringSync(''' +{ + "configVersion": 2, + "packages": [ + { + "name": "path_provider_linux", + "rootUri": "../../../path_provider_linux", + "packageUri": "lib/", + "languageVersion": "2.12" + } + ] +} +'''); + // Start with a generated_main.dart file. + globals.fs.directory('.dart_tool') + .childDirectory('flutter_build') + .childFile('generated_main.dart') + .createSync(recursive: true); + + final FlutterProject project = FlutterProject.fromDirectoryTest(fileSystem.currentDirectory); + + final ResidentRunner residentWebRunner = setUpResidentRunner(flutterDevice); + await residentWebRunner.runSourceGenerators(); + + // generated_main.dart should be untouched, indicating that its + // generation didn't run. If it had run, the file would have been removed as + // there are no plugins in the project. + expect(project.dartPluginRegistrant.existsSync(), true); + expect(project.dartPluginRegistrant.readAsStringSync(), ''); + }, overrides: { + FileSystem: () => fileSystem, + ProcessManager: () => processManager, + }); + testUsingContext('Successfully turns WebSocketException into ToolExit', () async { final BufferLogger logger = BufferLogger.test(); final ResidentRunner residentWebRunner = setUpResidentRunner(flutterDevice, logger: logger);