[ Widget Preview ] Fix crash when `@Preview` annotations appeared outside of `lib/` (#180768)
The widget preview scaffold requires that previews are defined within
importable libraries. Since Dart does not support absolute paths for
imports or relative imports outside of the project, it's not possible
for the widget previewer to display previews defined outside of `lib/`.
This change updates the preview detector to ignore libraries that don't
have a package name (e.g., libraries under `test/`) and updates
`packageName` properties to not allow for null to make it clear that
package names are required for previews.
Fixes https://github.com/flutter/flutter/issues/178651
diff --git a/dev/integration_tests/widget_preview_scaffold/lib/src/widget_preview.dart b/dev/integration_tests/widget_preview_scaffold/lib/src/widget_preview.dart
index 29be79b..9f1e65d 100644
--- a/dev/integration_tests/widget_preview_scaffold/lib/src/widget_preview.dart
+++ b/dev/integration_tests/widget_preview_scaffold/lib/src/widget_preview.dart
@@ -62,7 +62,7 @@
///
/// For example, if a preview is defined in 'package:foo/src/bar.dart', this
/// will have the value 'foo'.
- final String? packageName;
+ final String packageName;
/// A description to be displayed alongside the preview.
///
diff --git a/dev/integration_tests/widget_preview_scaffold/lib/src/widget_preview_rendering.dart b/dev/integration_tests/widget_preview_scaffold/lib/src/widget_preview_rendering.dart
index eed0f87..fdf79b2 100644
--- a/dev/integration_tests/widget_preview_scaffold/lib/src/widget_preview_rendering.dart
+++ b/dev/integration_tests/widget_preview_scaffold/lib/src/widget_preview_rendering.dart
@@ -881,11 +881,7 @@
///
/// For example, if a preview is defined in 'package:foo/src/bar.dart', this
/// will have the value 'foo'.
- ///
- /// This should only be null if the preview is defined in a file that's not
- /// part of a Flutter library (e.g., is defined in a test).
- // TODO(bkonyi): verify what the behavior should be in this scenario.
- final String? packageName;
+ final String packageName;
// Assets shipped via package dependencies have paths that start with
// 'packages'.
@@ -903,8 +899,7 @@
if (key == 'AssetManifest.bin' ||
key == 'AssetManifest.bin.json' ||
key == 'FontManifest.json' ||
- key.startsWith(_kPackagesPrefix) ||
- packageName == null) {
+ key.startsWith(_kPackagesPrefix)) {
return super.load(key);
}
// Other assets are from the parent project. Map their keys to package
diff --git a/packages/flutter_tools/lib/src/widget_preview/dependency_graph.dart b/packages/flutter_tools/lib/src/widget_preview/dependency_graph.dart
index d7e4fde..8a41686 100644
--- a/packages/flutter_tools/lib/src/widget_preview/dependency_graph.dart
+++ b/packages/flutter_tools/lib/src/widget_preview/dependency_graph.dart
@@ -230,6 +230,10 @@
void findPreviews({required ResolvedLibraryResult lib}) {
// Iterate over the compilation unit's AST to find previews.
final visitor = _PreviewVisitor(lib: lib.element);
+ // Previews can only be defined under the lib/ directory.
+ if (visitor.packageName == null) {
+ return;
+ }
lib.units.forEach(visitor.findPreviewsInResolvedUnitResult);
previews
..clear()
diff --git a/packages/flutter_tools/lib/src/widget_preview/preview_detector.dart b/packages/flutter_tools/lib/src/widget_preview/preview_detector.dart
index 42c4be7..eb0ff03 100644
--- a/packages/flutter_tools/lib/src/widget_preview/preview_detector.dart
+++ b/packages/flutter_tools/lib/src/widget_preview/preview_detector.dart
@@ -37,6 +37,7 @@
required this.onChangeDetected,
required this.onPubspecChangeDetected,
@visibleForTesting this.watcherBuilder = _defaultWatcherBuilder,
+ @visibleForTesting this.onPackageConfigChangeDetected,
}) : projectRoot = project.directory;
final Platform platform;
@@ -47,6 +48,8 @@
final Logger logger;
final void Function(PreviewDependencyGraph) onChangeDetected;
final void Function(String path) onPubspecChangeDetected;
+ @visibleForTesting
+ final void Function(String path)? onPackageConfigChangeDetected;
final WatcherBuilder watcherBuilder;
@visibleForTesting
@@ -64,20 +67,21 @@
PreviewDependencyGraph get dependencyGraph => _dependencyGraph;
final PreviewDependencyGraph _dependencyGraph = PreviewDependencyGraph();
- late final collection = AnalysisContextCollection(
- includedPaths: <String>[projectRoot.absolute.path],
- resourceProvider: PhysicalResourceProvider.INSTANCE,
+ @visibleForTesting
+ AnalysisContextCollection get collection => _collection;
+ late AnalysisContextCollection _collection;
+
+ late final String _packageConfigPath = fs.path.join(
+ projectRoot.absolute.path,
+ '.dart_tool',
+ 'package_config.json',
);
/// Starts listening for changes to Dart sources under [projectRoot] and returns
/// the initial [PreviewDependencyGraph] for the project.
Future<PreviewDependencyGraph> initialize() {
return mutex.runGuarded(() async {
- // Find the initial set of previews.
- await findPreviewFunctions(projectRoot);
-
- // Determine which files have transitive dependencies with compile time errors.
- _propagateErrors();
+ await _initializeAnalysisContextCollection();
final Watcher watcher = watcherBuilder(projectRoot.path);
_fileWatcher = watcher.events.listen(
@@ -118,15 +122,38 @@
await mutex.runGuarded(() async {
await _fileWatcher?.cancel();
_fileWatcher = null;
- await collection.dispose();
+ await _collection.dispose();
});
}
+ Future<void> _initializeAnalysisContextCollection() async {
+ _collection = AnalysisContextCollection(
+ includedPaths: <String>[projectRoot.absolute.path],
+ resourceProvider: PhysicalResourceProvider.INSTANCE,
+ );
+
+ // Find the initial set of previews.
+ await findPreviewFunctions(projectRoot);
+
+ // Determine which files have transitive dependencies with compile time errors.
+ _propagateErrors();
+ }
+
Future<void> _onFileSystemEvent(WatchEvent event) async {
// Only process one FileSystemEntity at a time so we don't invalidate an AnalysisSession that's
// in use when we call context.changeFile(...).
await mutex.runGuarded(() async {
final String eventPath = event.path;
+ final File file = fs.file(eventPath);
+
+ // If the package_config.json for the project has been changed, we need to tear down the
+ // analysis context collection and recreate it to pick up the changes.
+ if (file.absolute.path == _packageConfigPath) {
+ await _collection.dispose();
+ await _initializeAnalysisContextCollection();
+ onPackageConfigChangeDetected?.call(event.path);
+ return;
+ }
// Ignore any files under .dart_tool or ephemeral directories created by
// the tool (e.g., build/, plugin directories, etc.).
if (eventPath.doesContainDartTool ||
@@ -147,7 +174,7 @@
AnalysisContext context;
try {
- context = collection.contextFor(eventPath);
+ context = _collection.contextFor(eventPath);
} on StateError {
// The modified file isn't part of the analysis context and is safe to
// ignore.
@@ -168,7 +195,6 @@
// extension which may be worth using here.
// We need to notify the analyzer that this file has changed so it can reanalyze the file.
- final File file = fs.file(eventPath);
context.changeFile(file.path);
final List<String> potentiallyAffectedFiles;
try {
@@ -243,7 +269,7 @@
final PreviewDependencyGraph updatedPreviews = PreviewDependencyGraph();
logger.printStatus('Finding previews in ${entity.path}...');
- for (final AnalysisContext context in collection.contexts) {
+ for (final AnalysisContext context in _collection.contexts) {
for (final String filePath in context.contextRoot.analyzedFiles()) {
logger.printTrace('Checking file: $filePath');
if (!filePath.isDartFile || !filePath.startsWith(entity.path)) {
diff --git a/packages/flutter_tools/templates/widget_preview_scaffold/lib/src/widget_preview.dart.tmpl b/packages/flutter_tools/templates/widget_preview_scaffold/lib/src/widget_preview.dart.tmpl
index 29be79b..9f1e65d 100644
--- a/packages/flutter_tools/templates/widget_preview_scaffold/lib/src/widget_preview.dart.tmpl
+++ b/packages/flutter_tools/templates/widget_preview_scaffold/lib/src/widget_preview.dart.tmpl
@@ -62,7 +62,7 @@
///
/// For example, if a preview is defined in 'package:foo/src/bar.dart', this
/// will have the value 'foo'.
- final String? packageName;
+ final String packageName;
/// A description to be displayed alongside the preview.
///
diff --git a/packages/flutter_tools/templates/widget_preview_scaffold/lib/src/widget_preview_rendering.dart.tmpl b/packages/flutter_tools/templates/widget_preview_scaffold/lib/src/widget_preview_rendering.dart.tmpl
index eed0f87..fdf79b2 100644
--- a/packages/flutter_tools/templates/widget_preview_scaffold/lib/src/widget_preview_rendering.dart.tmpl
+++ b/packages/flutter_tools/templates/widget_preview_scaffold/lib/src/widget_preview_rendering.dart.tmpl
@@ -881,11 +881,7 @@
///
/// For example, if a preview is defined in 'package:foo/src/bar.dart', this
/// will have the value 'foo'.
- ///
- /// This should only be null if the preview is defined in a file that's not
- /// part of a Flutter library (e.g., is defined in a test).
- // TODO(bkonyi): verify what the behavior should be in this scenario.
- final String? packageName;
+ final String packageName;
// Assets shipped via package dependencies have paths that start with
// 'packages'.
@@ -903,8 +899,7 @@
if (key == 'AssetManifest.bin' ||
key == 'AssetManifest.bin.json' ||
key == 'FontManifest.json' ||
- key.startsWith(_kPackagesPrefix) ||
- packageName == null) {
+ key.startsWith(_kPackagesPrefix)) {
return super.load(key);
}
// Other assets are from the parent project. Map their keys to package
diff --git a/packages/flutter_tools/test/commands.shard/hermetic/widget_preview/preview_code_generator_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/widget_preview/preview_code_generator_test.dart
index cda8927..4c40e97 100644
--- a/packages/flutter_tools/test/commands.shard/hermetic/widget_preview/preview_code_generator_test.dart
+++ b/packages/flutter_tools/test/commands.shard/hermetic/widget_preview/preview_code_generator_test.dart
@@ -409,6 +409,7 @@
testUsingContext(
'correctly generates ${PreviewCodeGenerator.getGeneratedDtdConnectionInfoFilePath(fs)}',
() async {
+ await previewDetector.initialize();
// Check that the generated preview file doesn't exist yet.
final File generatedDtdConnectionInfoFile = project.widgetPreviewScaffold.childFile(
PreviewCodeGenerator.getGeneratedDtdConnectionInfoFilePath(fs),
diff --git a/packages/flutter_tools/test/commands.shard/hermetic/widget_preview/preview_detector/preview_detector_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/widget_preview/preview_detector/preview_detector_test.dart
index d1f6871..78b32f0 100644
--- a/packages/flutter_tools/test/commands.shard/hermetic/widget_preview/preview_detector/preview_detector_test.dart
+++ b/packages/flutter_tools/test/commands.shard/hermetic/widget_preview/preview_detector/preview_detector_test.dart
@@ -177,6 +177,33 @@
expect(nodesWithPreviews.keys, unorderedMatches(project.librariesWithPreviews));
expectNPreviewReloadTimingEvents(previewDetector, 2);
});
+
+ testPreviewDetector('does not detect previews defined in tests', (
+ PreviewDetector previewDetector,
+ ) async {
+ project = await createProject(
+ projectRoot: previewDetector.projectRoot,
+ pathsWithPreviews: <String>[],
+ pathsWithoutPreviews: <String>[],
+ );
+ // The initial mapping should be empty as there's no files containing previews.
+ const expectedInitialMapping = <PreviewPath, LibraryPreviewNode>{};
+
+ final PreviewDependencyGraph mapping = await previewDetector.initialize();
+ expect(mapping.nodesWithPreviews, expectedInitialMapping);
+ expectNPreviewReloadTimingEvents(previewDetector, 0);
+
+ // Add a file containing previews under the test directory. Previews outside of lib/ are
+ // not valid.
+ await waitForNChangesDetected(
+ n: 1,
+ changeOperation: () => project.addPreviewContainingTestFile(path: 'foo.dart'),
+ );
+ final PreviewDependencyGraph nodesWithPreviews =
+ previewDetector.dependencyGraph.nodesWithPreviews;
+ expect(nodesWithPreviews, isEmpty);
+ expectNPreviewReloadTimingEvents(previewDetector, 1);
+ });
}
testPreviewDetector('can detect changes in the pubspec.yaml', (
diff --git a/packages/flutter_tools/test/commands.shard/hermetic/widget_preview/preview_detector/preview_detector_workspace_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/widget_preview/preview_detector/preview_detector_workspace_test.dart
index 338b5c7..da63d6b 100644
--- a/packages/flutter_tools/test/commands.shard/hermetic/widget_preview/preview_detector/preview_detector_workspace_test.dart
+++ b/packages/flutter_tools/test/commands.shard/hermetic/widget_preview/preview_detector/preview_detector_workspace_test.dart
@@ -102,15 +102,20 @@
final PreviewDependencyGraph initialPreviews = await previewDetector.initialize();
expect(initialPreviews.nodesWithPreviews.length, 2);
+ late WidgetPreviewProject bazProject;
+ await waitForPackageConfigChangeDetected(
+ changeOperation: () async =>
+ bazProject = await workspace.createWorkspaceProject(name: 'baz'),
+ );
+
// Add a new project to the workspace with single preview and verify it's detected.
await waitForChangeDetected(
onChangeDetected: (PreviewDependencyGraph updated) {
// The new preview in baz.dart should be included in the preview mapping.
expect(updated.nodesWithPreviews.length, 3);
},
- changeOperation: () async =>
- (await workspace.createWorkspaceProject(name: 'baz'))
- ..writeFile((path: 'baz.dart', source: simplePreviewSource)),
+ changeOperation: () =>
+ bazProject.writeFile((path: 'baz.dart', source: simplePreviewSource)),
);
});
diff --git a/packages/flutter_tools/test/commands.shard/hermetic/widget_preview/utils/preview_detector_test_utils.dart b/packages/flutter_tools/test/commands.shard/hermetic/widget_preview/utils/preview_detector_test_utils.dart
index 1b13884..3b682ac 100644
--- a/packages/flutter_tools/test/commands.shard/hermetic/widget_preview/utils/preview_detector_test_utils.dart
+++ b/packages/flutter_tools/test/commands.shard/hermetic/widget_preview/utils/preview_detector_test_utils.dart
@@ -28,6 +28,7 @@
// Global state that must be cleaned up by `tearDown` in initializeTestPreviewDetectorState.
void Function(PreviewDependencyGraph)? _onChangeDetectedImpl;
void Function(String path)? _onPubspecChangeDetected;
+void Function(String path)? _onPackageConfigChangeDetected;
Directory? _projectRoot;
late FileSystem _fs;
@@ -43,6 +44,7 @@
tearDown(() {
_onChangeDetectedImpl = null;
_onPubspecChangeDetected = null;
+ _onPackageConfigChangeDetected = null;
_projectRoot?.deleteSync(recursive: true);
_projectRoot = null;
});
@@ -96,6 +98,7 @@
fs: _fs,
onChangeDetected: _onChangeDetectedRoot,
onPubspecChangeDetected: _onPubspecChangeDetectedRoot,
+ onPackageConfigChangeDetected: _onPackageConfigChangeDetectedRoot,
);
}
@@ -119,6 +122,10 @@
_onPubspecChangeDetected?.call(path);
}
+void _onPackageConfigChangeDetectedRoot(String path) {
+ _onPackageConfigChangeDetected?.call(path);
+}
+
/// Test the files included in [filesWithErrors] contain errors after executing [changeOperation].
Future<void> expectHasErrors({
required WidgetPreviewProject project,
@@ -161,6 +168,19 @@
return completer.future;
}
+/// Waits for a package_config.json changed event to be detected after executing [changeOperation].
+Future<String> waitForPackageConfigChangeDetected({required void Function() changeOperation}) {
+ final completer = Completer<String>();
+ _onPackageConfigChangeDetected = (String path) {
+ if (completer.isCompleted) {
+ return;
+ }
+ completer.complete(path);
+ };
+ changeOperation();
+ return completer.future;
+}
+
/// Waits for a change detected event after executing [changeOperation].
///
/// Invokes [onChangeDetected] when a change is detected before the returned future is completed.
diff --git a/packages/flutter_tools/test/commands.shard/hermetic/widget_preview/utils/preview_project.dart b/packages/flutter_tools/test/commands.shard/hermetic/widget_preview/utils/preview_project.dart
index 1eb2094..5b96d4f 100644
--- a/packages/flutter_tools/test/commands.shard/hermetic/widget_preview/utils/preview_project.dart
+++ b/packages/flutter_tools/test/commands.shard/hermetic/widget_preview/utils/preview_project.dart
@@ -92,7 +92,11 @@
PackageConfig(
<Package>[
for (final String package in [..._packages.keys, ?injectNonExistentProject])
- Package(package, workspaceRoot.childDirectory('packages').childDirectory(package).uri),
+ Package(
+ package,
+ workspaceRoot.childDirectory('packages').childDirectory(package).uri,
+ packageUriRoot: Uri(path: 'lib/'),
+ ),
Package(
'flutter',
Uri(scheme: 'file', path: '$flutterRoot/packages/flutter/'),
@@ -173,7 +177,9 @@
return (
path: file.path,
- uri: PackageConfig(<Package>[Package(packageName, projectRoot.uri)]).toPackageUri(file.uri)!,
+ uri: PackageConfig(<Package>[
+ Package(packageName, projectRoot.uri, packageUriRoot: Uri.parse('lib/')),
+ ]).toPackageUri(file.uri)!,
);
}
@@ -184,7 +190,7 @@
await savePackageConfig(
PackageConfig(
<Package>[
- Package(packageName, projectRoot.uri),
+ Package(packageName, projectRoot.uri, packageUriRoot: Uri.parse('lib/')),
Package(
'flutter',
Uri(scheme: 'file', path: '$flutterRoot/packages/flutter/'),
@@ -292,6 +298,13 @@
librariesWithoutPreviews.add(previewPath);
}
+ /// Writes a file containing previews under `test/$path`.
+ void addPreviewContainingTestFile({required String path}) {
+ projectRoot.childDirectory('test').childFile(path)
+ ..createSync(recursive: true)
+ ..writeAsStringSync(previewContainingFileContents);
+ }
+
/// Adds a new library with a part at [path].
///
/// If the file name specified by [path] is 'path.dart', the part file will be named
