Add "extension types" section to style guide #158466

nate-thegrate · 2024-11-11T18:43:24Z

View change (in markdown format)

Extension types can't add instance variables or override Object fields (==, hashCode, or toString()), but the "static-only interface" gives complete control over the API surface.

An extension type can implement a supertype and redeclare existing fields, even when it normally wouldn't be a valid override:

extension type SpecialWidget._(StatelessWidget widget) implements StatelessWidget {
  const SpecialWidget()
      : widget = const KeyedSubtree(key: ValueKey(42), child: SizedBox.shrink());

  @redeclare
  void key() {
    print('hello');
  }

  @redeclare
  int get build => widget.hashCode;
}

void foo() {
  const SpecialWidget special = SpecialWidget();
  special.key(); // prints 'hello'

  const Widget notSpecial = special; // Identical to "special" but doesn't use its static type interface.
  final Key? key = notSpecial.key;   // Will be set as `ValueKey(42)`
}

Examples

nate-thegrate · 2024-11-11T18:55:21Z

docs/contributing/Style-guide-for-Flutter-repo.md

 Optimize for readability. Write detailed documentation.
 Make error messages useful.
 Never use timeouts or timers.
-Avoid `is`, `print`, `part of`, `extension` and `_`.
+Avoid hidden dependencies: powerful systems are composed of simple, reusable parts.


I don't have strong feelings about the line I added here, but I do feel that the previous version is outdated.

Avoid is

This advice applies to widgets, but IMO it's a blanket statement that ignores the overall advantages of type promotion.

Avoid print

This is covered by a linter rule.

Avoid part of

I agree that this is good advice, but I seldom ever see a non-Googler creating a new file; IMO we are better off keeping the below section but removing it from the summary.

Avoid extension

That's what this PR is about!

Avoid _

Prior to Dart 3.0, _ was exclusively used for anonymous parameter names, but now it's also meant to designate a wildcard pattern.

This top section is intended to be just a brief tl;dr of whatever the most common mistakes are. For example, there was a time where people were frequently uploading PRs that used is instead of carefully designed polymorphism, so the is advice got promoted to the top here, and that helped. We should feel free to keep updating this section to reflect whatever is the most useful advice to give at the moment.

Hixie · 2024-11-13T19:48:36Z

docs/contributing/Style-guide-for-Flutter-repo.md

+[cannot be safely accessed](https://github.com/dart-lang/sdk/issues/51641)
+if the receiver is a subtype that implements the interface.\
+In these cases, it's better to house these fields in an `extension`
+or to restructure them into private top-level functions if possible.


This seems like a really weird case. Maybe I'm not understanding it correctly? Do you have an example of this?

Accessing privates in other objects is generally dubious design in the first place...

Do you have an example of this?

Yes indeed! Went ahead and filed an issue:

Unsafe GlobalKey subtypes #158876

(Fortunately in this case, it can be resolved without using an extension.)

Hixie · 2024-11-13T19:54:24Z

docs/contributing/Style-guide-for-Flutter-repo.md

+
+Prefer specifying a non-nullable supertype (e.g. `implements Object`) when
+the representation type is non-nullable: this improves static analysis for
+null-checks.


This reads more like a description of what extension types are, than advice for when it's appropriate to use them. (The heading, "Use extension types to improve an API surface", taken literally, suggests using them all the time, which is probably not what is intended.)

I would probably replace this whole section with a parenthetical sentence in the previous section that just says something like "(Unlike extension methods, the use of extension types is fine.)", maybe with a link to the Dart documentation on what extension types are, what they're useful for, etc.

Thanks again for bringing up that the heading should be changed.

As far as including a description, it seems that the dart.dev pages for extension methods and extension types are geared toward describing rather than differentiating.
I believe including a distinction in the style guide would likely help contributors better understand how they should apply to the Flutter framework, though perhaps there would be a better place for this.

The style guide is meant to teach people what we've learned are best practices, it's not meant to teach people Dart. We should assume people know what extension methods and extension types are, and if we feel we need to say something about extension types at all, it should be about when they're appropriate with concrete examples. But we don't need to mention every Dart feature, only those where the choice of using it or not using it, or how to use it, is not necessarily obvious (especially as shown by people submitting PRs that misuse them).

Apologies for the late response here—you bring up some good points but overall I'm not completely on board.

I 100% agree that the information we choose to put in the style guide should reflect what problems or confusion we notice in PRs. I also agree with the style guide's advice regarding extension methods:

Prefer instead adding methods directly to relevant classes.

Since the dart.dev docs don't discourage either extension or extension type, there isn't much of a reason to make an explicit effort to differentiate between them, but based on interactions I've had within PR discussions, I do feel that it'd be helpful to draw this distinction here.
I'm interested to hear further thoughts you have (along with others' perspectives), whether it's through this comment thread or the tentative upcoming Dash Forum :)

