dart-lang / language

Design of the Dart language
Other
2.66k stars 205 forks source link

Property mixins #1353

Open ds84182 opened 3 years ago

ds84182 commented 3 years ago

One of the most powerful features in Kotlin is the ability to abstract over properties via delegation to wrapper objects. https://kotlinlang.org/docs/reference/delegated-properties.html

In Kotlin, this allows the reuse of behaviors for getters and setters. Or in Dart terms, it is a mixin that can be applied to an object member. These mixin applications are not like typical mixin applications, since they anonymously "graft" into their targets in an implementation-defined way. They also do not add new members to the class, only overriding existing members or providing new members through property member access.

Real life example of where this would be useful:

// Allows body-less constructors for passing arguments into the property mixin. Not used in this example.
property mixin AutoAnimationController for AnimationController on TickerProviderStateMixin {
  // Property mixin fields are renamed to be a unique but inaccessible field.
  // For non-overrides, these fields can be accessed through a new unique syntax: `field::propertyField`
  // However, overrides work like normal mixins (e.g. can insert behavior and delegate upwards).
  // Note that these are overrides of the class in the on clause, and the implements clause is not allowed.
  // Implementations can either implement property mixins as runtime delegates to a separate object (open world?),
  // or rewrite the target class to add fields and methods as actual members to reduce GC overhead (closed world/AOT?). 
  late AnimationController _controller;
  AnimationController get => _controller;
  // Lack of setter means that the mixin must be applied to a final field.
  // In general, the matrix applies:
  //                 | Final | Non-Final
  // Getter          | OK    | Error
  // Setter          | Error | OK
  // Getter + Setter | Warn  | OK

  // These override methods to delegate and initialize the hidden _controller field.
  @override
  void initState() {
    super.initState();
    _controller = AnimationController(vsync: this);
  }

  @override
  void dispose() {
    _controller.dispose();
    super.dispose();
  }
}

class MyCoolState extends State<MyCoolWidget> with SingleTickerProviderStateMixin {
  final _controller with AutoAnimationController;
  // Shortcut syntax for zero-arg property mixins. equivalent to `with AutoAnimationController()`
  // mixins with arguments would take the form `with AutoAnimationController(duration: Duration(seconds: 120))`
  // In general, mixin arguments can be any expression, with the semantics of a hidden late final variable.
  // Implementations are free to optimize in the case where the mixin argument is a compile time constant.
  // Mixin arguments with side effects may have undefined evaluation order/timing.

  @override
  void build(BuildContext context) {
    return ...; // Do something cool with _controller!
  }
}

The code snippet above solves one of Flutter's pain points, by generalizing the cruft needed for AnimationController setup.

Another example, using arguments and public property fields:

// A property that listens to a Future and updates the [value] property member and marks the state for a rebuild.
property mixin AutoFuture<T> for FutureOr<T> on State {
  T value; // Accessible through property member access syntax.
  AutoFuture(this.value); // Initial value must be provided (NNBD)

  FutureOr<T>? _future;
  FutureOr<T>? get => _future;
  // If [f] is a value, update [value] and mark for rebuild.
  // If [f] is a Future, subscribe.
  // When Future completes, if the future hasn't been set again and still mounted,
  // set the value and mark for rebuild.
  set(FutureOr<T>? f) {
    if (!mounted) return;
    _future = f;
    if (f is T) {
      value = f;
      setState(() {});
    } else if (f is Future<T>) {
      f.then((v) {
        if (!identical(_future, f)) return;
        value = v;
        if (!mounted) return;
        setState(() {});
      });
    }
  }
}

class MyCoolerState extends State<MyCoolerWidget> {
  final _assetFuture with AutoFuture<String?>(null);

  @override
  void initState() {
    super.initState();
    _assetFuture = asyncAssetLoadingIncantationsHere();
  }

  @override
  Widget build() => Text(_assetFuture::value ?? 'Asset not loaded yet');
}

So... Why?

In general, Flutter devs face many issues when dealing with asynchronous resources, or animation controllers, or other disposable objects. There are so many ways to write buggy code, and in many cases the way to write it correctly is non-intuitive. While there are IDE integrations for generating things like animation boilerplate, things like properly handling Futures in a State can lead to bugs if proper mounted checks aren't done, this goes for all asynchronous resources really.

