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

Triggering discrete actions (via namespace or something similar) from a store? #158

Closed
kwhitaker opened this issue Dec 5, 2014 · 27 comments
Labels

Comments

@kwhitaker
Copy link

It's entirely possible that this is an architecture question, and not a functionality question. I'm coming from a backbone background, where we can namespace events coming out of a model:
this.trigger('namespace:eventName', args);

That namespace allows me to have various parts of an app share a model, but only take an action if they are listening for that particular eventName and namespace. Is there something like this in Reflux, or am I missing something in the inherent architecture here? All of the examples have a store firing a trigger event, with the entirety of its data.

For example, if I have an AppStore, and it hold the states for a few things (modal visibility, which menu is open). My modal component only needs to care about whether there's some modal data, my menus only which menu is active, etc.
Thanks.

@spoike
Copy link
Member

spoike commented Dec 7, 2014

I'm using reflux in a browserify build. So namespacing is enforced by commonjs modules.

Also the stores can be split up any way you want, you don't need to have an AppStore that handles all of the data. I find that to be a god class/big ball of mud anti-pattern.

In your example you may create a MenuStore that takes care of the data for the menu, ModalStore that takes care of the data for modal visibility. I remember in some discussion in another github issue referring to one store per component, which is partly true. Components may listen to more than one store as well, reacting to data coming in from several stores.

Do note that Stores may listen to events from other stores (since they share the same listener mixin interface with actions), they only need to do this.listenTo(store, callback) in order to get those events. This is the most flexible and reactive way for stores to share data that changes. I say flexible, because I personally find it easy to add new features as new stores that listen to ones that exist rather than continue building a store. I'd like to think it as Open-Closed Principle (one of the SOLID principles). Stores are open for extension (by listening to their change event) but closed for modification (by creating new stores).

Hope this makes sense for you.

@spoike spoike added the question label Dec 7, 2014
@kwhitaker
Copy link
Author

I see where you're coming from with the one component-one store architecture, though I admit it seems a bit heavy/verbose in its own way. I think I just need to wrap my head around the way React and Reflux do things, as opposed to the way in which Backbone et. al. do them.

Thanks!

@yonatanmn
Copy link
Contributor

I must say that this problem seems like something that need to be changed in architecture.
Let's say I want to have a button that displays the price of a stock in the time of pressing, and not changing until I press again. now, for another component I want similar button - but I don't want the other to change its state. I won't duplicate the entire store for this!

Stores aren't listening to views - they listen to events, same should be the other way around:
Stores trigger actions, and view listen to those actions. this way everything would be super-flexible.

In the meantime, since I've encountered similar problem in my project, I'm doing something similar to what I've suggested:

button1 -> action.getPrice('button1')
button2 -> action.getPrice('button2') 

store -> 
    onGetPrice(identifier) {ajax.done. trigger(identifier, data) }

button1View -> 
    Reflux.listenTo(store,"changePrice")
    changePrice(identifier,data) { 
        if(identifier==="button1") { 
            this.setState({price:data})   }}
//same goes to the other button. Just different IF statement 

This method could be generalized in other ways. This is kind of a workaround for something needs to implemented in the architecture to my opinion

╔═════════╗.......╔════════╗....╔═════════╗....╔═════════════════╗
║ Actions......... ║──>║ Stores........ ║─>║ Actions........ ║─>║ View Components.............║
╚═════════╝.......╚════════╝....╚═════════╝....╚═════════════════╝
^ ............................................................................................................. │
└────────────────────────────────────────────┘

@gregory90
Copy link

+1.
Code gets ugly when you want to listen only for specific action/actions without creating new store. Triggering discrete actions and listening to them in views would increase flexibility. I'd like that.

@spoike
Copy link
Member

spoike commented Dec 16, 2014

I fail to see what "discrete" actions are really. Either that or people seem to think each action should only do one thing... in that case it's a misnomer. Actions should be invoked with some kind of context so that e.g. stores know what to update.

@gregory90 View components are already able to listen to actions if you'd like to do that. Actions and stores have the same listen interface. I sometimes shortcut Action to Components because it is a lazy way to do simple eventemitting as long as the app in question is small (I wrote a chrome extension that does this).

In projects I've worked on so far, data stores handle application view logic, mostly arrays/lists as main data structure. So updating the whole view hierarchy with subcomponents shouldn't be much of a hassle as long as you expose the necessary data through props.

