[Marketing and API] Improvement suggestions for get_it and get_it_mixin

[dancamdev]

This issue is a collection of thoughts I had when reading the gskinner article on their state management and DI solution using get_it and get_it_mixin. The main goal here is to have a discussion that could lead to improvements in the marketing and API aspects of the packages.

Some context on my experience with state management and flutter.

• I’ve been using riverpod in production for a while.
• Before that I used GetIt + Provider, but the simplified way of accessing a provider from another provider that came with riverpod, made me drop get it.
• Another great feature that encouraged me to switch from provider to riverpod was the .when syntax on futureProviders and StreamProviders. Which I believe has been then added to Provider as well? Might be wrong on this one, has been a while.
• Riverpod comes with its complexities and flaws IMO, especially since version 1.0. Having to extend another type of widget, having to pass an instance of WidgetRef to access the providers and many more. These are problems that get_it + get_it_mixin solve.
• I’ve never used get_it alongside its mixin, nor studied its codebase thoroughly, so the following analysis is mostly based on this article here and my first impressions on the architecture, as well as my experience with other libraries in the dart and flutter ecosystem.

Get it & Get it mixin considerations

• It’s not obvious how get it mixin is related to getIt (if at all).

• People may not even know what a mixin is in dart and what it does. (You don’t need them that often in day to day flutter dev). Also the fact that it is a mixin doesn’t really help understand how it works.

• My thesis here, is that people care a lot about the package name, in the end it’s the first clue on what the package does. GetitMixin doesn’t really tell me anything, apart that it’s related to getIt in some way, but do I know getIt?

• Exporting getitmixin together with getIt could improve adoption.

• Eventually changing get_it_mixin package name to a more descriptive name could help. (get_it_watchable).

