Allow modifying function prototypes

[rrousselGit]

Hello! Not sure if this is the right place but:

At the moment, the functional_widget example is used as:

void main() {
  runApp(const MyApp());
}

@FunctionalWidget(widgetName: 'MyApp')
Widget _buildApp(BuildContext context,
    {String? appTitle, String? homePageTitle}) {
  return MaterialApp(
      title: appTitle ?? 'Flutter Demo',
      theme: ThemeData(primarySwatch: Colors.blue),
      home: MyHomePage(title: homePageTitle ?? 'Flutter Demo Home Page'));
}

This means the widget is defined as _buildApp but used as MyApp.
This isn't ideal at it breaks the developer experience:

• a newcomer searching for "MyApp" may not know that its implementation is instead "_buildApp"
• go-to-definition on "MyApp" will point to the generated code when it would be more intuitive to be redirected to _buildApp
• "find all references" on _buildApp will not fill all the places where MyApp is used. Instead, it will point to the generated code, and we would have to use "find all references" on MyApp.
• Since _buildApp and MyApp are separate entities, if we want to rename MyApp, we need to also rename _buildApp for consistency
• developers may sometimes make the mistake of using "_buildApp" instead of "MyApp", when they should always use the class instead of the function (this is a common mistake with the functional_widget package)

It would be great if rather than creating a new element, we could override an existing element with a different prototype. So that all the developer tooling keeps working.

For example, considering classes no longer need a "new" keyword, one solution could be to transform:

@functionalWidget
Widget MyApp(BuildContext context, {required String title}) {
  return Text(title);
}

into:

class MyApp extends StatelessWidget {
  const MyApp({Key? key, required this.title}): super(key: key);
  final String title;
  @override
  Widget build(BuildContext context) {
    return Text(title);
  }
}

[TimWhiting]

This would potentially require deleting the original function or modifying the name of the original function. But I agree that it would be more intuitive to users.

[jakemac53]

We currently do have a principal of not wanting to modify any existing code in unsurprising ways. This would include changing a function to be a class.

These decisions aren't yet final and we will keep the use case in mind though.

We will be trying to address most of these concerns through the general IDE experience though - although the details aren't fully fleshed out yet. We want jump to definition to work for generated code (exactly where it should take you is an open question). For instance we could say jump to definition of anything generated should just take you to the macro application in user code (so it would take you to the _buildApp function in this case, if you navigated to MyApp). Or we could take you to a synthetic file and show you the generated MyApp class. You would see the reference to the _buildApp function in the build method there and could then navigate to that. Or maybe we would have a quick link on generated declarations to take you to the macro application that generated that thing. Lots of options here.

[rrousselGit]

Note that this isn't limited to functional_widget
The desire for modifying existing code is voluntary. I could see many use-cases.

For example Mobx-like action:

class Example {
  @action
  void method() {
    print('hello');
  }

  void another() {
    method();
  }
}

would become:

class Example {
  @action
  void method() {
    final event = mobx.startAction();
    try {
      print('hello');
    } finally {
      event.stop();
    }
  }

  void another() {
    method();
  }
}

Or debounce:

class Example {
  @Debounce(Duration(seconds: 1))
  void method(int param) {
    print('hello $param');
  }

  void another() {
    method(42);
  }
}

would become:

class Example {
  Timer? _methodTimer;
  @Debounce(Duration(seconds: 1))
  void method(int param) {
    _methodTimer?.cancel();
    _methodTimer = Timer(Duration(seconds: 1), (_) {
      print('hello');
    });
  }

  void another() {
    method(42);
  }
}

With those examples, if generated code can't replace existing code, that would make the refactoring process extremely tedious.

And another difficulty is that it will be difficult to compose generators.
For example we may want to use both @action and @debounce:

class Example {
  @Debounce(Duration(seconds: 1))
  @action
  void method(int param) {
    print('hello $param');
  }

  void another() {
    method(42);
  }
}

which would generate:

