Skip to content

Fix hooks inputs outputs rebuilt #186701

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
auto-submit merged 15 commits into from
May 22, 2026
Merged

Fix hooks inputs outputs rebuilt #186701

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
fc54789
[hooks] report build hook files to be bundled as outputs
dcharkes May 12, 2026
9d5de55
bump deps
dcharkes May 18, 2026
91bfdc5
link hook caching logic fix
dcharkes May 19, 2026
b3c1566
Merge remote-tracking branch 'origin/master' into fix-hooks-inputs-ou…
dcharkes May 19, 2026
de55565
[hooks] Add build dependencies to link dependencies when linking is d…
dcharkes May 19, 2026
28dff0e
another version bump
dcharkes May 19, 2026
21e0207
add comments about dependency tracking for build hook dependencies
dcharkes May 19, 2026
67c7fc6
try removing BuildHooks DEPS from LinkHooks
dcharkes May 19, 2026
2f51a43
cleanup
dcharkes May 19, 2026
6c5d0e9
Trigger Build
dcharkes May 19, 2026
22fd24d
address comments
dcharkes May 20, 2026
077878f
Merge remote-tracking branch 'origin/master' into fix-hooks-inputs-ou…
dcharkes May 20, 2026
08cd19a
fix tests
dcharkes May 20, 2026
d822048
Merge branch 'master' into fix-hooks-inputs-outputs-rebuilt
dcharkes May 22, 2026
b5309dc
address comments
dcharkes May 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions dev/integration_tests/data_asset_app/hook/build.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ void main(List<String> args) async {
file: input.packageRoot.resolve('data/$id'),
),
);
output.dependencies.add(input.packageRoot.resolve('data/$id'));
}
}
});
Expand Down
6 changes: 3 additions & 3 deletions dev/integration_tests/data_asset_app/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ resolution: workspace
dependencies:
flutter:
sdk: flutter
hooks: ^1.0.2
data_assets: ^0.19.6
hooks: ^2.0.0
data_assets: ^0.20.0
data_asset_package:
path: ../data_asset_package

Expand All @@ -24,4 +24,4 @@ dev_dependencies:
flutter:
uses-material-design: true

# PUBSPEC CHECKSUM: jl85h
# PUBSPEC CHECKSUM: 9jmf6j
3 changes: 3 additions & 0 deletions dev/integration_tests/data_asset_package/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,6 @@ migrate_working_dir/
.flutter-plugins-dependencies
/build/
/coverage/

# Generated test file.
data/generated.txt
23 changes: 23 additions & 0 deletions dev/integration_tests/data_asset_package/hook/build.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:io';

import 'package:data_assets/data_assets.dart';
import 'package:hooks/hooks.dart';

Expand All @@ -17,7 +19,28 @@ void main(List<String> args) async {
file: input.packageRoot.resolve('data/$id'),
),
);
// If the file is modified, the hook needs to be rerun. (Technically, we
// don't need to rerun because we'd output the exact same thing. But the
// hook could be doing other things based on the input file.)
output.dependencies.add(input.packageRoot.resolve('data/$id'));
}

// Generate a data asset in the hook.
// It is better to generate to outputDirectoryShared, but users might do
// this instead and then delete the file manually.
final Uri generatedUri = input.packageRoot.resolve('data/generated.txt');
final file = File(generatedUri.toFilePath());
if (!file.parent.existsSync()) {
file.parent.createSync(recursive: true);
}
file.writeAsStringSync('generated content');
output.assets.data.add(
DataAsset(
package: input.packageName,
name: 'data/generated.txt',
file: generatedUri,
),
);
}
});
}
6 changes: 3 additions & 3 deletions dev/integration_tests/data_asset_package/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ resolution: workspace
dependencies:
flutter:
sdk: flutter
hooks: ^1.0.2
data_assets: ^0.19.6
hooks: ^2.0.0
data_assets: ^0.20.0