@yonatanmn You might need to do a more data driven approach. I assume you have a StockItem component. It should get data through props, such as the stock's identifier (e.g. "AAPL") and the current price, from the parent StockList component. The StockItem should have a generic StockRefreshButton, which gets the stock's identifier through props. The onClick for StockRefreshButton should call an action like this updateStockFor("AAPL").

Something will listen to this updateStockFor action, like a store that handles the data for all stocks loaded in the view, and do some ajax mumbo jumbo to get the stock for AAPL. The change event that it emits will update the StockList and magically all the subcomponents should update accordingly through props.

The flow diagram would look something like this:

updateStockFor ----> StockViewStore
    ^                      |
    | 1. onClick invokes   | 2. store updated list with
    |    action with       |    new data for `AAPL`
    |    id `AAPL`         |
    |                      v
    |                  StockList
    |                      |
    |                      | 3. price change propagate
    |                      |    through props
    |                      |
    |                      v
    |                  StockItem
    |                      |
    |                      | 4. no real change here,
    |                      |    id should still be same
    |                      v
    |              StockRefreshButton
    |                      |
    +----------------------+

StockRefreshButton can be reused as long as a stock identifier can be supplied through props. The button itself should be fairly simple to implement:

var StockRefreshButton = React.createClass({
    invokeAction: function() {
        updateStockFor(this.props.stockId);
    },
    render: function() {
        return (<div className="refreshbutton" onClick={this.invokeAction}>Refresh</div>);
    }
});

Hopefully this makes sense.
// Usage:
<StockRefreshButton stockId={stockId} />

@gregory90
Copy link

@spoike My post wasn't clear. What I meant is listening to individual action triggers from stores. Let's say I have list of dogs, form for adding dog, and button to delete each dog. Typical CRUD. I also have toast notifications. So I'd have DogActions("addDog", "removeDog" etc.), DogStore, DogListComponent and ToastComponent. What I want to achieve is:

  1. After dog is successfully added/removed - add/remove element to/from list (DogListComponent listens to addDog and removeDog),
  2. After dog is successfully removed - show notification(ToastComponent listens to removeDog) and if not - show error notification.

Right now I can listen to action in view component, but then it's triggered independently from store(which is ok), so it's partial solution.
Second option would be namespacing trigger call. Then it'd be for example in DogStore.onDelete this.trigger('onDelete', dog) and in my ToastComponent I'd use if statement to check if triggering function is really 'onDelete'. It seems hacky though.
Third option is to create new store for this specific situation, but then number of stores significantly and fast increases. Not pretty solution either.

Current approach works really well when you have to manage arrays/lists, but not so much when you need more control over what happens after specific action is handled by store.

That said, I'm just a beginner in React/Reflux - maybe I'm missing a bigger picture.

@yonatanmn
Copy link
Contributor

@spoike I don't think we are on the same page - perhaps I wasn't clear enough.

Here's a real world and hopefully a better example...

Let's say I have a video player on which I can play basketball matches. When I load it up, I get a list of games I can choose to play from. I can filter my list, and then choose a game to watch. The list of games is the basketballGames store. The filtering is done on the server, i.e., each search will result in an update of basketballGames, and a different list of games in the store.

Next to the video player, there is a list of moments played out throughout a game - like shots, fouls etc. This list is not bound to which game I'm currently watching. I can choose any of the available games from basketballGames to get a list of moments specific to that game.

The conflict here, is that if I filter the first list - the one for choosing a game to play, the second list would get filtered too – and that would limit the amount of games I can browse the moments of! That doesn't make sense, because they are separate components and the only thing they share is similar data (namely, way of getting this data) - but their functionality is totally unrelated.

How would you solve this? Thanks

@spoike
Copy link
Member

spoike commented Dec 17, 2014

@gregory90 It seems to me that the ToastComponent involves another data flow. I'd make a ToastMessageStore that listens to a addToastMessage action rather than individual actions. This means the stores need to invoke the addToastMessage action with relevant message (and optionally with a undo command callback as by example by google toast component design spec if you want the actions that have been taken to be undoable). In this case as soon as the dog has been removed either the ajax module or the store needs to send off the toast message. I'd say the store since it reasonably knows best how to create an "undo" callback.

