Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide a way to use StoreBuilder/StoreConnector without using StoreProvider to get the Store #109

Closed
dalewking opened this issue Feb 27, 2019 · 16 comments

Comments

@dalewking
Copy link

dalewking commented Feb 27, 2019

StoreConnector and StoreBuilder (which just wraps a StoreConnector) force you to use StoreProvider to get the store they are going to listen to. But there are times when you already have a reference to the store at the time you are constructing the StoreBuilder or StoreConstructor and having them use StoreProvider to get the store is just extra work.

For example, you might have a StoreBuilder with rebuild on change set to false because you have some buttons that need to call store.dispatch, but next to them you have a widget that needs to rebuild based on changes to part of the state. You want to only rebuild that widget when that particular part of the state changes and that is the only widget that needs to rebuild. So you wrap that widget in a store connector with distinct on. But having that StoreConnector use the StoreProvider to find the Store is unnecessary work.

I am also looking at this because I am looking at structuring my redux so that there is a global Store with the state for the entire app and also the ability to have child Stores that are specific to a view/page and that Store only exists while that view is in existence and goes away when that view/page goes away. In this case the Store is passed to the constructor of the view/page. Having to create a StoreProvider to provide the Store is extra hassle since the Store is stored in the state of the view/page. One of the justifications for this hierarchical state is that you might have multiple of these views on screen at the same time and they each need their own state.

So one way to handle this is to have an optional store parameter on StoreBuilder/StoreConnector that if set is the store that is listened to. If it is not set use StoreProvider.

Or alternatively it could be a storeProvider parameter which is function from BuildContext to Store and it has a default value which is to call StoreProvider.

Alternatively, I would be equally satisfied if _StoreStreamListener were simply made public, since StoreConnector is nothing more than a shallow wrapper around _StoreStreamListener that only adds the call to get the Store from StoreProvider and some asserts on parameters.

@brianegan
Copy link
Owner

Hey there -- good suggestions! Totally open to making one or both of these changes. Just to discuss the reasoning, I'm wondering a bit about this part:

there are times when you already have a reference to the store at the time you are constructing the StoreBuilder or StoreConstructor and having them use StoreProvider to get the store is just extra work.

Do you know if the current method has caused any perf issues? The mechanism used to find the Provider is the same mechanism used to find other important Flutter widgets, such as Theme. It is a core part of Flutter, and the team has done work to make sure it's fast. For example, there's a comment in the docs for the method being used: "Calling this method is O(1) with a small constant factor."

Therefore, if this is only for performance, I don't know if it's that necessary?

Having to create a StoreProvider to provide the Store is extra hassle since the Store is stored in the state of the view/page. One of the justifications for this hierarchical state is that you might have multiple of these views on screen at the same time and they each need their own state.

This part is interesting, and certainly a compelling reason to make the change!

So one way to handle this is to have an optional store parameter on StoreBuilder/StoreConnector that if set is the store that is listened to. If it is not set use StoreProvider.

This would be my suggestion as well -- this allows users to specify the exact store. It would solve your case, and be easy to use in a testing scenario as well.

Alternatively, I would be equally satisfied if _StoreStreamListener were simply made public

We could also do this as well if it helps solve your use-case! I kept it private since it felt like an implementation detail (how we setup all the callbacks and subscriptions), but happy to hear if it makes sense to make it public!

@dalewking
Copy link
Author

Therefore, if this is only for performance, I don't know if it's that necessary?

I certainly agree that if that were the only reason then it would probably not justify doing it.

This part is interesting, and certainly a compelling reason to make the change!

This notion of hierarchical state is something that I have thought long and hard about with flutter redux. I am working on a project now that has several pages including a couple different forms and having the validation logic for all these screens in one global state is messy. It would be nice to have scoped state. It also came up with a simple game I was working on where there are multiple levels and there is a view for the level that needs state when you are playing that level, but when you switch levels the old level is animated out while the new one is animated in so you need state for both levels to exist at the same time.

Since making this request I came across this article that talks about some of what I am talking about with the notion of Local State and Global State. His recommendation is Redux for global state and BLoC for Local State. I am simply trying to use redux for both.

This would be my suggestion as well -- this allows users to specify the exact store. It would solve your case, and be easy to use in a testing scenario as well.

I'm fine with any way of the solutions.

@brianegan
Copy link
Owner

brianegan commented Feb 27, 2019

This notion of hierarchical state is something that I have thought long and hard about with flutter redux.

His recommendation is Redux for global state and BLoC for Local State. I am simply trying to use redux for both.

Agreed, and think it's a totally sensible approach. It's interesting, the original Redux.js author Dan Abramov recommends putting everything into 1 store. But I don't really see a reason why you couldn't just split your store out into smaller parts that are responsible for more fine-grained components! In fact, I think it would be easier to reason about in a lot of ways.

The upside of global store: All data available everywhere, so it's easy to pull out the data. The downside: It requires a lot more coordination in your Mididleware and reducer layers, and it leads to more rebuilds / requires more care to prevent rebuilds. If the data isn't global / shared, I think it makes a lot of sense to break out individual stores!