dev_dependencies:
flutter_test:
Expand All @@ -22,4 +22,4 @@ dev_dependencies:
flutter:
uses-material-design: true

# PUBSPEC CHECKSUM: 2g5j7i
# PUBSPEC CHECKSUM: 6cfeb0
6 changes: 3 additions & 3 deletions dev/integration_tests/hook_user_defines/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@ hooks:
magic_value: 1000

dependencies:
hooks: 1.0.3
hooks: 2.0.0
logging: 1.3.0
native_toolchain_c: 0.17.6
native_toolchain_c: 0.19.0

dev_dependencies:
test: 1.29.0

# PUBSPEC CHECKSUM: 30ohqf
# PUBSPEC CHECKSUM: jmsrr2
6 changes: 3 additions & 3 deletions dev/integration_tests/record_use_test_package/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ environment:
dependencies:
flutter:
sdk: flutter
hooks: ^1.0.2
data_assets: ^0.19.6
hooks: ^2.0.0
data_assets: ^0.20.0
record_use: 0.6.0
meta: 1.18.2

Expand All @@ -23,4 +23,4 @@ dev_dependencies:
flutter:
uses-material-design: true

# PUBSPEC CHECKSUM: 4e6oin
# PUBSPEC CHECKSUM: gl7kb7
5 changes: 2 additions & 3 deletions packages/flutter_tools/lib/src/build_system/targets/hook_runner_native.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,18 +37,17 @@ class FlutterHookRunnerNative implements FlutterHookRunner {
environment,
);