@yonatanmn So you basically need a store that holds the context of what match is selected, e.g. SelectedMatchStore, which has the selected match loaded and also a list of moments for that match. This store shouldn't be updated by the list of basketball matches, only if the user decides to select a match (which should be an action by itself, e.g. selectMatch, or determined by a route in react-router). Just because it's similar data doesn't mean it's the same in different contexts.

Hope this makes sense.

@gregory90
Copy link

@spoike That's exactly how I'm managing such scenarios right know. It just didn't felt right to invoke action from the store. Thanks for your comments!

@Janekk
Copy link

Janekk commented Dec 17, 2014

@spoike, @gregory90 So if understand you correctly it is ok to invoke explicit actions from stores and not to base only on the trigger() method of a store? I guess it would be easier then to distinct in the listener callback (in component or in another store) what has actually changed in the store.

@spoike
Copy link
Member

spoike commented Dec 18, 2014

@gregory90 @Janekk Sure. I see no problems with invoking actions in stores. Stores are meant to emit events when data changes, other "kinds" of events are implemented as actions instead. E.e. I'm doing this in ajax stores to initiate new data flows, to be more specific: to update other dependencies as an application "heartbeat" or sending errors for toast notifications and google analytics.

In my mind it's easier to think of actions as entities that initiates a data flow and thus exceptional flows (such as errors) are seperate data flows which should be initiated by an action.

It is flexible enough for components since they may listen to actions as well. You may want to short circuit the flows by skipping creating a store if the data flow is stateless and data is passed as argument to the action. This also works well for spikes and other kinds of exploratory coding.

@Janekk
Copy link

Janekk commented Dec 18, 2014

@spoike - thanks for your insights, I like this approach of handling exceptional flows by invoking an action from the store.

What would you suggest in the following scenario: let's say that we have a store with a state containing a few different properties which resolution is handled by the store. These properties cannot be easily splitted between separate stores because their resolution is interdependent and would cause circular dependencies between stores.

There are also a few components which listen to changes of some subset of these properties (maybe only one or more of them). Now we want to notify the components about respective changes. It may be that only a subset of properties has changed and not all components are interested in it. What in your opinion would be the best/cleanest approach in this case:

  • invoke explicitly named actions only with the relevant part of store's state?
  • use the trigger() method only with the payload containing a part of data that has actually changed and then check in the component if data they're interested in has been sent (propably not the cleanest solution?..)
  • trigger() each time with complete store state and leave the resolution if the data has changed to components (for instance by using immutability helpers..)
  • some other approach?

@spoike
Copy link
Member

spoike commented Dec 19, 2014

@Janekk

invoke explicitly named actions only with the relevant part of store's state?

I think it is more useful to think about what payload is needed for the actual data flow. So... yeah. It needs relevant parts for any store that may listen to the action (since it is fairly possible in a larger system for several stores to listen to the same action). I don't really distinguish actions as explicit or discrete... they start data flows that may branch off to other data flows with their own payloads.

use the trigger() method only with the payload containing a part of data that has actually changed and then check in the component if data they're interested in has been sent (propably not the cleanest solution?..)

I don't do this often but only sending "visible" or changed is a performance optimization if you need to do that. But I'd profile first with the whole picture before refactoring it down to this, since as you guessed, it gets quite hacky very quickly. React already handles dirty checking in it's internal component DOM before it pushes it to the browser's DOM, so there usually is no need to do it.

trigger() each time with complete store state and leave the resolution if the data has changed to components (for instance by using immutability helpers..)

