mobx-state-tree
mobx-state-tree copied to clipboard
mobxjs
WIP: Feature: flow cancellation
WIP: await and or leverage improved flow implementation in Mobx-core package
Actually, I think, this should be done also for Mobx, because mobx also has flows, so this is an update for flow and not for MST.
I'm missing something: how to cancel the actual in-flight operation (like XHR) and not just ignore the result. this is possible with passing an AbortController
@Bnaya that's not really how request cancelation works (except maybe in Firefox). Most XHR cancelation implementations these days simply ignore the response and do not physically close the request. At least that's what I found in my research.
@duro i find it hard to believe. how did you check? lets say i'm fetching 50mb, and after 1MB i abort the xhr (or fetch()) you say that the browser will not stop the transfer? (if i will have wireshark open i will see 49mb more passing on the wire)
request cancellation is supported on libraries that build on top of
xmlhttprequest, it is just not supported by the window.fetch api. Whether
MobX / MST will cancel the request depends on whether the pending promise
exposes a cancel() methods. So with most libraries, you could support
request cancellation by giving the pending promise is cancel method and
wiring it up to the libs cancellation api (if applicable)
Op zo 29 apr. 2018 om 11:14 schreef Bnaya Peretz [email protected]:
@duro https://github.com/duro i find it hard to believe. how did you check? lets say i'm fetching 50mb, and after 1MB i abort the xhr (or fetch()) you say that the browser will not stop the transfer? (if i will have wireshark open i will see 49mb more passing on the wire)
— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/mobxjs/mobx-state-tree/pull/691#issuecomment-385237261, or mute the thread https://github.com/notifications/unsubscribe-auth/ABvGhHp2JbLFgi3c1Rsp3MGl3SfD2qwrks5ttYSPgaJpZM4Skxse .
@mweststrate Actually as chrome 66, it's supported in all major browsers for fetch, using AbortController (somekind of cancellation token) And seems that it will be the de-facto api for cancelling stuff. Adding cancellable promise to the standard was cancelled. You may add cancel, or any other method on Promise, but when you chain/pass to any standard promise, you will lose the cancel method
Ref https://developer.mozilla.org/en-US/docs/Web/API/AbortController#Browser_compatibility
Yeah I am aware of this. With this mechanism there is at least a possibliity of suporting it, with some effort, otherwise there is none (the abortController is afaik for window.fetch, but not generically applicable to promises)
Op zo 29 apr. 2018 om 16:38 schreef Bnaya Peretz [email protected]:
@mweststrate https://github.com/mweststrate Actuall as chrome 66, it's supported in all major browsers for fetch, using AbortController (somekind of cancellation token) And seems that it will be the de-facto api for cancelling stuff. Adding cancellable promise to the standard was cancelled. You may add cancel, or any other method on Promise, but when you chain/pass to any standard promise, you will lose the cancel method
— You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub https://github.com/mobxjs/mobx-state-tree/pull/691#issuecomment-385256060, or mute the thread https://github.com/notifications/unsubscribe-auth/ABvGhDYBfRyA1vzafyXhn4HLJV7o3WRJks5ttdBYgaJpZM4Skxse .
Uhm, instead of throwing a generic error with a text, shoul'nt we instead throw a custom error and provide an utility function like "isCancellation(error)" so users can check that inside the generator catch? :)
Is this ready to be merged, since this feature is released for MobX? Would be nice to have the CancellablePromise's in the MST flows as well since they work so well in MobX!
Also very interested in this. In trying to integrate mst with history pop state, it becomes clear that we need to cancel flow's when going forward/backward through history.
Just wanted to check in on the status of this PR, what sort of work is remaining to get this merged in? It would be great to get have cancellable flows available in MST.
Im in favor to aducate developers to use the acceptable practices of async operation cancelation, With after fetch got its abort controller, thats the way to go.
And then the promise the flow is yield on, will be rejected, and the developer may catch it with try catch and check if its abort error etc.
Imo there is no reason to add another api for that.
@mweststrate can you please close the PR if the api is not going to be added to MST - just so people know the status? (Personally, feature parity with mobx.flow would be nice but I understand if you don't want to add another api.)
Not really, but if you are willing to champion the PR and move it forward I'll happily merge (I think most of the work was actually done already, I don't remember anymore :-P).
On Thu, Aug 8, 2019 at 10:50 PM Alexander Zinchenko < [email protected]> wrote:
Are there any news?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mobxjs/mobx-state-tree/pull/691?email_source=notifications&email_token=AAN4NBGOZ5374DBCGMG7WMDQDSBK5A5CNFSM4EUTDMPKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD343UGA#issuecomment-519682584, or mute the thread https://github.com/notifications/unsubscribe-auth/AAN4NBHC22AI34JGEVQNNX3QDSBK5ANCNFSM4EUTDMPA .
Feel free to leave the TS part open (that is put in :any wherever
needed). It is actually a non trivial, but pretty cool subject to look
into; as it involves learning generators :)
On Tue, Dec 17, 2019 at 6:56 AM Asif Saif Uddin [email protected] wrote:
I will check it more thoroughly after I check the mobx flow more deeply.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mobxjs/mobx-state-tree/pull/691?email_source=notifications&email_token=AAN4NBHJQ4RNVXNRA37MVPLQZBZ3RA5CNFSM4EUTDMPKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHBLM7A#issuecomment-566408828, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAN4NBE6G2NLU4PYOBOJYTTQZBZ3RANCNFSM4EUTDMPA .
I am Python guy so new shiny ES.next stuffs are quite cool but not entirely unknown to me. I will surely give this a try.
Throwing in on-going spec-level efforts to make cancelation standard: Addin AbortController + AbortSignal to ES spec https://github.com/tc39/proposal-observable/issues/201
Stage 2:
https://github.com/tc39/proposal-explicit-resource-management
Symbol based mechanism
Symbol.dispose
IMO abort token/controller based pattern can be implemented on user-land, and can be abstracted-away easily when any spec will materialized.
While this feature is still pending, any suggestions on how to do this on the outside?
I wanted to add comment earlier but didn't have enough time. But I see that you are still discussing this feature, so I want to give my 5 cents.
Waiting for this feature. I absolutely agree that request cancellation it's better in many cases, but... it's not always necessary. You may deal with fetch API requests, web-sockets, different libraries or some custom async functions. And sometimes it doesn't really matter whether the called function will be interrupted or not. You just need to cancel some deferred code execution. As for now, I use custom utils written using flow util from mobx. I also want to mention redux-sagas as an example. Redux saga has API for cancellation and it's quite convenient.
I've run into a situation where I need this. I have state trees in a react native app handling "animated sequences". A flow representing a logical grouping of animations will yield several promises in sequence that each wait for an event from an RXJS subject indicating that a particular animation(s) is complete.
That makes it trivial to write out a series of animations in a flow as though it was a synchronous function. It would otherwise be a huge pain to coordinate these animations.
Work great!... except when the user presses the Android back button or otherwise exits the sequence early. There's no good way that I can find to cancel the ongoing flow. This PR would be ideal as I simply want to stop and ignore the rest of these flows if they get interrupted by the user.
Since this has been sitting for awhile I'm guessing it's probably not in a state ready to be merged anymore. I'll try to find time to update it but for now I don't think I'll be able to get to it very soon (or maybe at all).
Separately (but interestingly IMO) I think the new TS 4.1 features will let us have accurate types for generators/flows which is neat.
On reviewing this PR, it looks like Michel got this to a pretty good place and it's a desirable feature, but it will require a bit more thought, resolving conflicts, some documentation, some manual testing, and then for the core team to give the 👍 .
Going to leave this for now, but revisit later.
I forked this and got it all up and running. I'm using it in my app and it seems to work great so far. I'll send a PR as soon as I have the time.
AbortController & signal are now also part of node, which make it de-factor way to cancel things in js.
abort signal is added to more and more browser & node apis and not only browser's fetch
It can also easily polyfiled.
Cancellable promises are pulled away and are no longer considered to be part of ecma spec Instead of adding none-spec extension to the returned Promise, we can just tell users to make their async code cancelable and keep the API of the library more focused. refs: https://github.com/tc39/proposal-cancelable-promises#this-proposal-has-been-withdrawn https://github.com/nodejs/node/pulls?q=is%3Apr+abortsignal https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/abort_event#Examples
@Bnaya Good point. We should provide ample examples of how to do this.