final DartHooksResult dartHooksResult = await runFlutterSpecificHooks(
final (:DartHooksResult buildResult, results: _) = await runFlutterSpecificBuildHooks(
Copy link
Copy Markdown
Contributor

@bkonyi bkonyi May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need to bind results at all here.

Copy link
Copy Markdown
Contributor Author

@dcharkes dcharkes May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think records destructuring has a ... rest syntax. I tried a bunch of different syntaxes but the analyzer stays unhappy. Would you enlighten me what the right syntax is?

Copy link
Copy Markdown
Contributor

@bkonyi bkonyi May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, disregard. Gemini lied to me when I asked if final (:DartHooksResult buildResult) = await runFlutterSpecificBuildHooks(...) was valid 🤦

environmentDefines: environment.defines,
buildRunner: buildRunner,
targetPlatform: targetPlatform,
projectUri: environment.projectDir.uri,
fileSystem: environment.fileSystem,
buildCodeAssets: null,
buildDataAssets: true,
recordedUsesFile: null,
);

final FlutterHookResult flutterHookResult = dartHooksResult.asFlutterResult;
final FlutterHookResult flutterHookResult = buildResult.asFlutterResult;
_flutterHookResult = flutterHookResult;
logger?.printTrace('runHooks() - done');
return flutterHookResult;
Expand Down
69 changes: 51 additions & 18 deletions packages/flutter_tools/lib/src/build_system/targets/native_assets.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ class BuildHooks extends Target {
final FlutterNativeAssetsBuildRunner buildRunner =
_buildRunner ?? await createFlutterNativeAssetsBuildRunner(environment);
final (
results: SerializedBuildResults results,
dependencies: List<Uri> dependencies,
:SerializedBuildResults results,
:DartHooksResult buildResult,
) = await runFlutterSpecificBuildHooks(
environmentDefines: environment.defines,
buildRunner: buildRunner,
Expand All @@ -74,11 +74,20 @@ class BuildHooks extends Target {
dartBuildOutputJsonFile.parent.createSync(recursive: true);
}

dartBuildOutputJsonFile.writeAsStringSync(json.encode(results));
final String encodedResults = json.encode(results);
if (!dartBuildOutputJsonFile.existsSync() ||
dartBuildOutputJsonFile.readAsStringSync() != encodedResults) {
dartBuildOutputJsonFile.writeAsStringSync(encodedResults);
}

final Set<Uri> buildDependencies = buildResult.dependencies.toSet();
final depfile = Depfile(
<File>[for (final Uri dependency in dependencies) fileSystem.file(dependency)],
<File>[fileSystem.file(dartBuildOutputJsonFile)],
<File>[for (final Uri dependency in buildResult.dependencies) fileSystem.file(dependency)],
<File>[
fileSystem.file(dartBuildOutputJsonFile),
for (final Uri uri in buildResult.filesToBeBundled)
if (!buildDependencies.contains(uri)) fileSystem.file(uri),
],
);
Comment on lines 84 to 91
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The contains check on buildResult.dependencies (which is a List) inside the loop over buildResult.filesToBeBundled results in O(N*M) complexity. Converting dependencies to a Set before the loop improves performance, especially for projects with many assets and dependencies.

    final Set<Uri> buildDependencies = buildResult.dependencies.toSet();
    final depfile = Depfile(
      <File>[for (final Uri dependency in buildResult.dependencies) fileSystem.file(dependency)],
      <File>[
        fileSystem.file(dartBuildOutputJsonFile),
        for (final Uri uri in buildResult.filesToBeBundled)
          if (!buildDependencies.contains(uri)) fileSystem.file(uri),
      ],
    );

Comment thread
dcharkes marked this conversation as resolved.
final File outputDepfile = environment.buildDir.childFile(depFilename);
if (!outputDepfile.parent.existsSync()) {
Expand All @@ -98,7 +107,6 @@ class BuildHooks extends Target {
// If different packages are resolved, different native assets might need to
// be built.
Source.pattern('{WORKSPACE_DIR}/.dart_tool/package_config.json'),
// TODO(mosuem): Should consume resources.json. https://github.com/flutter/flutter/issues/146263
];

@override
Expand Down Expand Up @@ -197,6 +205,8 @@ class LinkHooks extends Target {
final buildMode = BuildMode.fromCliName(buildModeEnvironment);
final File? recordedUsesFileToPass = getRecordedUsesFile(environment, buildMode);

final linkingEnabled = buildMode != BuildMode.debug;

// Read the result of BuildHooks.
final File dartBuildOutputJsonFile = environment.buildDir.childFile(BuildHooks.resultFilename);
if (!dartBuildOutputJsonFile.existsSync()) {
Expand All @@ -207,29 +217,52 @@ class LinkHooks extends Target {
final Map<String, Map<String, Object?>> buildResults = serializedBuildResults
.cast<String, Map<String, Object?>>();

final DartHooksResult result = await runFlutterSpecificLinkHooks(
final DartHooksResult linkResult;
if (linkingEnabled) {
linkResult = await runFlutterSpecificLinkHooks(
environmentDefines: environment.defines,
buildRunner: buildRunner,
targetPlatform: targetPlatform,
projectUri: projectUri,
fileSystem: fileSystem,
buildCodeAssets: BuildCodeAssetsOptions(appBuildDirectory: environment.outputDir),
buildDataAssets: true,
buildResults: buildResults,
recordedUsesFile: recordedUsesFileToPass,
);
} else {
linkResult = DartHooksResult.empty();
}

final DartHooksResult combinedResult = combineBuildAndLinkResults(
environmentDefines: environment.defines,
buildRunner: buildRunner,
targetPlatform: targetPlatform,
projectUri: projectUri,
fileSystem: fileSystem,
buildCodeAssets: BuildCodeAssetsOptions(appBuildDirectory: environment.outputDir),
buildDataAssets: true,
buildResults: buildResults,
recordedUsesFile: recordedUsesFileToPass,
linkResult: linkResult,
);

final File dartHookResultJsonFile = environment.buildDir.childFile(resultFilename);
if (!dartHookResultJsonFile.parent.existsSync()) {
dartHookResultJsonFile.parent.createSync(recursive: true);
}
dartHookResultJsonFile.writeAsStringSync(json.encode(result.toJson()));

// TODO(dcharkes): The build system uses file hashing to determine if
// targets need to be rerun. Because combinedResult.toJson() includes
// transient build_start and build_end times, this file is rewritten on
// every build causing downstream targets to rerun. We should remove
// build_start and build_end from the JSON representation entirely in a
// future PR.
dartHookResultJsonFile.writeAsStringSync(json.encode(combinedResult.toJson()));
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To optimize incremental builds, the output JSON file should only be written if its content has changed. This prevents updating the file's modification timestamp unnecessarily, which avoids triggering downstream targets (like InstallCodeAssets) when the result is identical. This matches the optimization already applied to BuildHooks.

    final String encodedResult = json.encode(combinedResult.toJson());
    if (!dartHookResultJsonFile.existsSync() ||
        dartHookResultJsonFile.readAsStringSync() != encodedResult) {
      dartHookResultJsonFile.writeAsStringSync(encodedResult);
    }

Comment thread
dcharkes marked this conversation as resolved.
final Set<Uri> linkDependencies = linkResult.dependencies.toSet();
final depfile = Depfile(
<File>[for (final Uri dependency in result.dependencies) fileSystem.file(dependency)],
<File>[for (final Uri dependency in linkResult.dependencies) fileSystem.file(dependency)],
<File>[
fileSystem.file(dartHookResultJsonFile),
for (final Uri uri in result.filesToBeBundled) fileSystem.file(uri),
if (linkingEnabled)
for (final Uri uri in linkResult.filesToBeBundled)
if (!linkDependencies.contains(uri)) fileSystem.file(uri),
],
);
Comment on lines 259 to 267
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Same O(N*M) complexity issue as in BuildHooks. Using a Set for the contains check is more efficient.

    final Set<Uri> linkDependencies = linkResult.dependencies.toSet();
    final depfile = Depfile(
      <File>[for (final Uri dependency in linkResult.dependencies) fileSystem.file(dependency)],
      <File>[
        fileSystem.file(dartHookResultJsonFile),
        if (linkingEnabled)
          for (final Uri uri in linkResult.filesToBeBundled)
            if (!linkDependencies.contains(uri)) fileSystem.file(uri),
      ],
    );

Comment thread
dcharkes marked this conversation as resolved.
final File outputDepfile = environment.buildDir.childFile(depFilename);
Expand Down Expand Up @@ -278,8 +311,8 @@ class InstallCodeAssets extends Target {
final FileSystem fileSystem = environment.fileSystem;
final TargetPlatform targetPlatform = _getTargetPlatformFromEnvironment(environment, name);

// We fetch the result from the [LinkHooks].
final DartHooksResult dartHookResult = await LinkHooks.loadHookResult(environment);
// We fetch the combined result from the [LinkHooks].
final DartHooksResult combinedResult = await LinkHooks.loadHookResult(environment);

// And install/copy the code assets to the right place and create a
// native_asset.yaml that can be used by the final AOT compilation.
Expand All @@ -293,7 +326,7 @@ class InstallCodeAssets extends Target {
}

final List<File> installedFiles = await installCodeAssets(
dartHookResult: dartHookResult,
dartHookResult: combinedResult,
environmentDefines: environment.defines,
targetPlatform: targetPlatform,
projectUri: projectUri,
Expand All @@ -304,7 +337,7 @@ class InstallCodeAssets extends Target {
assert(fileSystem.file(nativeAssetsFileUri).existsSync());

final depfile = Depfile(<File>[
for (final Uri file in dartHookResult.filesToBeBundled) fileSystem.file(file),
for (final Uri file in combinedResult.filesToBeBundled) fileSystem.file(file),
], installedFiles);
final File outputDepfile = environment.buildDir.childFile(depFilename);
environment.depFileService.writeToFile(depfile, outputDepfile);
Expand Down
Loading
Loading