Yes. So my recommendation is try first with sending all of the data, measure the performance (preferably using a profiler such as the one in your favorite browser's developer tools), and change the implementation to do it pieces instead if necessary. Usually it's quicker than what you believe it would be, at least that's been my experience.

some other approach?

You may also split up the store into several ones if you can do the same with the data it handles. The benefit is having a more domain driven approach on your stores, if that's your thing. That's what I can think of from the top of my head.

@yonatanmn
Copy link
Contributor

@spoike, your answer still won't work. Even after selecting a match, the filtering of moments is done from the server, and I can not store it locally. Anyway, this is just an example. There will be many cases where one store will update two distinct components, where one should not change the data view of the other.
But, luckily, the architecture I have suggested is apparently something already implemented in reflux, as discussed above.
For naming convention, I use actions.TriggerName (similar to trigger), and then my view could listen to store only when I ask. Great!
I believe this wonderful option should be stated explicitly in the readme file, since, as you can see in this discussion, many weren't aware of that

@spoike
Copy link
Member

spoike commented Dec 19, 2014

@yonatanmn What do you mean "not store it locally"? Sure you can store it on the client-side, either:

  • cached in a variable
  • in local/session storage (has >IE8 support)
  • in WebSQL

Even if it is derivative data, you can still cache the previous one.

@yonatanmn
Copy link
Contributor

@spoike, the data is constantly changing. I want to get new data in the game section from the server using the store. That's what I mean by 'not saving locally '. And again, this is just an example.

@spoike
Copy link
Member

spoike commented Dec 19, 2014

Oh, okay. 👌

@lucsky
Copy link

lucsky commented Feb 3, 2015

+1

I would love to be able to distinguish different store update events which in my case cannot be done by simply using multiple stores. Simplified to the max, my use case is that I have two integer values which can vary independently, both increase, both decrease, increase/decrease or decrease/increase, and each of these case would ideally trigger a different event. Right now I'm just adding a parameter to the this.trigger() call but it obviously forces to test it on the handler side, which is ugly and which is what Reflux actions were supposed to prevent on the other side of the store :)

@yonatanmn
Copy link
Contributor

@lucsky , just use the solution offered above -
instead of this.trigger(),
register an action called TriggerSomething
in your store call :

Actions.TriggerSomething();

and in your component (or elsewhere)

componentDidMount: function () {
        this.listenTo(Actions.TriggerSomething, this.doSomething);
    },'

if you want other update from the store, register other action.

I'm building medium-large app now, and I found out that it's much easier to have limited number of stores, and many different trigger actions in each one.

@lucsky
Copy link

lucsky commented Feb 3, 2015

@yonatanmn Yes, you're right of course, I could totally do that and probably will. But it raises the following question then: what's the point of the trigger() method if a single action could replace it to begin with ? :)

@yonatanmn
Copy link
Contributor

@lucsky if you have a simple case, it just easier to listen to the store without registering an Action.
The mechanics are otherwise the same (I think...)

@jesstelford
Copy link

I found this discussion exceptionally informative. Thanks to everyone involved.

As a result, I have created a mixin for handling the example @yonatanmn gave:

// triggerable.js
module.exports = {
  init: function() {
    if (!this.triggerables) {
      return;
    }

    // attach the given action names as actions on the store
    this.triggerables.forEach(
      function attachAction(actionName) {
        this[actionName] = Reflux.createAction();
      },
      this
    );
  }
};

Usage:

// myStore.js
var Reflux = require('reflux'),
    triggerableMixin = require('./triggerable');

module.exports = Reflux.createStore({

  mixins: [triggerableMixin],

  listenables: {
    something: Reflux.createAction()
  },

  // The mixin turns each of these into a named action on the store.
  triggerables: [
    'somethingHappened'
  ],

  onSomething: function() {
    this.somethingHappened.trigger('foo');
  }

});

Later, in a component, etc, you can listen to this event:

var myStore = require('./myStore.js');

myStore.somethingHappened.listen(function(what) {
  console.log(what + ' happened');
});

Hopefully this helps someone else in the future who is struggling with the same situation.

@yonatanmn
Copy link
Contributor

Looks cool! I'll give it a try

@yonatanmn
Copy link
Contributor

Hi @spoike , I'm going to add Reflux option to react-webpack yeoman generator, and I think I will add @jesstelford's mixin.
What do you think about adding it to Reflux directly? it will be much more elegant (and I believe also useful.)

@jesstelford
Copy link

@yonatanmn / @spoike I have created an npm module out of my code from above (complete with a couple of enhancements, tests & docs): https://www.npmjs.com/package/reflux-triggerable-mixin

$ npm install --save reflux-triggerable-mixin

@apreg
Copy link

apreg commented May 10, 2015

Here is another example: imagine there are multiple auction cards in a gridview. Each card represents a different auction of course. The current price and some other attributes of the auction is coming from server through websockets. There is an AuctionStore which provides live data by trigger(). The AuctionCards connect to this store. The problem is this way every AuctionCard component will show the last data received. I could create somehow multiple actions dynamically per AuctionCard but it feels hacky and I don't know how to do that yet. What about one store per AuctionCard or event channel? There must be a better way than every listener gets the event then decide if it is for him by checking the payload.

@AnSavvides
Copy link

@jesstelford's mixin works really nicely :)

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

No branches or pull requests

9 participants