add initial autoListenable macro for fields #36
Conversation
Will take a look today! |
| widget.counter.value++; | ||
| } | ||
|
|
||
| void _handleCounter() { |
There was a problem hiding this comment.
This method is matched to widget.counter just by name? Wondering if that relationship can be made more explicit (i.e. less magic)?
There was a problem hiding this comment.
It could be made explicit via an annotation on the original field (a bit weird), or maybe an annotation on this method?
It is a bit magic for sure but the macro does try to keep you on the rails (it checks for the method, and that it matches the expected signature). I think this type of convention over configuration approach is actually highly desirable for a lot of people, but we could allow other more explicit options.
|
|
||
| final String title; | ||
|
|
||
| @override | ||
| State<MyHomePage> createState() => _MyHomePageState(); | ||
| } | ||
|
|
||
| @autoListenable |
There was a problem hiding this comment.
The placement of the annotation on the state object feels a little disconnected from the pieces that are more relevant to this (the actual Listenable or the listenable handler).
There was a problem hiding this comment.
I guess you could interpret this as "Hey, this state object listens to all Listenables it can find in the associated widget."
My guess is that most people (well, at least me) will be thinking the other way around: Either "here's a Listenable I want to listen to" or "here's a handler that I want to invoke when the listenable fires". Those two seem like more natural entry points / annotation points.
There was a problem hiding this comment.
Ya, given the widget/state separate this seemed the most intuitive to me (and least boilerplate). We can make other examples too that work a bit differently - and I do want to extend this one to also work for things from the state class (that are pulled off the context, etc). That will end up looking more like what you are talking about I think.
| } else { | ||
| // https://github.com/jakemac53/macro_prototype/issues/34 | ||
| throw ArgumentError( | ||
| '@autoListenable isn\'t compatible with a custom `initState` ' |
There was a problem hiding this comment.
Ya, just something specific to this prototype though. If I have the time I will try and make it work better here haha but this is all just throwaway stuff so I don't think its critical I get all these cases worked out.
Towards #23
cc @goderbauer can you do a review here? Much of this is actually checked in generated code - you can ignore all the
main.*.dartfiles exceptmain.gen.dartwhich is the hand written one.I want to follow up with exploring use cases for listenable objects that come from the build context next.
This also has some extra cleanup in the
fromPartsapis - I realized it doesn't make sense for them to auto-concat nested lists of code with a,. Instead you can supply a separator if you wish, otherwise pieces of code are just concatenated directly.I also did remove the usage of
autoDisposefrom the example - right now the prototype implementation doesn't support mixing these two macros together well, I filed a couple of related issues on that.