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

A better async/await actions usage? #1415

Closed
1 of 2 tasks
whinc opened this issue Nov 1, 2019 · 7 comments
Closed
1 of 2 tasks

A better async/await actions usage? #1415

whinc opened this issue Nov 1, 2019 · 7 comments

Comments

@whinc
Copy link

whinc commented Nov 1, 2019

Question

  • I've checked documentation and searched for existing issues
  • I tried the spectrum channel

I have read Creating asynchronous actions. The main reason we can't use async/await is that modification operations to state tree in async/await function cann't be detech automatically. so, i think we can write like below to continue use async/await by explicitly wrap operations in a action.I am not sure if there are potential side effects? If it's ok, i hope MST can provider natively the runInAction interface to the model type

  const Store = types.model({
        githubProjects: types.array(types.frozen),
        state: types.enumeration("State", ["pending", "done", "error"])
    })
    .actions(self => ({
        runInAction(action) {
          action.call(self)
        }
    }))
    .actions(self => ({
        async fetchProjects() {
            self.githubProjects = []
            self.state = "pending"
            try {
                const projects = await fetchGithubProjectsSomehow()
                self.runInAction(() => {
                    self.state = "done"
                    self.githubProjects = projects
                })
            } catch (error) {
                console.error("Failed to fetch projects", error)
                self.runInAction(() => {
                    self.state = "error"
                })
            }
        }
    }))
@ecklf
Copy link

ecklf commented Nov 8, 2019

Not sure if I understood right but this runInAction wrapping is already done if you use flow.

@qinhuaihe
Copy link

Not sure if I understood right but this runInAction wrapping is already done if you use flow.

😨...flow receive a generator function instead of an async function as it's parameter.

@beaulac
Copy link

beaulac commented Nov 23, 2019

Yes, but as mentioned in the docs, it's as simple as replacing async function x() {/*...*/} with flow(function* x() {/*...*/}), and await with yield.

The generator version of this action in the docs is shorter and simpler than the one using async+runInAction, and unlike async implementation it actually counts as 1 action (which it should!). This is an important part of why you shouldn't use the proposed async+runInAction pattern:

Imagine you have an onAction handler for your instance. If your code outside calls fetchProjects once, you would expect that onAction gets triggered once, but each runInAction step triggers an additional onAction (see this sandbox) which is not ideal / expected behavior.

This also breaks any notion of using actions as atomic transactions; see this sandbox.

I'm not sure why you would insist on using async/await inside the action... you can still use it as much as you want outside:

// in the model actions:
fetchAThing: flow(function* fetchAThing(thingId) {
	const thingFromServer = yield fetchThingFromServer(thingId)
	self.thing = thingFromServer
})

// ... somewhere outside the model:
async function fetchThingFromServer(thingId) {
	const response = await callTheServer(thingId)
	const thing = extractThing(response)
	return thing
}

@whinc
Copy link
Author

whinc commented Nov 24, 2019

@beaulac Thank you very much for your answers and examples. flow is able to keep the action transactional, which is enough to convince me to use flow instead of runInAction + async.

@whinc whinc closed this as completed Nov 24, 2019
@jonlambert
Copy link

jonlambert commented Dec 2, 2019

Also something worth noting is that unlike await, with TypeScript the compiler is unable to infer a type from a yield. It's not a huge issue but does mean it's not as simple as just swapping one for the other.

@beaulac
Copy link

beaulac commented Dec 2, 2019

Great point – it seems TS is a ways off inferring intermediate yields despite the improvements in 3.6.

As an alternative, it might be worth looking at actionAsync which allows async/await actions (so, with type inference)
for Mobx, though as as mentioned here by the author it would have to be re-implemented for MST.

@lock
Copy link

lock bot commented Jan 31, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs or questions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 31, 2020
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

5 participants