13

See here: https://reactjs.org/blog/2015/12/16/ismounted-antipattern.html And also here: How to cancel a fetch on componentWillUnmount And here: ismounted antipattern, track own property

In both cases they mention 3 approaches:

  • In your promise.resolve check this.IsMounted(), which React will return correctly for you, if the `Compounted Has Unmounted
  • In your promise.resolve check _isMounted, which you have manually tracked in ComponentWillUnmount() method.
  • Use cancellable promises, so that you promise doesn't ever resolve. And this will solve all your problems and make it lovely.

Except, in the 3rd case your promise will error(), but might also error() in other cases (e.g. the API is down).

So in reality the 3rd option boils down to: - In your promise.error check errorPayload.IsCancelled, which you have manually tracked in the cancellablePromise object, which has in turn been triggered by a manual call in ComponentWillUnmount.

So all three are pretty much identical:

When you are handling your promise outcomes, check the value of this variable that is directly tied to whether the component has already unmounted.

Why do they assert that the 3rd option is better than the other 2, and that the 1st option is an antipattern.

E_net4
  • 27,810
  • 13
  • 101
  • 139
Brondahl
  • 7,402
  • 5
  • 45
  • 74

3 Answers3

9

The key element here is that if (this.isMounted()) { setState(...) } is an antipattern in general. It can lead to the supression of useful warnings, and so its appearance should be treated with suspicion since, in the majority of cases, it represents an opportunity to mask real issues. As such, even in cases where its behaviour is functionally the same as some other approach, that other approach is preferable.

In the case of API calls, it is entirely reasonable that you might want to ignore the results of a promise because it is no longer relevant. The use of a canceled promise syntactically and semantically ties the logic of whether to ignore the result specifically to the API call, which prevents any possibility of future developers accidentally making use of the code in another context and potentially supressing meaningful warnings.

Though the difference may be semantic, the semantics themselves have value to maintainability. In this case, the cancelable promise serves to structurally colocate concerns, attaching a behaviour that can be a problem in general to the specific situation in which it is okay.

Sam
  • 163
  • 6
  • 3
    I get it ... we're saying that API calls *in isolation* are a reasonable use of `isMounted()`. But if we use cancellable promises, then that allows us to say "Now, `isMounted()` is NEVER correct.". Which is then more maintainable and better protected for those other cases. – Brondahl Jan 23 '19 at 15:53
  • 2
    Another analogy: GOTOs are (canonically) considered Harmful. But `if`, `while`, `break`, `continue` and `switch` are all fundamentally just doing the same thing, but declaring acceptable situations where it's reasonable to use the concept. – Brondahl Feb 19 '19 at 12:10
2

isMounted component method is deprecated. It cannot be used anymore in React 16.

There is no built-in _isMounted state, it should be defined by the user in component lifecycle hooks. It's a matter of preference to some degree. The problem with it is that it requires promise chain to be coupled to component instance to be able to access this._isMounted.

This is not a problem for cancellable promises. Even more, this pattern allows to actually cancel asynchronous process, e.g. Axios actually cancels XHR request during cancellation. Still, in order to make this work, this requires the entire promise chain to be cancellation-aware. This may be tedious since cancellation isn't supported in native promises, including async..await.

Observables provide even more powerful ways to control the execution. There is a single point (a subscription) that is capable to cancel the entire chain by calling unsubscription function:

const unsubscribe = fetchData()
.mergeMap(asynchronousProcessing)
.mergeMap(yetAnotherAsynchronousProcessing)
.subscribe(data => this.setState(data));
Estus Flask
  • 206,104
  • 70
  • 425
  • 565
-1

Let's imagine the action that will cause the future setState call does a lot of computations, or does a lot of network requests. All those ressources will be used, then somewhen in the future when the promise resolves, it realizes that all the work was not needed as the user already left that part of the page, and all the work will be discarded. Thats not good for performance & network/memory usage, especially if you got a lot of elements that appear and disappear. If you go with option 3, only a small amount of ressources gets wasted until the next stopping point gets reached and the ressources are freed. I assume they removed 1) to force codewriters to use 3), and proposed 2) for cases where it is impossible / hard to migrate.

Jonas Wilms
  • 132,000
  • 20
  • 149
  • 151
  • Not true - the cancellation of the promise doesn't do ANYTHING until the promise has resolved - these aren't cancellation tokens being passed up to the server doing the work - they're just flags on how the promise resolves. – Brondahl Jan 23 '19 at 12:37
  • @brondahl depends on [how](https://developers.google.com/web/updates/2017/09/abortable-fetch) you cancel the underlying actions. – Jonas Wilms Jan 24 '19 at 13:17
  • Interesting ... I've not come across that before, however ... based on that page you still have exactly the same promise logic, just now the check is `if (err.name === 'AbortError')` rather that `if(err.isCancelled)`. (1/2) – Brondahl Jan 25 '19 at 11:13
  • With regards to stopping work, I'd be pretty surprised if that was ACTUALLY telling the server to stop doing work (i.e. kill a server thread), if it's being implemented at the browser level. (Since that would require every browser to know how to send that message to every server and server-side framework) I suspect it's just telling the browser to not listen to the server's response. So all your doing is pushing the "but then do nothing" one inexpensive step further up. (2/2) – Brondahl Jan 25 '19 at 11:14