chunhtai

In general changing style guide like this should probably go through a dash forum. It required a more wider audience for this kind of change

docs/contributing/Style-guide-for-Flutter-repo.md

nate-thegrate · 2024-11-13T21:36:31Z

docs/contributing/Style-guide-for-Flutter-repo.md

+ * `SimpleAPI` is an _extension type_.
+ * `SomeAPI` is the _representation type_ (and `_someAPI` is the "representation field").
+ * `Object` is the _supertype_.
+ * `run()` is "a `SimpleAPI` method".


These bullet points were inspired by the canoncial terminology section, though the goal here is to help establish the distinction between extension & extension type so contributors have a better understanding of how each is used appropriately.

nate-thegrate · 2024-11-13T21:40:35Z

docs/contributing/Style-guide-for-Flutter-repo.md

 It's the job of the reviewer to check that all these are present when reviewing a PR.


+### Effect of `extension` on API usability.


(I currently feel that the best spot for this is under Conventions.)

nate-thegrate · 2024-11-14T21:52:31Z

In general changing style guide like this should probably go through a dash forum. It required a more wider audience for this kind of change

This is a good point, thank you @chunhtai! I just filed #158954, so we have something to refer to if we go down this path.

nate-thegrate · 2024-11-19T17:15:40Z

Marking as draft for now, will reopen after a forum!

flutter-dashboard · 2024-11-19T17:19:19Z

This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

goderbauer · 2025-01-21T23:41:52Z

(triage): This is waiting on the dash forum.

nate-thegrate · 2025-01-30T17:56:18Z

No longer waiting on a dash forum!

github-actions bot added c: contributor-productivity Team-specific productivity, code health, technical debt. framework flutter/packages/flutter repository. See also f: labels. d: docs/ flutter/flutter/docs, for contributors labels Nov 11, 2024

nate-thegrate commented Nov 11, 2024

View reviewed changes

chunhtai self-requested a review November 12, 2024 23:03

justinmc requested a review from Hixie November 13, 2024 18:53

Hixie reviewed Nov 13, 2024

View reviewed changes

update style guide

700628b

nate-thegrate force-pushed the extension-style-guide-update branch from 43eab11 to d7b9c4b Compare November 13, 2024 21:22

adjustments based on code review

0256d79

nate-thegrate force-pushed the extension-style-guide-update branch from d7b9c4b to 0256d79 Compare November 13, 2024 21:26

chunhtai requested changes Nov 13, 2024

View reviewed changes

nate-thegrate commented Nov 13, 2024

View reviewed changes

Piinks mentioned this pull request Nov 13, 2024

TappableLabel extension type #158465

Closed

This was referenced Nov 14, 2024

Material.of(context) in the test framework #156341

Closed

[dash forum?] Adoping new features for readability #158954

Closed

nate-thegrate marked this pull request as draft November 19, 2024 17:15

nate-thegrate mentioned this pull request Nov 26, 2024

WidgetStateMap observer #159333

Closed

nate-thegrate closed this Jan 30, 2025

		It's the job of the reviewer to check that all these are present when reviewing a PR.


		### Effect of `extension` on API usability.

Add "extension types" section to style guide #158466

Add "extension types" section to style guide #158466

Uh oh!

Conversation

nate-thegrate commented Nov 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Examples

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!

Choose a reason for hiding this comment

Uh oh!

nate-thegrate Nov 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chunhtai left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nate-thegrate commented Nov 14, 2024

Uh oh!

nate-thegrate commented Nov 19, 2024

Uh oh!

flutter-dashboard bot commented Nov 19, 2024

Uh oh!

goderbauer commented Jan 21, 2025

Uh oh!

nate-thegrate commented Jan 30, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nate-thegrate commented Nov 11, 2024 •

edited

Loading

nate-thegrate Nov 18, 2024 •

edited

Loading