I've got a couple of other issues on a couple of other repos that need some love before this one, but I'll try to get to it in the next week. If you need the functionality soon and have the time to contribute a fix, happy to review any PRs!

Thanks again for raising the issue :)

@doc-rj-celltrak
Copy link

doc-rj-celltrak commented Nov 15, 2019

Hi, I was just stopping by and wondering if there was any more traction on these ideas? My team is about to embark on a large Flutter app, very likely with Redux. One of the concerns is about needing to separate local state from global state by breaking out into individual stores, and how to do that without messing up the single store principle. I'm new to Redux and don't really know how large JS apps deal with this question. Tip number 3 here helps me a bit though. @dalewking I saw you have a similar situation.

@dalewking
Copy link
Author

dalewking commented Nov 15, 2019

I unfortunately have only been dabbling in Flutter, but I think I have been able to handle the design of this. Revisiting how I did this, I have multiple ReduxStore types. This was for an app I started, but didn't finish. I had an overall app store that basically just had a single UI visible state of whether we were logged in. I had a store that handled local state for the login screen including the current values for what they had entered, validation functions and state whether login failed. I also had a store for the dashboard once you logged in. If I had created other screens I would have created state local to them.

So take the login screen. The app state middleware had a ShowLogin action, it would actually create the instance of the store for login state (after consulting shared preferences to get the saved user name). That instance of the store is passed to navigation.

For navigation I try to separate UI from redux. I don't want redux having a dependency on UI so I create an abstract class called ReduxNavigator which defines methods that the middleware can call to do navigation. So that class might look like this:

 abstract class ReduxNavigator {
     void showLogin(Store<LoginStateModel> store);
     void showDashboard(Store<DashboardStateModel> store);
     // Other navigation methods for showing dialogs and modals.
 }

The reference to the ReduxNavigator is stored in the App State.

The UI creates the implementation of that abstract class and passes it to redux in a start redux call that creates the application state and starts the ball rolling (need to investigate using dependency injection instead of passing it in like this). The important parts of that class for this discussion looks like this:

class AppNavigator extends NavigatorObserver implements ReduxNavigator {
  @override
  void didPush(Route<dynamic> route, Route<dynamic> previousRoute) {
    if (route.settings.name == Navigator.defaultRouteName) {
      Future.microtask(() => startRedux(this));
    }
  }

  @override
  void showLogin(Store<LoginStateModel> store) {
    navigator.pushAndRemoveUntil(
        MaterialPageRoute<Object>(
          builder: (context) => LoginPage(store),
        ),
        ModalRoute.withName(Navigator.defaultRouteName));
  }

  @override
  void showDashboard(Store<DashboardStateModel> store) {
    navigator.pushReplacement(
      MaterialPageRoute<Object>(
        builder: (context) => DashboardPage(store),
      ),
    );
  }

So that gets kicked off by the App widget:

final GlobalKey<NavigatorState> _navigatorKey = new GlobalKey<NavigatorState>();

class MyApp extends MaterialApp {
  MyApp()
      : super(
          navigatorKey: _navigatorKey,
          theme: ThemeData(primarySwatch: Colors.blue),
          routes: {
            Navigator.defaultRouteName: (context) => LoadingPage(),
          },
          navigatorObservers: [AppNavigator()],
        );
}

So that pushes the default route (a loading page) which gets observed by the App Navigator which then starts redux which then will cause the showLogin method to be called with the local store to use for that page.

The login page widget then looks like this:

class LoginPage extends StatefulWidget {
  final Store<LoginStateModel> store;

  const LoginPage(this.store, {Key key}) : super(key: key);

In the case of the login page I don't actually need a StoreBuilder or StoreConnector, but my login page state references the state from store as widget.store. So for example in my initState I have this line:

_usernameController = TextEditingController(text: widget.store.state.username);

In the case of my Dashboard page I do use multiple StoreConnector. So similarly when a login is successful the app state middleware in response to a LoginSuccessful action will create the instance of the dashboard state and pass it to the showDashboard navigation method that creates the dashboard page:

class DashboardPage extends StatelessWidget {
  final Store<DashboardStateModel> store;

  DashboardPage(this.store);

