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

Question: Referencing stores from actions #178

Closed
seanconnollydev opened this issue May 7, 2015 · 17 comments
Closed

Question: Referencing stores from actions #178

seanconnollydev opened this issue May 7, 2015 · 17 comments
Labels

Comments

@seanconnollydev
Copy link

I am currently using flummox in an application (and loving it so first and foremost thanks!) that has two main components:

  • A list of items
  • Filter selectors for those items

At a high level, clicking a filter needs to do the following:

  1. Initiate an AJAX call to the server to fetch the new list of items based on current filter selections
  2. Add a "selected" class to the filter to indicate that it is selected
  3. When the AJAX call returns, update the list of items

So in flummox terms I have an ItemStore and ItemActions. ItemActions has a searchItems() function and a few filter functions (e.g. filterName(), filterLocation(), etc.) The ItemStore state looks something like this:

{
  Items: [],
  Filters: {
    SelectedNameFilters: [],
    SelectedLocationFilters: []
  }
}

My reading of flux best practices indicates that the AJAX call should be initiated from action creators (i.e. ItemActions). It also seems to be recommended that actions not trigger other actions. In this scenario I have two problems:

  1. Within the filter actions (e.g. filterLocations), I'll need to first update the state of the filters in the store (e.g. SelectedLocationFilters) and then trigger the searchItems() action. I'm not exactly sure how I can even do this with flummox.
  2. The AJAX call in searchItems() needs to pass data related to the selected filters (e.g. SelectedLocationFilters), so I would need to reference the store. I'm sure its possible with something like flux.getStore("items"), but it doesn't seem right.

Any advice you can give on best practice would be most appreciated, thanks!

@johanneslumpe
Copy link
Collaborator

For 1: You could just call two actions sequentially after each other in your component in question.

class YourComponent extends React.Component {

  onSomeEvent(e) {
    this.props.flux.getActions('first').method();
    this.props.flux.getActions('second').method();
  }

  render() {
    /* ... */
  }
}

If you rather want these actions to be bundled (because maybe you want to call the same combination from multiple places), I'd suggest to use some kind of "meta actions" object, which bundles the calls to those methods. Basically a wrapping function around those calls, which you then pass to your component.

For 2) Don't access your stores inside your actions. Just get the currently active filters in your component. Possibly you will retrieve them from props. Since you'll trigger an ajax request when filters change, you will do that from within your filter component, which already needs all active filters in order to mark them as active I assume. Then pass them to the action creator directly.

@malte-wessel
Copy link

I think subsequent actions are not the right approach, have a look at these issues:
facebookarchive/flux#47 (comment)
facebookarchive/flux#185
malte-wessel/minimal-flux#1

@johanneslumpe
Copy link
Collaborator

As far as I can see it your first link is irrelevant to the question, as each action has its own dispatch cycle. This isn't "dispatching during a dispatch". Furthermore these two actions are independent of each other. Whether the request fails or not doesn't matter, so triggering them after each other shouldn't pose a problem. Care to elaborate a bit on your standpoint instead of just linking to other issues? ;)

@malte-wessel
Copy link

Sorry for beeing so short ;) Subsequent actions would cause two independent dispatch cycles leading the views to render twice. This is not a big deal, but if you got another action firing (e.g. a request comming back) between these two cycles you don't know what things have changed and whether you want to (or can) fire the second action. Therefore I think that read access to stores from actions, in order to decide if you need to make a request to the server, would be appropriate.

@johanneslumpe
Copy link
Collaborator

If one actions depends on the result of another action, then they shouldn't be triggered without knowing the result of the previous one, I agree. But this isn't the case here, as far as I can tell.

I - personally - would then rather have some kind of meta action object, which does the store lookup for me and then decides whether to trigger an action, instead of the action itself reading from the store.

@malte-wessel
Copy link

Yes, the Facebook guys suggest to let the WebAPIUtils handle that logic facebookarchive/flux#185 (comment)

@johanneslumpe
Copy link
Collaborator

Yeah, whether to do it in a meta action or directly in the apiutils is probably just a matter of taste/style :)

@seanconnollydev
Copy link
Author

So my current implementation initiates both actions from the component, just like Johanne's example, except it is a little funky because it looks more like this:

class YourComponent extends React.Component {

  onSomeEvent(e) {
    this.props.flux.getActions('items').filterBySomething();
    this.props.flux.getActions('items').search(this.props.flux.getStore("items").Filters);
  }

  onSomeOtherEvent(e) {
    this.props.flux.getActions('items').filterBySomethingElse();
    this.props.flux.getActions('items').search(this.props.flux.getStore("items").Filters);
  }

  render() {
    /* ... */
  }
}

And I will have more calls like this so I see the value of the WebAPIUtils or the meta action. Say I go with WebAPIUtils. Let me see if I'm following your recommendations with the assumptions below.

Since flummox does not use singletons, it means I will need to pass the WebAPIUtils instance explicit references to either the flux object or whichever stores/actions it needs to know about. I would then need to be able to reference WebAPIUtils from my action creators. So I think my WebAPIUtils would look like this:

class WebAPIUtils{
  constructor(flux){
    this.flux = flux;
  }

  search(){
    $.ajax({
      url: mySearchAPIEndpoint,
      data: this.flux.getStore("items").Filters,
      success: this.flux.getActions('items').searchCompleted
      error: // do something else
    });
  }
}

And my component would be updated to look like this:

class YourComponent extends React.Component {

  onSomeEvent(e) {
    this.props.flux.getActions('items').filterBySomething();
  }

  onSomeOtherEvent(e) {
    this.props.flux.getActions('items').filterBySomethingElse();
  }

  render() {
    /* ... */
  }
}

But now my problem is that I'm not sure how to accomplish "The action creator calls the WebAPIUtils module after dispatching the action." as mentioned in this comment

import { Actions } from 'flummox';

class ItemActions extends Actions {

  constructor(webAPIUtils){
    this.WebAPIUtils = webAPIUtils;
  }

  filterBySomething() {
    this.WebAPIUtils.search(); // This isn't right, because I want this to happen *after* the Filters are set in the store...
    return {};
  }
}

So I think I'm missing something, so far this seems far more complicated than just calling the two actions from the component.

@AndrewRayCode
Copy link

I have a similar problem I'm not sure how to reconcile with flummox. In my (action creator, non flummox) app, I have something like:

createAndEdit(data) {
    createThing.dispatch(data);
    var newestThing = thingStore.getNewest();
    editThing.dispatch(newestThing);
}

Basically I have an action creator that creates an object then edits it (which updates the UI). However, I can't see how this plays with flummox. I have an action creator so I can trigger two actions. I also need to access the store to know which thing to edit.

There are cases where I wouldn't want to edit the newest thing, which is why my store wouldn't respond to createThing to set the editing state to true.

@hakanderyal
Copy link

If I'm understanding correctly, async actions is your answer.

Using babel.js, es6:

store:

import { Store } from 'flummox';

export default class SearchStore extends Store {

  constructor(flux) {
    super();

    this.registerAsync(flux.searchActionIds.searchStuff, this.beginSearch, this.successSearch, this.failSearch);

    this.state = {
      result: [],
      status: 'null',
      filters: []
    };
  }

  successSearch(response) {
    this.setState({
      result: response.data,
      status: 'success'
    });
  }

  beginSearch(filters) {
    this.setState({
      result: [],
      status: 'searching',
      filters: filters
    });
  }

  failSearch(response) {
    this.setState({
      result: response.data,
      status: 'failed'
    });
  }

}

action: (axios is an ajax lib, axios.get returns a promise, use whatever library you want)

import { Actions } from 'flummox';
import axios from 'axios';

export default class SearchActions extends Actions {
  constructor() {
    super();
  }

  async searchStuff(filters) {
    return axios.get("yourSearchUrl", filters));
  }
}

Store gets updated with filters at the beginning of the request, and gets updated with results at the success/fail.

@AndrewRayCode
Copy link

My issues are that once one action completes, I need to read from the store then fire another action. AKA doing work in an "action creator" which flummox doesn't seem to embrace.

I need to access the store from an action, and trigger multiple actions from one place

@bdowning
Copy link

Each "action creator" implicitly dispatches with its return value. So if you want to implement "custom dispatch" as in:

createAndEdit(data) {
    createThing.dispatch(data);
    var newestThing = thingStore.getNewest();
    editThing.dispatch(newestThing);
}

you could:

createThing(data) { return data; }
editThing(newestThing) { return newestThing; }
createAndEdit(data) {
    this.createThing(data);
    var newestThing = thingStore.getNewest();
    this.editThing(newestThing);
}

which is fundamentally equivalent, as Flummox action creators that don't return (or return undefined) don't dispatch.

But for cases where you'd be doing:

asyncThing(data) {
    this.asyncThingStarted();
    asyncStuff(function (err, data) {
        if (err) { this.asyncThingError(err) }
        else { this.asyncThingSuccess(data); }
    });
}

you should promisify asyncStuff and use Flummox's async action support; it's really helpful!

@bradobro
Copy link

But @bdowning, is it safe to call action creators from within other action creators? From your second snippet...

createThing(data) { return data; }
editThing(newestThing) { return newestThing; }
createAndEdit(data) {
    this.createThing(data); 
    var newestThing = thingStore.getNewest();
    this.editThing(newestThing);
}

...this.createThing) isn't really an action creator--just something that will be wrapped into an action creator when instantiated by the flux contstructor's call to this.createActions(). Not sure here. Maybe Flummox.createActions() fixes that all up.

I'd like the docs to address this. Actions triggering actions can often be a smell, something to move up into your UI, but I do think there are times when it needs to be done, and it'd be nice if there were a clean, documented way to approach it. I have had action dependencies cause React to complain about mutable DOM.

I'd be glad to edit the docs, but I'm not sure on the best approach.

@bdowning
Copy link

Flummox dispatches at the "edges" of its Actions. It's impossible to get into a dispatch loop just by having Actions call Actions, even if they are async. You'd have to call an Action from a store handler or a store-triggered component change to cause a loop. My snippet is exactly the same as what in another Flux would be:

createThing(data) {
    Dispatcher.dispatch({action: Constants.CREATE_THING_ACTION, data});
}
editThing(newestThing) {
    Dispatcher.dispatch({action: Constants.EDIT_THING_ACTION, newestThing});
}
createAndEdit(data) {
    this.createThing(data); 
    var newestThing = thingStore.getNewest();
    this.editThing(newestThing);
}

...which I don't think would be considered a smell, just a different way of structuring code.

For a more complex example, I have:

    async startOrderEditor(orderId) {
        await this.flux.getActions('tix').fetchOrders({
            id: orderId
        });
        let order = this.flux.getStore('orders').getOrder(orderId);
        await this.flux.getActions('tix').fetchEvents({
            id: order.eventId
        });
        return { orderId };
    }

...which is perfectly safe against dispatch loops and ensures that everything is loaded and ready for the UI by the time the final action (startOrderEditor_Finished) fires, and isolates all async stuff in this Action(-Creator) (and away from my stores).

Maybe it's not idiomatic Flux but it's the best and least-code-duplication way I've found to get done what I need.

@bradobro
Copy link

Thanks @bdowning. I dug into the flux code a bit and this approach looks good to me too.

@johanneslumpe
Copy link
Collaborator

@sconno05 is your problem solved? :)

@seanconnollydev
Copy link
Author

Lots of good suggestions here, so I think we can close this one.

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

7 participants