• API is great and simple, once you know how to access it. watchX() for example is not a really descriptive name unless you know where it’s coming from. (What does the X mean? Could we just have watch?

• Even better would be if under the hood we’d use some extension and have something like:

mySingleton.watch()

• Similarly, watchOnly could become watchValue to make it obvious what its meant to be used for.
• In general taking inspiration for something like go_router when it comes to extension methods and how they work can help a lot IMO.
• Dependency override for testing is currently done by pushing a new scope. It could not be clear at first what a scope is and how we should use them. Is it possible to abstract it away and call something like:

GetIt.I.override([...]);

Personally, as a user of other state management solution, the gskinner article helped me hugely to understand what this is all about, so maybe an improvement on the documentation explaining core concepts and how to use them with practical examples can benefit the package a lot as well.

[esDotDev]

Ya I agree with a lot that is said here:

• Having it be one combined package does make sense from a marketing / user perspective
• The naming does leave something to be desired in terms of clarity, the X throws me as well
• The use of extensions (or just methods) is very interesting and I also like it a quite a bit, both foo.override() and foo.watch() but would have to think on it some more.

[esDotDev]

I think there are a couple of threads here that we can probably split out:

• Should GetItMixin be automatically exported from GetIt
  • If so, we should do a wholistic pass on the readme for GetIt, and really explain how the libs play together.
• Naming / API changes
  • Maybe we can split this into it's own issue, or a discussion thread? I agree there might be room for improvement here, but would be nice to see a more complete proposal.

I agree that the naming is not the most communicative, in some sense all the users really want to know about is GetIt and the mixin is just a tool that comes with it, like ConsumerWidget or HooksWidget etc.

It's probably fine that it continues to exist as it's own package with it's current name, as long as, from the GetIt page, we treat it as a first class citizen, and mention it explicitly as the way to watch GetIt data.

[esDotDev]

Also, @dancamdev I'd like to get your feedback on the new proposed readme for GetIt (short-term solution to some of these problems):
escamoteur/get_it_mixin#24

[dancamdev]

Sure, I'll have a look at the read me ASAP.

I opened this issue to sum up a few of my thoughts, it's definitely worth splitting them into multiple ones, each one focusing on a specific topic.

[esDotDev]

General API thoughts, just to get them out there:

• watchX, watchOnly, watchXOnly etc are confusing. watch, watchValue/select and get make a lot of sense to me.

This feels most natural to me:
get<AppModel>().watch()
get<AppModel>().select((m) => m.isLoggedIn)

The override proposal looks nice, but I'm not sure it will work properly without specifying generics for each type, which isn't much of an improvement in the end.

GetIt.I.override([
  () => GetIt.I.registerSingleton<Foo>(FooMock())
]);

You actually lost a line over the manual push... it's also less clear what is actually happening. /shrug. If anything, maybe pushScopeAndOverride? But I dunno... kinda seems better to just teach people how to use scopes. They are simple and powerful and worth talking about more?

@escamoteur It would be nice to know if you think we can get rid of watch, watchOnly, watchXOnly and replace with just contextual watch and watchOnly (or select). Maybe we can get rid of watchFuture and watchStream if we do something like:

get<AppModel>().streamA.watch()
get<AppModel>().futureB.watch()

I don't really understand all the various use cases enough to say, nor the level of effort required to implement things this way :D

[esDotDev]

This would also address one of my main annoyances with the mixin now:

I usually set up getters for my singletons, like SettingsController get settings => GetIt.I.get<SettingsController>(), so when reading from my views I simply use settings.doStuff(), but then when it comes to binding, I have to use an alternate syntax:
watchX((SettingsController s) => s.foo);

Would be kinda cool if this could all be consolidated:

settings.doStuff();
settings.watch(this);
settings.select(this, (s) => s.foo);

this would be a mixin in this case, as I guess we still need to pass a reference somehow.

This could be re-written without my helper methods as:

get<SettingsController>().doStuff();
get<SettingsController>().watch(this);
get<SettingsController>().select(this, (s) => s.foo);

This would be accomplished by adding extension methods to the various types that the mixin supports (ValueListenable, Future etc). In this case settings would be a ChangeNotifier, which has gained the watch and select methods. ValueListenables, Streams and Futures would only get watch (I think?)

[esDotDev]

If we make a shared interface / abstract class that the StateMixin and the Mixin share, then the extension method thing should be doable?? Looks like they largely share the same interface:

[esDotDev]

well, fwiw, I tried to do a proof of concept and immediately found a dart compiler bug :D
dart-lang/sdk#48468

[dancamdev]

Would love to pick up the discussion where we left it off, I'm planning to contribute more on this issue.

[escamoteur]

Hi, I haven't forgotten this. I will try to get back to it after vacation

[dancamdev]

@escamoteur would be great to have your feedback on this!

[escamoteur]

Hi, I'm sorry for the long time. I have to reread all the discussion again.
Unfortunately someone else published a flutter_getit package which would have been the name for a new combined package.
I'm currently using get_it, the mixin and flutter_command in a real project which gives me a lot of impulses.

[escamoteur]

I definitely am open to a renaming of the methods, even add some more that are easier to use with less parameters.
As nice as the extensions look like, I don't like that we have to pass in the mixin as this .
A flutter_get_it package could also contain a GetItWidget instead of the Mixin which probably would it make more friendly for many people.
If you have a good name idea for a successor package let me know

[cliftonlabrum]

After spending time with Provider, Riverpod, and GetX state management, I recently discovered Get It + Get It Mixin and feel like it's a super-elegant solution. I agree that merging these packages makes things a lot clearer for someone that is new to this.

[escamoteur]

@cliftonlabrum if you want to get involved more, I'm happy about any hand that joins the effort.

[cliftonlabrum]

@escamoteur I'm still fairly new to Flutter, so I'm unsure how much dev help I can provide, but I can help with design and documentation.

Are you considering an entirely new name? I always thought it a touch confusing that there is GetX and GetIt in Flutter, but they are entirely different. 😅

If you are open to new names, what about Crux?

an essential point requiring resolution or resolving an outcome; a main or central feature

[escamoteur]

Yeah, the name conflict with getX is annoying. get_it is much older than getX. But especially on documentation and feedback from a normal users view will be helpfull.

Crux sounds a bit like crutch :-) but lets collect ideas

[cliftonlabrum]

I also have an idea for simplifying the API in widgets.

Let's say I have a UI class that I register:

GetIt getIt = GetIt.instance;
getIt.registerSingleton(UI());

My class looks like this:

class UI extends ChangeNotifier {
  //+++
  static UI get to => getIt<UI>();
  //+++
  bool darkMode = true;
  String accentColor = 'blue';
  int unreadCount = 0;
}

If I want to watch three properties from my class in a widget, I could do this:

class MyWidget extends StatelessWidget with GetItMixin {
  MyWidget({ Key? key }) : super(key: key);

  @override
  Widget build(BuildContext context) {
    //+++
    final darkMode = watchOnly((UI data) => data.darkMode);
    final accentColor = watchOnly((UI data) => data.accentColor);
    final unreadCount = watchOnly((UI data) => data.unreadCount);
    //+++
    return Container(
      color: darkMode ? Colors.black : Colors.white,
      etc.
    );
  }
}

Or sometimes I do it like this:

class MyWidget extends StatelessWidget with GetItMixin {
  MyWidget({ Key? key }) : super(key: key);

  @override
  Widget build(BuildContext context) {
    //+++
    watchOnly((UI data) => data.darkMode);
    watchOnly((UI data) => data.accentColor);
    watchOnly((UI data) => data.unreadCount);
    //+++
    return Container(
      color: UI.to.darkMode ? Colors.black : Colors.white,
      etc.
    );
  }
}

But if I have several properties to watch, and all I had to do was pass the name of the variables, I think that could be even simpler:

class MyWidget extends StatelessWidget with GetItMixin {
  MyWidget({ Key? key }) : super(key: key);

  @override
  Widget build(BuildContext context) {
    //+++
    watch([UI.to.darkMode, UI.to.accentColor, UI.to.unreadCount]);
    //+++
    return Container(
      color: UI.to.darkMode ? Colors.black : Colors.white,
      etc.
    );
  }
}

I'm sure there could be an even more succinct way to handle it without the .to getter I made. But if we're accessing properties of a singleton, it seems like we just need to tell our widget what to watch in order to rebuild. This is kind of how SwiftUI does it with @Published variables in a view.

Just my perspective after a few days with Get It. 😄

[esDotDev]

I think in the cases where you use the value of the field you are watching (almost always?), it would be good practice to encourage them to store off the value, otherwise it would be quite easy to forget to bind to the method and then create a bunch of hard to spot bugs.

eg, If you do it like this, nothing can break:

final count = watchOnly((UI data) => data.unreadCount);
final accent = watchOnly((UI data) => data.accentColor);