Minimizing the amount of duplicate code and relying on a single implementation helps Flutter developers immensely, and giving developers a library of handy property mixins is a step in the right direction.


cc @rrousselGit, who wrote https://github.com/rrousselGit/flutter_hooks as an alternative way of solving this problem

lrhn commented 3 years ago

As I read it, the general effect is to introduce a field (getter, setter and initializer, possibly some more private support stuff for those) through a mixin. The only thing new is that you get to choose the name that the mixed in field is introduced as, so you are really creating a mixin where you abstract over one of the member names. Which is why it's applied to the field declaration instead of the class, so you can specify the field name.

(Or, in other words, it's a code template which abstracts over the name.)

ds84182 commented 3 years ago

@tatumizer In comparison with Kotlin, you have to specify the field type at the field declaration site, but here the intention is that the type is inferred from the mixin declaration. Due to this the mixin declaration is a bit cluttered, but field declaration isn't.

For both examples, without inference: final AnimationController _controller with AutoAnimationController; and final FutureOr<String>? _assetFuture with AutoFuture<String?>(null); which has a considerable amount of noise. The name of the property mixin should declare the type (if not completely generic) and the behavior.

You could also infer the type from the getter and setter, but then the field type of a property mixin isn't easy to see at a glance. Lots of different tradeoffs here.

mraleph commented 3 years ago

@lrhn I think requested effect is larger than that. I would expect that initState needs to be auto-delegated from surrounding class into each of its properties, so that we get something like this:

class MyCoolState extends State<MyCoolWidget> with SingleTickerProviderStateMixin {
  final _controller with AutoAnimationController;

  // Implicit:
  // @override
  // void initState() {
  //  super.initState();
  //  _controller.initState();
  // }

Am I guessing your intent right @ds84182?

ds84182 commented 3 years ago

@mraleph Yes, that's correct. The intent is to match the same behavior as normal mixins while disallowing the introduction of new members.

I wouldn't say this desugars to delegations though, consider this example:

abstract class SerializationObject {
  Map<String, dynamic> serialize() => {};
}

property mixin SerializedProperty<T> for T on SerializationObject {
  final String _name;
  SerializedProperty(this._name);

  T _data;
  T get => _data;
  set (T value) => _data = value;

  @override
  Map<String, dynamic> serialize() => super.serialize()..[_name] = _data;
}

Because of how super.serialize() is called here, a simple approach to delegation won't work. Adding more SerializedProperty fields places each field in a chain in declaration order, similar to what happens with mixins today.

One additional thing to explore is whether we can expose a plain field from a property mixin, so the field can be initialized within a constructor. There are some cases, like SerializedProperty, where the result you want is a normal field, but you also want additional behaviors around that field.

lrhn commented 3 years ago

A normal mixin with an void initState() { something(); super.initState(); } would also get called when you call the resulting class' initState. That's exactly what mixin applications do, so you get that for free.

If you look at the property mixin declaration, the only thing which really differs from a normal mixin is that get and set doesn't have a name, and you attach the mixin to a class and field declaration, where the latter supplies the name (and there are some extra restrictions to make the types match up), and everything except that get and set can be just mixed in as normal.

So, if you instead had a hypothetical "syntactic mixin" where you pass in a name as some kind of parameter, and it then declares a member by that name, instead of using a fixed name, I think you could achieve the same. (Lots of other things will have to be different because of that, for example the mixin cannot have an interface,)

ds84182 commented 3 years ago

@tatumizer The point of initState is that its a Flutter-ism. Being able to insert behavior into the mixin's target class is valuable, especially for Flutter. initState is not a workaround for the lack of having constructors, and the proposal completely supports body-less constructors for property mixins.

lrhn commented 3 years ago

This seems like a job for macros. #1482

rrousselGit commented 3 years ago

@lrhn

This seems like a job for macros. #1482

I don't quite agree with that

This proposal seem to be targeting https://github.com/flutter/flutter/issues/51752 and related issues.
But one complexity of the issue is to deal with partial widget rebuild – which macros cannot help with.

While we could use macros to implement something similar to what is described in this issue, this would cause a performance downgrade by rebuilding more widgets than necessary.

We'd likely need https://github.com/dart-lang/language/issues/1874 to solve the rebuild issue, which doesn't seem planed as part of the current macro proposal