lucavenir / go_router_riverpod

An example on how to use Riverpod + GoRouter
469 stars 70 forks source link

The riverpod_generator package does not support ChangeNotifier values #17

Closed agordeev closed 1 year ago

agordeev commented 1 year ago

In router.dart:

Screenshot 2023-03-11 at 15 12 54

This warning comes from riverpod_lint. The app works fine though.

lucavenir commented 1 year ago

Hi! These examples were written before the linter was implemented. I'd have to review this lint warning, but it looks like a false positive to me. I'll look up into it and I'll let you know.

lucavenir commented 1 year ago

Hi! I've talked with Remi about this just to be sure.

It is not a false positive and - alas - the linter is right. I didn't think about it, but go_router is a ChangeNotifier. See its definition:

class GoRouter extends ChangeNotifier implements RouterConfig<RouteMatchList> {
  ...
}

Thus, what the linter is trying is:

look out, if you inject this provider you will not get any updates (no rebuilds) because the generator doesn't support ChangeNotifier

Which is fine for this usage. In the end, you never want to directly inject a GoRouter instance around your code. That's why GoRouter APIs have a dedicated InheritedWidget (and its relative extension methods, e.g. context.go).

Nonetheless, I'll change the code and remove this at least in the codegen version. I don't want to spread bad practices AND honestly it doesn't really matter much; as I said above, router isn't injected anywhere else.

Thank you for this feedback, as you've probably helped tons of other devs with this potential issue. Closing and fixing ASAP.

lucavenir commented 1 year ago

Closed with this commit