return Text(count, style: TextStyle(color: color);

Where as this is more brittle:

watchMultiple((UI data) => [data.unreadCount, data.accentColor]);

return Text(data.unreadCount.value, style: TextStyle(color: data.accentColor.value);

It is easy to omit the watch completely, and compiler would show no errors. You would only spot the error at runtime, when you notice things are not updating as expected.

Seems like saving a cpl lines here is not worth the increased likelihood for bugs,

[escamoteur]

while experimenting on how to realize a GetItWidget that could be const checked how flutter_hooks are doing it and it's in a way brilliant but also very bold.
As you can't have a not final HookeState object in the Widget the state is stored in the component element (HookElement). And to make it accessible inside the build functions the HookElement overrides its build function and assigns its state to a static field (more or less a global variable) before it calls the real build function.

(here is the part of flutter_hooks https://github.com/rrousselGit/flutter_hooks/blob/c89f05fa24e699f3d478bc87aad4df770f160e56/packages/flutter_hooks/lib/src/framework.dart#L414)

With this trick hook functions can be normal functions and still access the hookstate object.

We could do the same, which would reduce the amount of code of the mixin because we wouln't need two mixins (one for stateless and one for stateful widgets) to make the watch functions accessible. If people want to continue use the mixins, they would only need mixins on the widgets and not a separate on the state.

Possible drawbacks:

• global functions which may polute the namespace
• As long as widget code is always single threaded this should work without problems. Not sure what happens if Flutter starts to support multiple windows.

Does anyone of you see any potential problems?

Instead of global functions we could add them as functions to GetIt like

    userName = GetIt.I.watchProperty( (UserManager m) => m.userName);

the disadvantage of that would be that the get_it package would depend on Flutter.

If we start with a completely new package name (like Remi di with Riverpod) that wouldn't matter.

[escamoteur]

adding @kekland to the discussion

[kekland]

I think in the cases where you use the value of the field you are watching (almost always?), it would be good practice to encourage them to store off the value, otherwise it would be quite easy to forget to bind to the method and then create a bunch of hard to spot bugs.

eg, If you do it like this, nothing can break:

final count = watchOnly((UI data) => data.unreadCount);
final accent = watchOnly((UI data) => data.accentColor);

return Text(count, style: TextStyle(color: color);

Where as this is more brittle:

watchMultiple((UI data) => [data.unreadCount, data.accentColor]);

return Text(data.unreadCount.value, style: TextStyle(color: data.accentColor.value);

It is easy to omit the watch completely, and compiler would show no errors. You would only spot the error at runtime, when you notice things are not updating as expected.

Seems like saving a cpl lines here is not worth the increased likelihood for bugs,

@esDotDev @escamoteur It might potentially be possible to use something like https://pub.dev/packages/custom_lint to create lints to help users avoid situations where they can accidentally introduce some runtime bugs while using get_it_mixin

[escamoteur]

I think in the cases where you use the value of the field you are watching (almost always?), it would be good practice to encourage them to store off the value, otherwise it would be quite easy to forget to bind to the method and then create a bunch of hard to spot bugs.
eg, If you do it like this, nothing can break:

final count = watchOnly((UI data) => data.unreadCount);
final accent = watchOnly((UI data) => data.accentColor);

return Text(count, style: TextStyle(color: color);

Where as this is more brittle:

watchMultiple((UI data) => [data.unreadCount, data.accentColor]);

return Text(data.unreadCount.value, style: TextStyle(color: data.accentColor.value);

It is easy to omit the watch completely, and compiler would show no errors. You would only spot the error at runtime, when you notice things are not updating as expected.
Seems like saving a cpl lines here is not worth the increased likelihood for bugs,

@esDotDev @escamoteur It might potentially be possible to use something like https://pub.dev/packages/custom_lint to create lints to help users avoid situations where they can accidentally introduce some runtime bugs while using get_it_mixin

I might add, that if I want to update a lot of values from the properties of an object it's probably more readable to watch the whole object and return it.

[escamoteur]

cc) albertodev01

[escamoteur]

@esDotDev @kekland @dancamdev I would be interested in your opinion in #246 (comment)

[escamoteur]

Ok, decided to keep it tow seperate packages, but the mixin gets renamed to

Watch_it the simple state management powered by get_it :-)

and we will overhaul the function names

[esDotDev]

I think WatchIt is a great name! Don't see any real issues with your recommended approach. Would love to see these under a unified name w/ a simpler usage.

[escamoteur]

@esDotDev I will keep it two packages but watch_it will include get_it so you only have to import one package.
I will publish a proposal of the function names. They will be global functions so you no longer will need to add a separate mixin on a State of a statefull widget and it will additonal to the mixins include WatchingWidgets/ WatchingStatefulWidget for people who don't like mixins :-)

[esDotDev]

Sounds great! Looking forward to the discussion on new method names as well.

Seems like we could omit the GetIt.I and just end up with

final foo = watch<Foo>();
final bar = watchField((Foo f) => f.bar);

It would be nice if that could be all we needed, and it could handle the various supported types, like Futures, Streams, ValueNotifiers, but not sure if that is doable with dart currently.

[escamoteur]

I think that will be possible but it might be needed to pass some generic types in some cases.

As the necessity to have more than one GetIt instance is pretty rare,
watch_it will define a global variable di or get_it and all watch functions will use that. we could add an option last parameter to the watch functions where you can pass the get_it instance that should be used if someone wants to use a different instance

[escamoteur]

can be closed now with the release of https://pub.dev/packages/watch_it

[esDotDev]

Looks great! I'll make a blog post sometime soon on gskinner.com to try and raise a bit of awareness. Thanks Thomas!

[escamoteur]

That would be awesome. I used your Readme as base and extended it a bit. We switched today our whole project to it and the code looks so much better Am 15. Juni 2023, 20:46 +0200 schrieb Shawn ***@***.***>:

…

Looks great! I'll make a blog post sometime soon on gskinner.com to try and raise a bit of awareness. Thanks Thomas! — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you modified the open/close state.Message ID: ***@***.***>