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

What types of logic belong inside actions? #7

Open
mipatterson opened this issue Nov 4, 2018 · 2 comments
Open

What types of logic belong inside actions? #7

mipatterson opened this issue Nov 4, 2018 · 2 comments

Comments

@mipatterson
Copy link

mipatterson commented Nov 4, 2018

I have a question about what types of logic belong within actions.

In the services example, the HTTP call to save a new developer is made and awaited within the addDeveloper action and when the promise resolves, a new state is returned and the action is completed.

export async function addDeveloper(state: DeveloperState, category: DeveloperCategory, name: string, skills: string[]) {
  if (!category || !name || !skills) {
    return state;
  }

  const newState = Object.assign({}, state);

  // we await the Promise returned by the service method
  const res = await developerService.addNewDeveloper(category, name, skills);

  // depending on whether the currently active category is all or the one matching the new dev we'll add the new dev as well
  if (newState.activeCategory === category || newState.activeCategory === "all") {
    newState.developers = [ ...state.developers, { ...res }];
  }

  return newState;
}

At first, this seemed fine to me, until I realized that until the HTTP call promise resolves, no other state changes can be made, because the addDeveloper action will block any other actions from being handled. (They can be queued, but not actually handled.) So if the HTTP call to persist the new developer data takes a long time because of high load on the server, and during that time another unrelated action is dispatched, that unrelated action will not be handled right away. In some cases, this might not be a big deal, but it has the potential to create a poor user experience depending on what that queued action was supposed to do and what the consequences are of it being delayed.

Because of the way actions are queued and handled, it seems to me that all actions should be as fast as possible and that server calls should almost never be placed inside actions. For example, I would rewrite the above action as a series of actions, wrapped in a separate method:

function startSavingDeveloperAction(state: DeveloperState): DeveloperState {
  return {
    ...state,
    savingDeveloper: true,
  };
}

function completeSavingDeveloperAction(state: DeveloperState, newDeveloper: any): DeveloperState {
  return {
	...state,
	savingDeveloper: false,
	developers: [...state.developers, newDeveloper],
  };
}

export async function addDeveloper(store: Store<DeveloperState>, category: DeveloperCategory, name: string, skills: string[]) {
  // Validate
  if (!category || !name || !skills) {
    return;
  }

  // Dispatch action to update flag in state to indicate we are saving a new developer
  await store.dispatch("startSavingDeveloperAction");

  // Save the new developer via service HTTP call
  const res = await developerService.addNewDeveloper(category, name, skills);

  // Dispatch action to add new developer to state
  await store.dispatch("completeSavingDeveloperAction");
}

The above code takes more of a redux-thunk style approach to asynchronous actions, keeping the actions small and quick and chaining them together inside another function that has the ability to dispatch actions. I can definitely see the potential need for calling asynchronous code from within an aurelia-store action (perhaps the need to call into a third-party library), but it seems to me that in many scenarios this is not the right approach.

Could you clarify in which scenarios making actions asynchronous is the correct approach and what types of logic should be inside actions? Perhaps I am misunderstanding how aurelia-store handles these scenarios.

Thanks!

@Vheissu
Copy link
Collaborator

Vheissu commented Nov 5, 2018

I think it all comes down to preference. Developers seem to wrestle with this question in VueX, Redux and other state management libraries as well. Do you make API requests in your actions or do you move the logic outside?

I like making API calls inside of my actions. Sure, there is the potential to slow down your application with blocking calls, but it's something I manage by keep requests light, using caching where applicable and overall just trying not to do too much.

But then you run into different circumstances:

  • Does the user have a bad internet connection?
  • Has the user lost connectivity?
  • Is the API responding slowly?
  • Is the API not localised and you're contending with high latency?

I guess the difference between Aurelia Store and VueX, for example, is that in VueX actions are asynchronous, but mutations are synchronous and it is recommended you only make API requests in your actions. In Aurelia Store, mutations and actions are rolled into one.

Really it all comes down to what works for you and what doesn't. I am not doing anything overly complicated or big in my application, so I am willing to take the potential slowdown hit for the sake of convenience and less typing. But if performance is a concern, then your approach of breaking things up is definitely the way to go.

Maybe there is potential here to introduce an opt-in solution where mutations and actions are separate (like VueX) where actions can be async and mutations are sync. Definitely, something to think about.

@zewa666
Copy link
Owner

zewa666 commented Nov 5, 2018

@mipatterson excellent question, which is currently a lot under discussion. I would sum up and say, yes you're approach is better as it's more defensive. As @Vheissu explained if you keep the requests minimal with fast respones, you'll likely not notice anything. But working with external service, potentially cross-continents, it's easily possible you'll hit some high latency issues.

So in general if you want to be on the safe side, having a starting and ending action (splitting up the request and response-handling) is definitely the way to go. There is just no right or wrong per se as it heavily depends on the use-case, sometimes there are things you explicitely need to wait for and other times you just want to keep the UI responsive, but I think you summed it up pretty well.

Would you like to extend the service sample with another feature, e.g. deleting a developer, showing off your approach? In the comments you could address the difference from the addDeveloper approach, so we have both documented.

EDIT: If we have this, we should also update the store docs with a link to the specific feature to make people aware of this fact

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