Skip to content
This repository has been archived by the owner on Mar 23, 2023. It is now read-only.

Why not using the action to do update the stores? (higher compling) #67

Closed
totty90 opened this issue Sep 26, 2014 · 12 comments
Closed

Why not using the action to do update the stores? (higher compling) #67

totty90 opened this issue Sep 26, 2014 · 12 comments

Comments

@totty90
Copy link

totty90 commented Sep 26, 2014

Hy!

Currently I have an app that has a changed version of flux in which the action manages all the stores changes:

  • Some stores;
  • Communicates with the server;
  • Tracking;

I don't need to wait for stores (anyway flux's waitFor() is synchronous), I just put the things in the order. For now is scaling well, but I might wonder what would be the benefits of implementing a strict flux architecture (where the stuff is done in the register callback in each model)?

The first thing that can go wrong is having a lot of stuff in each action. But you have the benefit of knowing in one glance what is going on each action. In the strict flux architecture you would have to open each store and check it's relative action listener.

This is the real action, without any modification:

define(function(require, exports, module){

    var _         = require('underscore');
    var utils     = require('client/utils');
    var async     = require('async');
    var constants = require('client/constants');
    var tempShipIdCounter = 0;
    return function(args, cb){
        var logic = require('client/logic/logic');
        var _this = this;
        var data  = _this.ctx.data;

        var planet  = args.planet;
        var taskId  = args.taskId;
        var payload = args.payload;

        async.series({
            _: function(cb){
                var res = planet.getTask().startTask(taskId, payload, data.authStore.currentPlayer);
                if(res.err){
                    data.popupStore.addPopup('task', {
                        planet    : planet,
                        taskState : 'error',
                        err       : res.err
                    });
                    return;
                }
                _this.api('startTask', res.data, {handleError: true}).then(function(res){
                    if(res.err){
                        if(DEBUG){
                            debugger
                            return console.error(res.err);
                        }else{
                            return location.reload();
                        }
                    }
                })
                // We need to get from the res.data because the taskId can be undefined
                // when we auto-restart last task.
                taskId = res.data.taskTypeId;
                data.sounds.startTask.play(taskId);
                woopra.track('start ' + taskId)
                data.taskStore.trackPlanet(planet);
                data.popupStore.addPopup('task', {
                    added     : res.details.added,
                    task      : planet.getTask().toJSON(),
                    taskState : 'started',
                    planet    : planet
                });

                cb();
            }
        }, cb)
    };
});
@sterpe
Copy link

sterpe commented Sep 27, 2014

Open license; you're free to do whatever you want.

Personally, I'm finding the trick with React & Flux is how to model some particular state change such that you don't find yourself trying/needing/wanting to issue dispatches from inside component life cycle functions or otherwise circumvent the dispatch flow as you are doing here.

@totty90
Copy link
Author

totty90 commented Sep 27, 2014

@sterpe I'm not dispatching from the component. This code is an action from flux. So instead of making a global dispatch that stores listen to and do stuff, I do the stuff needed in the action and in this particular order, which is easier to read IMO. No need for waitFor. I'm only asking what are the benefits of using a strict flux architecture vs my version.

  • Flux flow: view->action->dispatcher->stores->dispatch->views-updated
  • My flow: view->action->stores->dispatch->views-updated

I'm only skipping the dispatcher from the view.

@briandipalma
Copy link

stores->dispatch->views-updated

eh, stores should be event emitters and that should be their mechanism to notify the views listening to them of updated, I guess that was typo?

@totty90
Copy link
Author

totty90 commented Sep 27, 2014

Yes, isn't that what I've said?

  • Store emits (dispatches) the change;
  • Views which listened to store update;

Store->dispatches->then the views update

@briandipalma
Copy link

Sorry I read it as another action being dispatched, which seemed wrong.

@sterpe
Copy link

sterpe commented Sep 27, 2014

@totty90, As the application grows, say you add ten more stores for planetary defense, ship drive efficiency, etc you will have to modify each "action" fn to call & update the particular store. Compare with using the dispatcher where you would only declare what actions this store updates against within the store itself.

@fisherwebdev
Copy link
Contributor

Please take questions about variations on Flux or quasi-Flux experiments to the React Google Group, rather than this repo.

@totty90
Copy link
Author

totty90 commented Sep 29, 2014

@fisherwebdev Ok.
@sterpe But in your case with 100 stores and 100 actions. If your flow is distributed between 100 of stores, how can you read the code? You will have to jump from store to store to see the sequence, then you should also check all the waitFor preconditions. In your case, where you have the code in the store you will have stores which handles lets say 100 actions (not real, just for the example). Which one is easier to debug/check the dependencies and check the flow of the program?

Continue here: https://groups.google.com/forum/#!topic/reactjs/krY5aiMk540

@briandipalma
Copy link

If your flow is distributed between 100 of stores, how can you read the code? You will have to jump from store to store to see the sequence, then you should also check all the waitFor preconditions.

No one sits down to just read an entire codebase, normally you want to focus on one concern and with stores that one concern should be in one place.

(not real, just for the example)

Exactly. Not real at all, what you are describing is a God object not a well factored store.

@totty90
Copy link
Author

totty90 commented Sep 30, 2014

@briandipalma even with my game example above, putting everything in each store is harder to maintain.

No one sits down to just read an entire codebase, normally you want to focus on one concern and with stores that one concern should be in one place.

Normally you don't need to read it all, but for example I would like to know what exactly happens when I click on a button. If you have unexpected behaviour you have to check all the stores for that particular action. If you have an action you read it all there. I can't see any use case / making my life easier about storing that logic in each store. But this is me. Have you developed any app with pure flux? Show me your code or part of it.

@briandipalma
Copy link

Following that logic you should put all your code in one file so you don't have to look at any other files.

If you have unexpected behaviour you have to check all the stores for that particular action.

Personally I wouldn't do it that way, I'd open up the view and then the store/stores that provide data to the view that appears incorrect and debug in that order. What you are doing in your pattern is making the stores mutable from outside. This negates one of the advantages of flux, as long as you are dealing with manageable interactions this seems fine but it's more difficult to scale with increasing complexity.

By the way I believe the action is simply an object literal with a type property, the action creator is the module/class with the API on it.

I think it's quite handy to decouple the action creator and the store processing the action as it allows users of your flux component to easy add their own stores without having to modify the action creator code. All they do is register to the dispatcher, while in your pattern they would have to register to the dispatcher and modify the action creator code. Also if you wanted to see what action a store responds to you would have to look at all action creators and see if they call the store. In essence the reverse of your "what stores are notified by what action" problem.

@totty90
Copy link
Author

totty90 commented Sep 30, 2014

Following that logic you should put all your code in one file so you don't have to look at any other files.

This is not the same logic. I've said to only keep in an action the whole flow. So when I look at an action I can see everything it does. Is not the same as putting all the code in the same file, as you can think.

All they do is register to the dispatcher, while in your pattern they would have to register to the dispatcher and modify the action creator code.

No, just change the action creator code.

Also if you wanted to see what action a store responds to you would have to look at all action creators and see if they call the store. In essence the reverse of your "what stores are notified by what action" problem.

That's also true. So the final question is: do you want to see what an action does or what a store responds to. Normally you want to know the flow other times you want to see what changes a store. Both can't be done together. For me is more useful to see what happens when an something triggers an action than knowing what changes a store.

This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants