From cd785544fa73c12a86ca82dd4576294b7d0bcea9 Mon Sep 17 00:00:00 2001 From: Andrew Kolos Date: Sun, 24 Mar 2024 21:13:11 -0700 Subject: [PATCH] make hot reload reflect changes to asset transformer configurations (#144660) In service of https://github.com/flutter/flutter/issues/143348 This PR makes hot reloads reflect changes to transformer configurations under the `assets` section in pubspec.yaml. This PR is optimized for ease of implementation, and should not be merged as-is. If it is merged as-is, seriously consider creating a tech debt issue and assigning it to someone to make sure it gets resolved. --- packages/flutter_tools/lib/src/asset.dart | 23 +++++-- .../test/general.shard/asset_bundle_test.dart | 67 +++++++++++++++++++ 2 files changed, 84 insertions(+), 6 deletions(-) diff --git a/packages/flutter_tools/lib/src/asset.dart b/packages/flutter_tools/lib/src/asset.dart index 0bc661a5b87..5fa7e7d0e01 100644 --- a/packages/flutter_tools/lib/src/asset.dart +++ b/packages/flutter_tools/lib/src/asset.dart @@ -14,6 +14,7 @@ import 'base/deferred_component.dart'; import 'base/file_system.dart'; import 'base/logger.dart'; import 'base/platform.dart'; +import 'base/utils.dart'; import 'build_info.dart'; import 'cache.dart'; import 'convert.dart'; @@ -84,9 +85,8 @@ enum AssetKind { /// Contains all information about an asset needed by tool the to prepare and /// copy an asset file to the build output. -@immutable final class AssetBundleEntry { - const AssetBundleEntry(this.content, { + AssetBundleEntry(this.content, { required this.kind, required this.transformers, }); @@ -96,6 +96,10 @@ final class AssetBundleEntry { final List transformers; Future> contentsAsBytes() => content.contentsAsBytes(); + + bool hasEquivalentConfigurationWith(AssetBundleEntry other) { + return listEquals(transformers, other.transformers); + } } abstract class AssetBundle { @@ -425,11 +429,11 @@ class ManifestAssetBundle implements AssetBundle { final File variantFile = variant.lookupAssetFile(_fileSystem); inputFiles.add(variantFile); assert(variantFile.existsSync()); - entries[variant.entryUri.path] ??= AssetBundleEntry( + _setIfConfigurationChanged(entries, variant.entryUri.path, AssetBundleEntry( DevFSFileContent(variantFile), kind: variant.kind, transformers: variant.transformers, - ); + )); } } // Save the contents of each deferred component image, image variant, and font @@ -459,11 +463,11 @@ class ManifestAssetBundle implements AssetBundle { for (final _Asset variant in assetsMap[asset]!) { final File variantFile = variant.lookupAssetFile(_fileSystem); assert(variantFile.existsSync()); - deferredComponentsEntries[componentName]![variant.entryUri.path] ??= AssetBundleEntry( + _setIfConfigurationChanged(deferredComponentsEntries[componentName]!, variant.entryUri.path, AssetBundleEntry( DevFSFileContent(variantFile), kind: AssetKind.regular, transformers: variant.transformers, - ); + )); } } } @@ -563,6 +567,13 @@ class ManifestAssetBundle implements AssetBundle { return true; } + void _setIfConfigurationChanged(Map entryMap, String key, AssetBundleEntry entry,) { + final AssetBundleEntry? existingEntry = entryMap[key]; + if (existingEntry == null || !entry.hasEquivalentConfigurationWith(existingEntry)) { + entryMap[key] = entry; + } + } + void _setLicenseIfChanged( String combinedLicenses, TargetPlatform? targetPlatform, diff --git a/packages/flutter_tools/test/general.shard/asset_bundle_test.dart b/packages/flutter_tools/test/general.shard/asset_bundle_test.dart index e3114aa511d..d0a015697b3 100644 --- a/packages/flutter_tools/test/general.shard/asset_bundle_test.dart +++ b/packages/flutter_tools/test/general.shard/asset_bundle_test.dart @@ -428,6 +428,73 @@ flutter: ), ); }); + + testWithoutContext("AssetBundleEntry::content::isModified is true when an asset's transformers change in between builds", () async { + final FileSystem fileSystem = MemoryFileSystem.test(); + + fileSystem.file('my-asset.txt').createSync(); + + final BufferLogger logger = BufferLogger.test(); + final FakePlatform platform = FakePlatform(); + fileSystem.file('.packages').createSync(); + fileSystem.file('pubspec.yaml') + ..createSync() + ..writeAsStringSync(r''' +name: example +flutter: + assets: + - path: my-asset.txt + transformers: + - package: my-transformer-one +'''); + final ManifestAssetBundle bundle = ManifestAssetBundle( + logger: logger, + fileSystem: fileSystem, + platform: platform, + flutterRoot: Cache.defaultFlutterRoot( + platform: platform, + fileSystem: fileSystem, + userMessages: UserMessages(), + ), + ); + + await bundle.build( + packagesPath: '.packages', + flutterProject: FlutterProject.fromDirectoryTest( + fileSystem.currentDirectory, + ), + ); + + expect(bundle.entries['my-asset.txt']!.content.isModified, isTrue); + + await bundle.build( + packagesPath: '.packages', + flutterProject: FlutterProject.fromDirectoryTest( + fileSystem.currentDirectory, + ), + ); + + expect(bundle.entries['my-asset.txt']!.content.isModified, isFalse); + + fileSystem.file('pubspec.yaml').writeAsStringSync(r''' +name: example +flutter: + assets: + - path: my-asset.txt + transformers: + - package: my-transformer-one + - package: my-transformer-two +'''); + + await bundle.build( + packagesPath: '.packages', + flutterProject: FlutterProject.fromDirectoryTest( + fileSystem.currentDirectory, + ), + ); + + expect(bundle.entries['my-asset.txt']!.content.isModified, isTrue); + }); }); group('AssetBundle.build (web builds)', () {