Skip to content

Add "triggers" to quickly decide when to not run builders. #4084

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.

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
merged 3 commits into from
Aug 12, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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 build_config/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ triggers:

An `annotation` trigger causes the builder to run if an annotation is used.
So, `- annotation MyAnnotation` is a check for `@MyAnnotation` being used.
A part file included from a library is also checked for the annotation.

An `import` trigger says that the builder runs if there is a direct import
of the specified library. This might be useful if a builder can run on code
Expand Down
39 changes: 37 additions & 2 deletions build_runner_core/lib/src/generate/build.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
import 'dart:async';
import 'dart:convert';

import 'package:analyzer/dart/analysis/utilities.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:build/build.dart';
// ignore: implementation_imports
import 'package:build/src/internal.dart';
Expand Down Expand Up @@ -589,14 +591,47 @@ class Build {
return false;
}
final primaryInputSource = await readerWriter.readAsString(primaryInput);
final compilationUnit = _parseCompilationUnit(primaryInputSource);
List<CompilationUnit>? compilationUnits;
for (final trigger in buildTriggers) {
if (trigger.triggersOnPrimaryInput(primaryInputSource)) {
return true;
if (trigger.checksParts) {
compilationUnits ??= await _readAndParseCompilationUnits(
readerWriter,
primaryInput,
compilationUnit,
);
if (trigger.triggersOn(compilationUnits)) return true;
} else {
if (trigger.triggersOn([compilationUnit])) return true;
}
}
return false;
}

/// TODO(davidmorgan): cache parse results, share with deps parsing and
/// builder parsing.
static CompilationUnit _parseCompilationUnit(String content) {
return parseString(content: content, throwIfDiagnostics: false).unit;
}

static Future<List<CompilationUnit>> _readAndParseCompilationUnits(
AssetReader reader,
AssetId id,
CompilationUnit compilationUnit,
) async {
final result = [compilationUnit];
for (var directive in compilationUnit.directives) {
if (directive is! PartDirective) continue;
final partId = AssetId.resolve(
Uri.parse(directive.uri.stringValue!),
from: id,
);
if (!await reader.canRead(partId)) continue;
result.add(_parseCompilationUnit(await reader.readAsString(partId)));
}
return result;
}

Future<Iterable<AssetId>> _runPostBuildPhase(
int phaseNum,
PostBuildPhase phase,
Expand Down
65 changes: 49 additions & 16 deletions build_runner_core/lib/src/package_graph/build_triggers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import 'dart:convert';

import 'package:analyzer/dart/ast/ast.dart';
import 'package:build_config/build_config.dart';
import 'package:built_collection/built_collection.dart';
import 'package:built_value/built_value.dart';
Expand Down Expand Up @@ -80,7 +81,7 @@ class BuildTriggers {
/// Parses [triggers], or throws a descriptive exception on failure.
///
/// [triggers] is an `Object` read directly from yaml, to be valid it should
/// be a list of strings that parse with [BuildTrigger.tryParse].
/// be a list of strings that parse with [BuildTrigger._tryParse].
static (Iterable<BuildTrigger>, List<String>) parseList({
required Object triggers,
}) {
Expand All @@ -96,7 +97,7 @@ class BuildTriggers {
BuildTrigger? trigger;
String? warning;
if (triggerString is String) {
(trigger, warning) = BuildTrigger.tryParse(triggerString);
(trigger, warning) = BuildTrigger._tryParse(triggerString);
}
if (trigger != null) {
result.add(trigger);
Expand Down Expand Up @@ -124,13 +125,13 @@ class BuildTriggers {
}

/// A build trigger: a heuristic that possibly skips running a build step based
/// on the primary input source.
/// on the parsed primary input.
abstract class BuildTrigger {
/// Parses a trigger, returning `null` on failure, optionally with a warning
/// message.
///
/// The only supported trigger is [ImportBuildTrigger].
static (BuildTrigger?, String?) tryParse(String trigger) {
static (BuildTrigger?, String?) _tryParse(String trigger) {
if (trigger.startsWith('import ')) {
trigger = trigger.substring('import '.length);
final result = ImportBuildTrigger(trigger);
Expand All @@ -143,18 +144,21 @@ abstract class BuildTrigger {
return (null, 'Invalid trigger: `$trigger`');
}

bool triggersOnPrimaryInput(String source);
/// Whether the trigger matches on any of [compilationUnits].
///
/// This will be called either with the primary input compilation unit or all
/// compilation units (parts) depending on [checksParts].
bool triggersOn(List<CompilationUnit> compilationUnits);

/// Whether [triggersOn] should be called with all compilation units, not just
/// the primary input.
bool get checksParts;
}

// Note: `built_value` generated toString is relied on for digests, and
// equality for testing.

/// A build trigger that checks for a direct import.
///
/// TODO(davidmorgan): this currently over-matches, it just checks for a
/// matching substring. It's a heuristic so over matching is okay, but it might
/// be better to actually parse directives and do a correct match because
/// directives are needed when resolving source.
abstract class ImportBuildTrigger
implements
Built<ImportBuildTrigger, ImportBuildTriggerBuilder>,
Expand All @@ -166,18 +170,28 @@ abstract class ImportBuildTrigger
factory ImportBuildTrigger(String import) =>
_$ImportBuildTrigger._(import: import);

@memoized
String get packageImport => 'package:$import';

@override
bool triggersOnPrimaryInput(String source) {
return source.contains('package:$import');
bool get checksParts => false;

@override
bool triggersOn(List<CompilationUnit> compilationUnits) {
for (final compilationUnit in compilationUnits) {
for (final directive in compilationUnit.directives) {
if (directive is! ImportDirective) continue;
if (directive.uri.stringValue == packageImport) return true;
}
}
return false;
}

String? get warning =>
import.contains(_regexp) ? null : 'Invalid import trigger: `$import`';
}

/// A build trigger that checks for an annotation.
///
/// TODO(davidmorgan): like `ImportBuildTrigger` this over-matches.
abstract class AnnotationBuildTrigger
implements
Built<AnnotationBuildTrigger, AnnotationBuildTriggerBuilder>,
Expand All @@ -190,8 +204,27 @@ abstract class AnnotationBuildTrigger
_$AnnotationBuildTrigger._(annotation: annotation);

@override
bool triggersOnPrimaryInput(String source) {
return source.contains('@$annotation');
bool get checksParts => true;

@override
bool triggersOn(List<CompilationUnit> compilationUnits) {
for (final compilationUnit in compilationUnits) {
for (final declaration in compilationUnit.declarations) {
for (final metadata in declaration.metadata) {
var name = metadata.name.name;
if (metadata.constructorName != null) {
name = '$name.${metadata.constructorName}';
}
if (annotation == name) return true;
final periodIndex = name.indexOf('.');

Choose a reason for hiding this comment

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

Do we end up comparing to metadata.constructorName or could name (before line 216) have dots in it where we want to remove the first part? Could there be a comment explaining? (and either compare directly to metadata.constructorName or make sure we don't depending on what we're actually trying to achieve)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, added comment + a bit of test coverage on the awkward case. (Import prefixes and class names can't actually be distinguished).

if (periodIndex != -1) {
name = name.substring(periodIndex + 1);
if (annotation == name) return true;
}
}
}
}
return false;
}

String? get warning =>
Expand Down
10 changes: 10 additions & 0 deletions build_runner_core/lib/src/package_graph/build_triggers.g.dart

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ triggers:
- import trigger/trigger.dart
''');
tester.importGraph({
'a.1': ['package:trigger/trigger.dart'],
'a.1': ['package:trigger/trigger'],
});
});

Expand Down Expand Up @@ -122,7 +122,7 @@ triggers:
''');
await tester.build();
tester.importGraph({
'a.1': ['package:trigger/trigger.dart'],
'a.1': ['package:trigger/trigger'],
});
expect(await tester.build(change: 'a.1'), Result(written: ['a.2']));
});
Expand All @@ -138,7 +138,7 @@ targets:
run_only_if_triggered: true
''');
tester.importGraph({
'a.1': ['package:trigger/trigger.dart'],
'a.1': ['package:trigger/trigger'],
});
await tester.build();
expect(
Expand Down Expand Up @@ -230,7 +230,7 @@ triggers:
- import trigger/trigger.dart
''');
tester.importGraph({
'a.1': ['package:trigger/trigger.dart'],
'a.1': ['package:trigger/trigger'],
});
await tester.build();
tester.importGraph({'a.1': []});
Expand All @@ -252,7 +252,7 @@ triggers:
- import trigger/trigger.dart
''');
tester.importGraph({
'a.1': ['package:trigger/trigger.dart'],
'a.1': ['package:trigger/trigger'],
});
await tester.build();
expect(
Expand Down
Loading
Loading