class Example {
  Timer? _methodTimer;
  @Debounce(Duration(seconds: 1))
  @action
  void method(int param) {
    _methodTimer?.cancel();
    _methodTimer = Timer(Duration(seconds: 1), (_) {
      final event = mobx.startAction();
      try {
        print('hello $param');
      } finally {
        event.stop();
      }
    });
  }

  void another() {
    method(42);
  }
}

If we can't override existing code, that will make such composition awkward.

---

Alternatively, we could have flutter_hooks use code-generation to transform:

class Example extends StatelessWidget {
  Widget build(context) {
    @state int count = 0;

    return Scaffold(
      body: Text('$count'),
      floatingButton: FloatingButton(
        onPressed: () => count++;
      ),
    );
  }
}

into:

class Example extends StatefulWidget {
  createState() => _ExampleState();
}

class _ExampleState extends State<Example> {
  int _count = 0;

  Widget build(context) {
    return Scaffold(
      body: Text('$_count'),
      floatingButton: FloatingButton(
        onPressed: () => setState(() => _count++);
      ),
    );
  }
}

[jakemac53]

The @action and @debounce ones are a bit different than what you want with functional widget - you aren't converting a method to a different type of declaration or changing the static type of it.

We currently do allow method wrapping, and the intention is to enable this style of macro, but the api exposed currently only allows injecting statements above and below the existing body, and they are also in their own scope and can't control exactly how the original body is wrapped. That wouldn't enable either of these so I think we should go back to the drawing board a bit on that api 👍 .

Suggestions welcome - I will start thinking more about this as well. We do want to ensure that the original method body isn't modified though.

[jakemac53]

One possible option here is we could just provide you with the original body as a Code object. That allows you to plop it directly into the new method, and just trust that you will use that somewhere. That is really flexible and users would likely complain to macro authors if they wrote macros that just discarded the entire original body haha.

[jakemac53]

cc @munificent thoughts?

[rrousselGit]

The @action and @debounce ones are a bit different than what you want with functional widget

Debounce is technically a function prototype change, as it modifies the return type from T -> void or T -> Future<T> depending on the implementation.

[jakemac53]

Debounce is technically a function prototype change, as it modifies the return type from T -> void or T -> Future<T> depending on the implementation.

True, I don't think we would want to allow that though. I understand it also feels weird to ask the user to make their function async when they don't do anything async - but I think its worse if the jump to definition for a method shows you the wrong return type in the actual code.

I am not sure exactly what the right balance to strike here would be, we could allow code like this to be valid, as long as the macro eventually modifies the function to return a Future for instance:

@debounce
Future<void> method() {
  print('hello');
} 

But that also is a bit weird to look at, as the function looks invalid at first glance.

[TimWhiting]

Random thought, feel free to reject. What if the macro system generates code into a collapsed block right after the annotation. Then you have both signatures side by side and the user would be less confused by changes / have to hop between files.

For the compiler: any declaration in a collapsed macro with the same name supersedes any declaration in the actual declaration list of a class, mixin, library, etc.

The main problem I see with having generated stuff in the same file is that users might think they can edit it.

@debounce{{ /// <- collapsible region
  Future<void> method() async {
    print('hello');
  } 
}}
void method() {
  print('hello');
} 

[rrousselGit]

One possible option here is we could just provide you with the original body as a Code object. That allows you to plop it directly into the new method, and just trust that you will use that somewhere. That is really flexible and users would likely complain to macro authors if they wrote macros that just discarded the entire original body haha.

In this case, rather than the function body, could the macro receive the entire annotated bloc (so the function + parameters)?

I believe the rule of "users would complain if the macro changes the code in a way that isn't expected" still applies.

If the prototype of a function changes due to a macro, then this is part of the contract of the said macro. Therefore if a user sees a method annotated, they would know that the prototype changed.

This is similar to what JS allows with decorators. Some examples would be withTheme/withReducer from popular React packages, which do something very similar to what the functional_widget example does.

[jakemac53]

I do still currently feel that we should draw a line at allowing modification of the static shape of existing declarations. Modifying function bodies through conceptually just "wrapping" them feels a fair bit less magic/unexpected to me. A user just looking at my API doesn't necessarily understand all the macros I am using, or how they modify my API. The exact implementation of my APIs matters a lot less imo, than the static shape of them, to my users.

I will acknowledge however that there are additional use cases to be had by allowing such modification. For instance the current flutter widget transformer adds a parameter to all widget constructors (and field to the class I believe), and then also modifies all constructor invocation sites to pass an argument for that (this is how the debug tool can navigate to the line of code that created a widget from the inspector). Being able to migrate that to a macro would indeed be nice, as the transformer is brittle.

So, I won't take it completely off the table yet :).

[rrousselGit]

user just looking at my API doesn't necessarily understand all the macros I am using, or how they modify my API.

Is the concern about the public interface of packages?

If so, that's a fair concern.
But is there no way to fix this? Like have the IDE/go-to-definition when pointing to a package show the generated code?

[jakemac53]

Is the concern about the public interface of packages?

Correct, this is my primary concern.

But is there no way to fix this? Like have the IDE/go-to-definition when pointing to a package show the generated code?

We are going to end up leaning heavily on the IDE (and analyzer) for a lot of things, but we do want to maintain a certain level of support for non-IDE users. There is a pretty big difference imo between synthesizing whole new declarations that those users just won't be able to see (outside maybe a debugger), and completely changing the shape of the program as they perceive it.

[rrousselGit]

Rather than the IDE, could this be taken care of by pub?

As in, when installing dependencies, their generators are already executed.
So it would be similar to publishing generated code on pub.

[jakemac53]

As in, when installing dependencies, their generators are already executed.

This gets complicated with path dependencies (at a minimum - the root package). You have to regenerate often for these. In general having this happen outside of the compilers is the largest complaint of the existing build_runner package. We also don't want to actually modify code on disk.

Ultimately one of the biggest problems we want to solve is the direct compiler integration, to make workflows like the edit/refresh cycle in flutter better, and improve the efficiency of code generation by sharing state with the compiler and directly hooking in to the hot reload flow.

And then also the usablity in terms of not needing to explicitly import part files and reference generated symbols.

[munificent]

I do still currently feel that we should draw a line at allowing modification of the static shape of existing declarations.

That line doesn't exist for users writing code generators today, does it? With code gen, you can do whatever you want and yet users seem to generally do reasonable things.

Modifying function bodies through conceptually just "wrapping" them feels a fair bit less magic/unexpected to me.

Agreed. I would prefer that most macros don't muck with signatures or method bodies in surprising ways. But I don't know if it's necessary for the macro API to strictly prohibit that, and I don't know if prohibiting it is a net win if it also prohibits beautiful, powerful use cases.

Unless there are technical reasons to restrict macros in certain ways, I would tend towards letting users do what they want and rely on their taste (and the negative feedback of their peers!) to guide them towards not writing macros that are nightmares to use.

I can remember in the early days of Dart users told us that Dart allowing operator overloading was a huge mistake because users would invariably use it to write nightmarish unreadable APIs. But... it turns out users don't want to write nightmarish unreadable APIs because then no one uses them and users like people to use their code.

A user just looking at my API doesn't necessarily understand all the macros I am using, or how they modify my API. The exact implementation of my APIs matters a lot less imo, than the static shape of them, to my users.

True. The closer a macro's output can reflect the structure of its input, the easier it will be for users to understand. But that's only one of several trade-offs a macro author might have to make and it might be good to give them enough flexibility to let them make that trade-off.

Also, users will need to see how macros modify the implementations of APIs so that they can step through them. We'll have to do the tooling work here regardless, so that will probably be equally useful in showing how a macro modifies signatures if we allow macros to do that.

[jakemac53]

That line doesn't exist for users writing code generators today, does it? With code gen, you can do whatever you want and yet users seem to generally do reasonable things.

Existing codegen (via package:build) is actually very restrictive. You can only create new parts or libraries, so there is no way to overwrite anything existing.

We don't generally hear complaints about that part in particular, people are much less happy with the other aspects like having to declare outputs as a mapping of input extension to output extension which is very limiting.

Unless there are technical reasons to restrict macros in certain ways, I would tend towards letting users do what they want and rely on their taste (and the negative feedback of their peers!) to guide them towards not writing macros that are nightmares to use.

I generally agree with giving users power, and we are already giving a huge amount of power here - significantly more than existing codegen does.

I believe that drawing a line at arbitrary manipulation of the shape of the program is a very reasonable place to draw the line.

We could take the "give the users power" argument all the way until we became Javascript and you could overwrite the prototype of even the core platform APIs, we have to draw a line somewhere :).

[rrousselGit]

Existing codegen (via package:build) is actually very restrictive. You can only create new parts or libraries, so there is no way to overwrite anything existing.

We don't generally hear complaints about that part in particular, people are much less happy with the other aspects like having to declare outputs as a mapping of input extension to output extension which is very limiting.

As a code-generator author, I very much dislike those restrictions.

They force generators to work around the language, causing very unnatural syntaxes.

Like Freezed which requires:

class Example with _$Example

That "with" shouldn't be necessary.

The alternative is to clone the library and expect users to import the clone instead of the original. But that is equally unnatural and breaks IDE/tooling workflows.

[jakemac53]

Like Freezed which requires:

class Example with _$Example

These types of things should no longer be needed though, that is definitely one of the primary pain points we are trying to make better by making this a real language feature.

You can see the data_class example, and its usage - it doesn't require any references to generated symbols or part files, etc.

[rrousselGit]

I see. But I suppose this wouldn't support adding new parameters to existing constructors

A common request from Freezed users is to be able to define:

@freezed
class Union with _$Union {
  factory Union.person(String name, int age) = Person;
  factory Union.company(String name, DateTime creationDate) = Company;
}

into:

@freezed
class Union with _$Union {
  factory Union.person(int age) = Person;
  factory Union.company(DateTime creationDate) = Company;

  int get name;
}

which is supposed to inject the "name" parameter in all constructors.

[jakemac53]

I see. But I suppose this wouldn't support adding new parameters to existing constructors

Correct, we wouldn't be able to support this exactly as you have outlined it, but you could support it some other way, where you generate those entire constructors. It would likely be less verbose as well, maybe something like this:

@Freezed(delegatingConstructors: {
  'person': Person,
  'company': Company,
})
class Union with _$Union {
  int get name;
}

Then you don't have to duplicate the customized parameters of those constructors either.

[rrousselGit]

Correct, we wouldn't be able to support this exactly as you have outlined it, but you could support it some other way, where you generate those entire constructors. It would likely be less verbose as well, maybe something like this:

@Freezed(delegatingConstructors: {
  'person': Person,
  'company': Company,
})
class Union with _$Union {
  int get name;
}

This isn't possible, no. Because the generator generates Person and Company based on the constructor.

We could need to reinvent constructors through decorators like:

@Freezed({
  'person': Case(
    name: 'Person',
    properties: [
      Property<int>('age', positional: true),
    ],
  ),
})
class Union {
  int get name;
}

which would be very unnatural

[jakemac53]

This isn't possible, no. Because the generator generates Person and Company based on the constructor.

Oh, so the Person and Company classes themselves are also generated in this case?

[rrousselGit]

Yes.

In many way what Freezed does is similar to what the data-class example does, with added support for union-types. But rather than defining the properties and generating the constructor, it uses the opposite mechanism: Define the constructor and it generate the implementation classes.

So a user define:

@freezed
class State with _$State {
  const factory State.loading({
    /// If non-null, the data before this state was refreshed
    String? lastData,
  }) = StateLoading;

  const factory State.data(String data) = StateData;
}

And this will generate:

class StateLoading implements State {
  const StateLoading({this.lastData});

  /// If non-null, the data before this state was refreshed
  final String? lastData;
}

class StateData implements State {
   const StateData(this.data);
  final String data;
}

and will also generate copyWith & co, followed by utilities to perform a switch-case on a union:

State state;

// equivalent `if state is StateLoading else if state is StateData`, but with all cases as "required"
state.when(
  loading: (String? lastData) {...},
  data: (String data) {...}
)

This syntax has the benefit of giving more control over the constructors. Folks can make parameters named required or positional optional if they want to.

Hence the desire for extracting shared parameters from all constructors, which requires macros being able to override the constructor prototype.

[jakemac53]

@Hixie @goderbauer @rrousselGit I am curious what you (or others, feel free to chime in) think about a design like this for eliminating stateful widget boilerplate, that plays more to the strengths of the current system.

/// I think we could likely come up with a better way of defining the state fields,
/// I know this way is pretty ugly.
///
/// Maybe something like: @statefulWidget(fields: [int count])
@statefulWidget(stateFields: {'count': int});
Widget _example(BuildContext context, ExampleState state, String title) {
  return Scaffold(
    body: Text('$title ${state.count}'),
    floatingButton: FloatingButton(
      onPressed: () => state.setState(() => state.count++;);
    ),
  );
}

Generates:

class Example extends StatefulWidget {
  final String title;
  Example(this.title) : super();

  createState() => ExampleState();
}

class ExampleState extends State<Example> {
  int _count = 0;

  Widget build(context) {
    return _example(context, this, widget.title);
  }
}

I realize there is an issue here with the _count variable initialization as well (this is playing loose and just initializing to 0). But I think if we assume we could have a more succinct way of defining the fields in the metadata annotation, the rest still makes sense.

[TimWhiting]

The main problem I see with this approach is that you are creating a method _example but have to use it as Example() with fewer parameters. Which is what this issue is about (changing shape of API).

Using the state as a parameter though is a good idea, though I would suggest a better way of doing it would be to annotate the state class instead.

@statefulWidget((BuildContext context, ExampleState state, String title){
  return Scaffold(
    body: Text('$title ${state.count}'),
    floatingButton: FloatingButton(
      onPressed: () => state.increment())
    ),
  );
});
class ExampleState extends State<Example> {
  @setsState
  int count = 0;
  void increment() => count++;
}

which would generate:

class Example extends StatefulWidget {
  final String title;
  Example(this.title) : super();

  createState() => ExampleState();
}

class ExampleState extends State<Example> {
  int get count => _count;

  int _count = 0;

  void set(int count){
    _count = count;
    setState((){}); 
  }
  void increment() => count++;

  Widget build(context) {
     final state = this;
     final title = widget.title;
     return Scaffold(
      body: Text('$title ${state.count}'),
      floatingButton: FloatingButton(
        onPressed: () => state.increment())
      ),
    );
  }
}

Most of which can be done with the current prototype except copying and pasting the build function from the annotation to the generated code. This would keep the naming separate from the generated code and consistent with what users' expect, rather than having to reference private functions. The other issue is replacing the field with a getter/setter pair.

[rrousselGit]

@statefulWidget(stateFields: {'count': int});

I don't think that this is a reasonable approach. This denatures too much the syntax

In general my experience working on code-generator is that we should avoid as much as possible from using annotations, strings and Type as much as possible, and instead prefer a syntax that relies on either function parameters of class properties.

For example Freezed could work with:

@Freezed({
  'count': int,
}
class Example {...}

But instead does:

@freezed
class Example {
  factory Example(int count) = _Example;
}

While this is technically more verbose when counting the characters count, the second syntax is a lot more natural.

On top of that, it allows things like documentation or annotations, where an alternate example would be:

@freezed
class Example {
  factory Example(
     /// Documentation for the [count] property. This comment is extracted by the generator
     /// and copy-pasted into the _Example.count property
    @Deprecated('We can use annotations as usual too')
    int count,
  ) = _Example;
}

[rrousselGit]

Similarly I don't think that the macro proposal solves the issue with StatefulWidgets.

The title of flutter/flutter#51752 is unfortunate. The issue isn't about how many characters it takes to type a StatefulWidget, but rather about how there is no way to share logic between StatefulWidgets.

flutter/flutter#51752 is about state logic composition, which flutter_hooks solves by allowing users to define something similar to mixins.

Something like @statefulWidget(stateFields: {'count': int}); would not solve this problem. It would focus on reducing the number of lines that a StatetfulWidget takes, but not make stateful logic any more reusable.

[jakemac53]

What I am trying to do is provide counter proposals of how might accomplish similar goals, without changing any existing API. My fundamental viewpoint is that macros should be purely additive - that is they should only be able to add things to the program. It is imo a very bad thing if I can look at a piece of code, but I can't actually use that code as written.

The question I want to answer is, can we solve the same problems that these other approaches are setting out to solve, while maintaining that fundamental approach? So that is what I was exploring with this idea :).

The main problem I see with this approach is that you are creating a method _example but have to use it as Example() with fewer parameters. Which is what this issue is about (changing shape of API).

That is why I made the method private - it is really just an implementation detail of the generated classes. Users don't interact with this method and can't see it.

Using the state as a parameter though is a good idea, though I would suggest a better way of doing it would be to annotate the state class instead.

I really like the idea of using the state class here, that avoids the issues around how to define fields.

Possibly we could even go a bit further, and instead of having the function be passed to the annotation, lets say the macro instead annotates a method on the class directly. So something like this (I left off the setsState part from your example just because I think its a different feature):

class ExampleState extends State<Example> {
  int count = 0;
  void increment() => setState(() => count++);
  
  @statefulWidget
  Widget _build(BuildContext context, String title) {
    return Scaffold(
      body: Text('$title ${count}'),
      floatingButton: FloatingButton(
        onPressed: () => increment())
      ),
    );
  }
}

That to me, actually starts to become something pretty reasonable.

The other issue is replacing the field with a getter/setter pair.

Yes, this part in particular is a piece of contention for sure. Today what you would do is just require that the field is private, and then the macro generates the public getter/setter pair. You can't actually implement a concrete field with a getter/setter pair unfortunately, is the problem. For instance how would you initialize it in a constructor, etc. I think the private field model is a lot simpler, and just sidesteps all the problems, although it may be a bit less convenient.

[rrousselGit]

What I am trying to do is provide counter proposals of how might accomplish similar goals, without changing any existing API.

Which goal are you referring to?

[rrousselGit]

I am asking this because it is likely that there is a misunderstanding on the goals.

My understanding is that we are trying to cover use-cases from my packages, which are functional_widget, flutter_hooks and freezed, which are related to multiple macro proposals and this + other issues.

But I don't see the relationship with these packages and the @statefulWidget annotation.

If this is about hooks / flutter/flutter#51752, reducing the number of lines that it takes to write a StatefulWidget is a non-goal.

The primary goals are state composition and improving readability by reducing the indentation.

[jakemac53]

In general my experience working on code-generator is that we should avoid as much as possible from using annotations, strings and Type as much as possible, and instead prefer a syntax that relies on either function parameters of class properties.

Yes I definitely agree on this point. I think moving things to the state class is a pretty reasonable solution here.

The title of flutter/flutter#51752 is unfortunate. The issue isn't about how many characters it takes to type a StatefulWidget, but rather about how there is no way to share logic between StatefulWidgets.

There are example macros in this repository which seem to me to solve the same fundamental types of issue though? You can write a macro, which looks at a field and then injects some code into all the necessary lifecycle methods to manage the lifecycle of that field, for instance.

If that doesn't solve the same fundamental problem that flutter_hooks solves, then it would be helpful to understand why.

See for example the autoDispose and RenderAccessors examples in this repo.

Which goal are you referring to?

This particular case was looking at simplifying stateful widgets (ie: functional_widget).

Note that I am not a flutter developer, and I am not actually familiar with your packages. So I need concrete feedback/explanations as to what you are trying to solve, and why you think that this proposal doesn't help.

But also remember that my goal here is not to allow you to replicate exactly any existing packages. It is to enable new packages that solve the same problems, in a way that is directly integrated into the compiler. This requires a fresh look at the problems those packages are trying to solve, to see how (or if) we can solve them in this system. We can then compare the different solutions from a number of different perspectives.

[goderbauer]

As @TimWhiting already stated, I don't really like the disconnect between call site and implementation that this is creating. I.e. you instantiate Example(), but there's no trace in the code where that's coming from. If I search for class Example in my code base e.g. on Github, nothing comes up. You need the implicit knowledge that @statefulWidget will generate that class out of the _example method or ExampleState class. I wonder how much harder this makes to read and understand the code.