Add `@widgetFactory` annotation #117455

blaugold · 2022-12-21T14:43:24Z

This PR adds the @widgetFactory annotation, which allows developers to mark extension methods as widget factories for the purpose of widget creation tracking.

Instrumentation of widget factories has been implemented in the Dart SDK (dart-lang/sdk#50067).

Related issues:

Improve debuggability of Widgets returned from extension factory methods #112468
Misleading widget creation location leads to bad debugging experience devtools#4772

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

goderbauer · 2023-01-03T23:09:44Z

packages/flutter/lib/src/widgets/widget_inspector.dart

This is a legit warning. Please fix it instead of ignoring it.

I was following the example of how pure marker annotations are typically defined, e.g. in package:meta. They usually use a private const class and expose only a const instance of that class, but with the private class type visible so that the type is available during static analysis. The static type is also what the CFE transformer relies on.

I'm not sure what a better solution in this case would look like. Maybe you have a suggestion?

I see, that makes sense. As per our style guide please leave a comment here explaining why the ignore is there.

goderbauer · 2023-01-03T23:11:13Z

packages/flutter/lib/src/widgets/widget_inspector.dart

Where does this instrumentation happen? Where is that implemented? As far as I can tell, this PR just defines an annotation that isn't doing anything?

I mentioned it in the PR description, but maybe not clearly enough :)

The instrumentation is implemented in the CFE kernel transformer that also implements the instrumentation of all classes extending Widget. This is the commit that landed support for widget factories in the Dart SDK: dart-lang/sdk@fe3f218.

Thanks for the clarifications!

goderbauer · 2023-01-03T23:11:39Z

cc @kenzieschmoll @jacob314

jacob314

goderbauer · 2023-01-18T18:53:52Z

packages/flutter/lib/src/widgets/widget_inspector.dart

These limitations seem rather artificial and make this annotation more cumbersome to use. Any reason why these have to exist? Can they be removed? Will I get a meaningful error if I use the annotation incorrectly?

The limitation regarding optional positional parameters applies to normal widget constructors too, and is related to how creation location tracking is implemented currently. There is a TODO to fix this.

There is also a TODO to create a lint rule for supported use of @widgetFactory.

The approach for instrumentation is currently geared towards static call targets like constructors, so extending it to extension methods which are lowered to static functions was relatively simple. I'm not sure what supporting instance methods would involve. Other static functions should not be too much work. Creating widgets in extension methods is common in some packages (e.g. flutter_animate) and allows for a style of writing build methods that avoids nesting, which is why I like it. But the dev experience was bad, which motivated this feature. I haven't seen patterns with non extension methods where creation location tracking is an issue in the same way, but I might not be aware of those uses cases.

I see. I defer to @jacob314 to determine if this limitation is acceptable.

Widgets rarely use optional positional parameters so this limitation has been acceptable in practice.
Potentially we should add a lint that Widgets should never use optional positional parameters as that prevents them from also supporting optional positional parameters which are crucial for most widgets.

goderbauer · 2023-02-07T18:20:40Z

Could you resolve the merge conflict? Thanks!

goderbauer

LGTM

auto-submit · 2023-02-14T23:22:45Z

auto label is removed for flutter/flutter, pr: 117455, due to - Please get at least one approved review if you are already a member or two member reviews if you are not a member before re-applying this label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

auto-submit · 2023-02-14T23:22:46Z

auto label is removed for flutter/flutter, pr: 117455, due to Validations Fail.

goderbauer · 2023-02-15T00:09:27Z

@jacob314 Can you approve this one as well?

jacob314

blaugold · 2023-02-15T14:47:47Z

@goderbauer The "Google testing" check seems stuck. Is there anything I should do?

goderbauer · 2023-02-15T17:12:04Z

Can you rebase this with the latest master? That should get the check unstuck.

blaugold · 2023-02-15T18:29:24Z

Can you rebase this with the latest master? That should get the check unstuck.

Hm, looks like that didn't help. 🤔

Edit: OK. It just took a bit longer :)

* 3ad7ea3 Roll Plugins from 9c312d4d2f5f to 2ce625f1a87e (5 revisions) (flutter/flutter#120793) * 7865713 Roll Flutter Engine from 1328c4bc6299 to 4db9673d48d6 (2 revisions) (flutter/flutter#120796) * 541a8bf Fix switching from scrollable and non-scrollable tab bars throws (flutter/flutter#120771) * ab1390e Use black30 for CupertinoTabBar's border (flutter/flutter#119509) * a513d4e Fix `flutter_localizations` README references (flutter/flutter#120800) * a664f08 In test of --(no-)fatal-infos analyzer flags, pin missing_return to info (flutter/flutter#120797) * ef49f56 Add Android unit tests to plugin template (flutter/flutter#120720) * a12e242 Improve CupertinoContextMenu to match native more (flutter/flutter#117698) * a9f4366 Fix the `flutter run -d linux` tests (flutter/flutter#120721) * dff0955 09da59a5a Roll Dart SDK from c022d475e9d8 to 5d17a336bdfe (1 revision) (flutter/engine#39649) (flutter/flutter#120816) * f35de0c Adds wide gamut saveLayer integration test (flutter/flutter#120131) * 99dcaa2 Roll Flutter Engine from 09da59a5adcf to a8b3d1af55b6 (3 revisions) (flutter/flutter#120821) * 8d15083 Use the impellerc GLES output flag when compiling shaders for Android (flutter/flutter#120647) * c6b636f [flutter_tools] Replace Future.catchError() with Future.then(onError: ...) (flutter/flutter#120637) * 2b7d709 Add `@widgetFactory` annotation (flutter/flutter#117455) * e65dfba Add Linux unit tests to plugin template (flutter/flutter#120814) * dccec41 5de007b90 Remove "bringup: true" from "Linux Fuchsia FEMU" (flutter/engine#39651) (flutter/flutter#120826) * d6de6bc 9f3b061b7 Roll buildroot to 64b0c3deecaff8e66c2deb74e2171e8297b2bfcd (flutter/engine#39653) (flutter/flutter#120830) * da2508c bb1ff84b6 Add a white background to app anatomy diagram (flutter/engine#39638) (flutter/flutter#120832) * 1f85497 [flutter_tools] Add the NoProfile parameter to the PowerShell execution statement (flutter/flutter#120786) * 4ad47fb Fix `StretchingOverscrollIndicator` not handling directional changes correctly (flutter/flutter#116548) * 9a721c4 Update AndroidManifest.xml.tmpl (flutter/flutter#120527) * c0b7d2d Roll Flutter Engine from bb1ff84b6c4f to 02a379db1d38 (4 revisions) (flutter/flutter#120845) * a10e295 Added identical(a,b) short circuit to Material Library lerp methods (flutter/flutter#120829) * efde350 Roll Flutter Engine from 02a379db1d38 to a966cf878ffd (2 revisions) (flutter/flutter#120846) * cc473e4 Roll Flutter Engine from a966cf878ffd to 3fc40ca5beb9 (3 revisions) (flutter/flutter#120850) * d125242 Roll Flutter Engine from 3fc40ca5beb9 to 9fa2a5c3cfbd (2 revisions) (flutter/flutter#120856) * 22e17bb ea1d087c4 Roll Skia from b8b36146c7a0 to 7b3fb04bc3d4 (3 revisions) (flutter/engine#39673) (flutter/flutter#120860) * f85438b c8b1d2ffa Roll Fuchsia Mac SDK from YpQKlqmyn8r_snx06... to xl9Y8o-9FDyvPogki... (flutter/engine#39675) (flutter/flutter#120887) * 174a562 d699b4a91 Roll Flutter from e3471f0 to df41e58 (83 revisions) (flutter/plugins#7184) (flutter/flutter#120888) * 170539f Roll Flutter Engine from c8b1d2ffaec8 to 0d8d93306822 (2 revisions) (flutter/flutter#120891)

* 3ad7ea3c9 Roll Plugins from 9c312d4 to 2ce625f (5 revisions) (flutter/flutter#120793) * 786571368 Roll Flutter Engine from 1328c4bc6299 to 4db9673d48d6 (2 revisions) (flutter/flutter#120796) * 541a8bfd9 Fix switching from scrollable and non-scrollable tab bars throws (flutter/flutter#120771) * ab1390e0a Use black30 for CupertinoTabBar's border (flutter/flutter#119509) * a513d4e7b Fix `flutter_localizations` README references (flutter/flutter#120800) * a664f08a5 In test of --(no-)fatal-infos analyzer flags, pin missing_return to info (flutter/flutter#120797) * ef49f5661 Add Android unit tests to plugin template (flutter/flutter#120720) * a12e242c0 Improve CupertinoContextMenu to match native more (flutter/flutter#117698) * a9f43665c Fix the `flutter run -d linux` tests (flutter/flutter#120721) * dff09558d 09da59a5a Roll Dart SDK from c022d475e9d8 to 5d17a336bdfe (1 revision) (flutter/engine#39649) (flutter/flutter#120816) * f35de0c80 Adds wide gamut saveLayer integration test (flutter/flutter#120131) * 99dcaa2d9 Roll Flutter Engine from 09da59a5adcf to a8b3d1af55b6 (3 revisions) (flutter/flutter#120821) * 8d150833b Use the impellerc GLES output flag when compiling shaders for Android (flutter/flutter#120647) * c6b636fa5 [flutter_tools] Replace Future.catchError() with Future.then(onError: ...) (flutter/flutter#120637) * 2b7d709fd Add `@widgetFactory` annotation (flutter/flutter#117455) * e65dfba8e Add Linux unit tests to plugin template (flutter/flutter#120814) * dccec41d5 5de007b90 Remove "bringup: true" from "Linux Fuchsia FEMU" (flutter/engine#39651) (flutter/flutter#120826) * d6de6bc68 9f3b061b7 Roll buildroot to 64b0c3deecaff8e66c2deb74e2171e8297b2bfcd (flutter/engine#39653) (flutter/flutter#120830) * da2508c9f bb1ff84b6 Add a white background to app anatomy diagram (flutter/engine#39638) (flutter/flutter#120832) * 1f85497ef [flutter_tools] Add the NoProfile parameter to the PowerShell execution statement (flutter/flutter#120786) * 4ad47fb47 Fix `StretchingOverscrollIndicator` not handling directional changes correctly (flutter/flutter#116548) * 9a721c456 Update AndroidManifest.xml.tmpl (flutter/flutter#120527) * c0b7d2ddd Roll Flutter Engine from bb1ff84b6c4f to 02a379db1d38 (4 revisions) (flutter/flutter#120845) * a10e295a0 Added identical(a,b) short circuit to Material Library lerp methods (flutter/flutter#120829) * efde35081 Roll Flutter Engine from 02a379db1d38 to a966cf878ffd (2 revisions) (flutter/flutter#120846) * cc473e4f1 Roll Flutter Engine from a966cf878ffd to 3fc40ca5beb9 (3 revisions) (flutter/flutter#120850) * d1252428c Roll Flutter Engine from 3fc40ca5beb9 to 9fa2a5c3cfbd (2 revisions) (flutter/flutter#120856) * 22e17bb71 ea1d087c4 Roll Skia from b8b36146c7a0 to 7b3fb04bc3d4 (3 revisions) (flutter/engine#39673) (flutter/flutter#120860)

…r#7186) * 3ad7ea3c9 Roll Plugins from 9c312d4 to 2ce625f (5 revisions) (flutter/flutter#120793) * 786571368 Roll Flutter Engine from 1328c4bc6299 to 4db9673d48d6 (2 revisions) (flutter/flutter#120796) * 541a8bfd9 Fix switching from scrollable and non-scrollable tab bars throws (flutter/flutter#120771) * ab1390e0a Use black30 for CupertinoTabBar's border (flutter/flutter#119509) * a513d4e7b Fix `flutter_localizations` README references (flutter/flutter#120800) * a664f08a5 In test of --(no-)fatal-infos analyzer flags, pin missing_return to info (flutter/flutter#120797) * ef49f5661 Add Android unit tests to plugin template (flutter/flutter#120720) * a12e242c0 Improve CupertinoContextMenu to match native more (flutter/flutter#117698) * a9f43665c Fix the `flutter run -d linux` tests (flutter/flutter#120721) * dff09558d 09da59a5a Roll Dart SDK from c022d475e9d8 to 5d17a336bdfe (1 revision) (flutter/engine#39649) (flutter/flutter#120816) * f35de0c80 Adds wide gamut saveLayer integration test (flutter/flutter#120131) * 99dcaa2d9 Roll Flutter Engine from 09da59a5adcf to a8b3d1af55b6 (3 revisions) (flutter/flutter#120821) * 8d150833b Use the impellerc GLES output flag when compiling shaders for Android (flutter/flutter#120647) * c6b636fa5 [flutter_tools] Replace Future.catchError() with Future.then(onError: ...) (flutter/flutter#120637) * 2b7d709fd Add `@widgetFactory` annotation (flutter/flutter#117455) * e65dfba8e Add Linux unit tests to plugin template (flutter/flutter#120814) * dccec41d5 5de007b90 Remove "bringup: true" from "Linux Fuchsia FEMU" (flutter/engine#39651) (flutter/flutter#120826) * d6de6bc68 9f3b061b7 Roll buildroot to 64b0c3deecaff8e66c2deb74e2171e8297b2bfcd (flutter/engine#39653) (flutter/flutter#120830) * da2508c9f bb1ff84b6 Add a white background to app anatomy diagram (flutter/engine#39638) (flutter/flutter#120832) * 1f85497ef [flutter_tools] Add the NoProfile parameter to the PowerShell execution statement (flutter/flutter#120786) * 4ad47fb47 Fix `StretchingOverscrollIndicator` not handling directional changes correctly (flutter/flutter#116548) * 9a721c456 Update AndroidManifest.xml.tmpl (flutter/flutter#120527) * c0b7d2ddd Roll Flutter Engine from bb1ff84b6c4f to 02a379db1d38 (4 revisions) (flutter/flutter#120845) * a10e295a0 Added identical(a,b) short circuit to Material Library lerp methods (flutter/flutter#120829) * efde35081 Roll Flutter Engine from 02a379db1d38 to a966cf878ffd (2 revisions) (flutter/flutter#120846) * cc473e4f1 Roll Flutter Engine from a966cf878ffd to 3fc40ca5beb9 (3 revisions) (flutter/flutter#120850) * d1252428c Roll Flutter Engine from 3fc40ca5beb9 to 9fa2a5c3cfbd (2 revisions) (flutter/flutter#120856) * 22e17bb71 ea1d087c4 Roll Skia from b8b36146c7a0 to 7b3fb04bc3d4 (3 revisions) (flutter/engine#39673) (flutter/flutter#120860)

flutter-dashboard bot added the framework flutter/packages/flutter repository. See also f: labels. label Dec 21, 2022

goderbauer reviewed Jan 3, 2023

View reviewed changes

jacob314 reviewed Jan 4, 2023

View reviewed changes

goderbauer reviewed Jan 18, 2023

View reviewed changes

blaugold force-pushed the feat/widget-factory branch from 87ff420 to 4b142cb Compare February 7, 2023 18:43

goderbauer approved these changes Feb 14, 2023

View reviewed changes

goderbauer added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 14, 2023

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Feb 14, 2023

jacob314 approved these changes Feb 15, 2023

View reviewed changes

goderbauer added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 15, 2023

blaugold added 5 commits February 15, 2023 18:14

Add @widgetFactory annotation

66e99a6

Simplify and fix example

301b0ef

Specify TargetKinds for widgetFactory

27e89ae

Explain why library_private_types_in_public_api is ignored.

f2b55d6

Trigger CI

8e56530

blaugold force-pushed the feat/widget-factory branch from bc36db6 to 8e56530 Compare February 15, 2023 17:14

auto-submit bot merged commit 2b7d709 into flutter:master Feb 15, 2023

This was referenced Feb 16, 2023

Roll Flutter from df41e58f6f4e to 22e17bb71050 (28 revisions) flutter/packages#3211

Closed

Roll Flutter from df41e58f6f4e to 22e17bb71050 (28 revisions) flutter/plugins#7186

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 16, 2023

2b7d709 Add @widgetFactory annotation (flutter/flutter#117455)

80832fd

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 16, 2023

2b7d709fd Add @widgetFactory annotation (flutter/flutter#117455)

de0550e

engine-flutter-autoroll mentioned this pull request Feb 16, 2023

Roll Flutter from df41e58f6f4e to 174a562a665d (30 revisions) flutter/packages#3212

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 16, 2023

2b7d709 Add @widgetFactory annotation (flutter/flutter#117455)

ad0a49b

engine-flutter-autoroll mentioned this pull request Feb 16, 2023

Roll Flutter from df41e58f6f4e to 170539f83dc8 (31 revisions) flutter/packages#3214

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 16, 2023

2b7d709 Add @widgetFactory annotation (flutter/flutter#117455)

b39fe0b

blaugold deleted the feat/widget-factory branch February 18, 2023 10:35

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023

2b7d709 Add @widgetFactory annotation (flutter/flutter#117455)

921a6cd

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023

2b7d709 Add @widgetFactory annotation (flutter/flutter#117455)

c28dfa8

Add @widgetFactory annotation #117455

Add @widgetFactory annotation #117455

Uh oh!

Conversation

blaugold commented Dec 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

goderbauer commented Jan 3, 2023

Uh oh!

jacob314 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

goderbauer commented Feb 7, 2023

Uh oh!

goderbauer left a comment

Choose a reason for hiding this comment

Uh oh!

auto-submit bot commented Feb 14, 2023

Uh oh!

auto-submit bot commented Feb 14, 2023

Uh oh!

goderbauer commented Feb 15, 2023

Uh oh!

jacob314 left a comment

Choose a reason for hiding this comment

Uh oh!

blaugold commented Feb 15, 2023

Uh oh!

goderbauer commented Feb 15, 2023

Uh oh!

blaugold commented Feb 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Add `@widgetFactory` annotation #117455

Add `@widgetFactory` annotation #117455

blaugold commented Dec 21, 2022 •

edited

Loading

blaugold commented Feb 15, 2023 •

edited

Loading