Repository navigation
[ Widget Preview ] Fix crash when @Preview annotations appeared outside of lib/ - #180768
Conversation
…side of `lib/` 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 flutter#178651
There was a problem hiding this comment.
Code Review
This pull request correctly fixes a crash that occurs when @Preview annotations are used outside of the lib/ directory. The approach of making packageName non-nullable and ignoring libraries without a package name is sound. The changes are consistent across the codebase, including templates, and a new test case has been added to verify the fix. I have one high-severity comment regarding a potential inconsistency in the test utilities that could lead to test failures.
jyameo
left a comment
There was a problem hiding this comment.
Overall lgtm! However seems there are some CI test failures, PTAL
|
autosubmit label was removed for flutter/flutter/180768, because The base commit of the PR is older than 7 days and can not be merged. Please merge the latest changes from the main into this branch and resubmit the PR. |
…peared outside of `lib/` (flutter/flutter#180768)
…peared outside of `lib/` (flutter/flutter#180768)
…peared outside of `lib/` (flutter/flutter#180768)
…peared outside of `lib/` (flutter/flutter#180768)
…peared outside of `lib/` (flutter/flutter#180768)
…peared outside of `lib/` (flutter/flutter#180768)
…peared outside of `lib/` (flutter/flutter#180768)
…peared outside of `lib/` (flutter/flutter#180768)
…peared outside of `lib/` (flutter/flutter#180768)
…peared outside of `lib/` (flutter/flutter#180768)
…peared outside of `lib/` (flutter/flutter#180768)
…peared outside of `lib/` (flutter/flutter#180768)
…side of `lib/` (flutter#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 flutter#178651
…eneration (flutter#192326) ## Description Fixes flutter#191755 Previews outside of `lib/` (such as at the root of a project or in `test/`) do not have a `package:` URI, so `FlutterWidgetPreviewDetails.packageName` is null. In flutter#180768, it was established that widget previews must be defined within importable libraries under `lib/`. Files outside `lib/` cannot be safely imported into the widget preview scaffold without breaking asset path mappings or failing to resolve `dev_dependencies`. While legacy `PreviewDetector` explicitly ignores libraries where `packageName == null`, the Dart Analysis Server (LSP) preview detector returns all `@Preview` annotations across the workspace. When `_buildPreviewsLsp` generated the preview list, it force-unwrapped `preview.packageName!`, crashing with `_TypeError: Null check operator used on a null value`. This change filters out previews where `packageName == null` in `PreviewCodeGenerator._buildGeneratedPreviewMethodLsp`, ensuring that previews outside `lib/` are safely ignored when using LSP preview detection. ## Tests - Added hermetic unit test `ignores previews with null packageName for LSP updates` in `packages/flutter_tools/test/commands.shard/hermetic/widget_preview/preview_code_generator_test.dart`.
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 updatespackageNameproperties to not allow for null to make it clear that package names are required for previews.Fixes #178651