  @override
  Widget build(BuildContext context) => StoreProvider<DashboardStateModel>(
        store: store,
        child: Scaffold(...

So I use a store provider at the top of the page.

One thing I forgot to mention is that these child store states all contain a reference to the parent store:

class DashboardState implements DashboardStateModel {
   final Store<AppState> parentStore;

This reference is so they can query the parent state, to get the reference to the ReduxNavigator and to delegate actions to the parent. In the login state middleware middleware list I have this at the bottom to delegate all actions to the parent:

 (store, action, next) => store.state.parentStore.dispatch(action),

In the case of the dashboard the only event that would need to be delegated is log out so I have this:

  TypedMiddleware<DashboardState, Logout>(
    (store, action, next) => store.state.parentStore.dispatch(action),
  ),

to make it easy to get a reference to the navigator I create this in the child states:

ReduxNavigator get navigator => parentStore.state.navigator;

Hopefully that points you in the right direction and I would be happy to discuss it further (though this is probably not the best place).

@doc-rj-celltrak
Copy link

This is extremely helpful @dalewking. Thanks, I appreciate the thought and time you put into this. I see your gmail, so I'll probably shoot you a few more questions there.

@dalewking
Copy link
Author

I should probably find some simple multiscreen app example to implement to demonstrate the method and write a Medium article on it.

@dalewking
Copy link
Author

dalewking commented Dec 13, 2019

@doc-rj-celltrak So after that long explanation, I would no longer recommend the approach. I was storing things in the state only to make them available for the middleware, and then having to link child state to parent state to access them as well, but that is really the job of dependency injection.

I have a new example, which is a coding exercise I did as part of applying for a job that I think is a better approach: https://gitlab.com/dalewking/weaver_app

But I am actually looking to move away from redux, but not sure where to yet. Not a huge fan of BLoC per se. The trouble with it is what it even means. Is there a BLoC per screen, does a screen have multiple BLoCs. Lots of contradictory info.

Been trying to look at RxVAMS and/or RxCommand but there isn't a lot out there on it.

I am coming to the conclusion that none of these approaches are what should be what the UI interacts with. I am thinking the UI should be given an instance of an abstract ViewModel class that contains abstract methods for invoking changes of state (the dispatch side of redux), getters for Observables of state, and in some cases query methods to ask questions that do not depend on state (e.g. a view model for a form might have a method bool isEmailValid(String email)). The implementation of that ViewModel could use redux or BLoC or whatever, but the UI should not be exposed to that implementation detail.

@doc-rj-celltrak
Copy link

doc-rj-celltrak commented Dec 13, 2019

Thanks a lot @dalewking, I will take a look at your example for sure. Curious why you're moving away from Redux, but I'll follow up later in an email. My problem with BLoC is that the structure is much less defined (outside of the popular flutter_bloc library) and not great for larger projects. Even with the library, there's no provided way or template for inter-BLoC sharing, as far as I know.

@dalewking
Copy link
Author

@doc-rj-celltrak See the edits I made to my comment

@dalewking
Copy link
Author

@doc-rj-celltrak Also my example shows some things you can do with the new extension methods.

@dalewking
Copy link
Author

@doc-rj-celltrak Just discovered async redux which seems to address many of the issues with normal redux, such as lots of boilerplate, complexity growing exponentially as middleware grows, navigation, error handling, sending events to the UI.

https://pub.dev/packages/async_redux/

@doc-rj-celltrak
Copy link

@dalewking We were impressed with async_redux when we tried it out, but we decided to stick with flutter_redux. Here's some notes that I took at the time: "We're mostly convinced to go with flutter_redux, because of its maturity, large user community, maintenance and support of the libraries, and tooling around it. The async_redux package looks promising in many ways, but the community and tooling aren't there yet. Also, it does away with middleware, but this piece has an important role in Redux that we may want to take advantage of." One piece of tooling that flutter_redux had going for it is the 'time travel' stuff when in development mode.

@dalewking
Copy link
Author

The support issue does not matter that much to me. These packages are both so simple it isn't that big of a deal. It is an issue with RxCommand though where there seems to be little knowledge or support or examples.

Regarding middleware though, that isn't true. It just blurs the lines and eliminates the formality and ceremony between middleware vs. reducer. Reducers can be synchronous vs. asynchronous. a synchronous reducer that simply returns a new state is the same as the normal reducer. You can write pieces that are pure middleware (that only do async stuff and only dispatch other actions, that would be an async reducer that calls dispatch and returns null (no state change). But it also lets you have an async reducer that also returns a new state to eliminate those cases where you dispatch an action simply to tell a reducer to change the state.

So if your only wrote synchronous reducers that did not call dispatch and only returned new state (or null for no state change) and asynchronous reducers that only called dispatch and always returned null (no state change) then you have exact equivalents to reducers and middleware.

What interests me primarily are the event pieces and error handling after struggled with trying to report errors to the UI to for example to have it pop up a dialog and seeing how trying to convey that through state doesn't actually work well. I could sometimes get multiple pop-ups at once. Events are something that could easily be used with normal redux however.

@doc-rj-celltrak
Copy link

@dalewking async_redux is still interesting to me, and we're open to switching to it later if needed. Thanks for your thoughts on it. I'm new to Redux overall, and middleware seems like a necessary add-on that somehow doesn't quite fit in with the whole philosophy. Then, async_redux takes things a step further by combining reducers and middleware and probably breaks the whole "reducers are pure functions with no side effects" thing, but I'm not too worried about that if it works well and is easily test-able. Please let me know how it goes if you use it, and if you have any good sample apps.

Also, we're still interested in the question of managing local versus global state, and details like how and where to do screen routing/navigation within the Redux framework.

By the way, Fish Redux might be interesting too. It's the architecture that Alibaba uses. Have you experimented with it?

@brianegan
Copy link
Owner

Closing this due to inactivity, please reopen if further discussion is warranted

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants