mobxjs / mobx-state-tree

Full-featured reactive state management without the boilerplate
https://mobx-state-tree.js.org/
MIT License
6.94k stars 640 forks source link

Aborting action with AbortController does not work immediately, causes trouble with React.StrictMode #1987

Open DamianPereira opened 1 year ago

DamianPereira commented 1 year ago

Bug report

Sandbox link or minimal reproduction code

Here is the reproduction codesandbox: https://codesandbox.io/s/mobx-state-tree-abort-bug-irs5ch?file=/src/App.js

Describe the expected behavior

I expect that aborting the promise (with AbortController) should cause flow to end the action and run the catch block before starting a new action. This race condition happens because React StrictMode will render every component twice. Since I start store actions in useEffect, I want to run a cleanup action when unmounting, so the component is ready for the next run.

Describe the observed behavior

The catch block is ran a while later, not sure exactly when, but it ends up causing the second action to run before the first one finishes. Notice that if you disable React StrictMode it works ok.

Also, the documentation for mobx mentions being able to cancel flow actions here: https://mobx.js.org/actions.html#cancelling-flows-. But this is not available for the flow in mobx-state-tree right?

coolsoftwaretyler commented 1 year ago

I've been playing around with this a little bit and I'm not sure I have a sense of the actual bug, but I found another piece of debugging info. Here's a forked CodePlayground: https://codesandbox.io/s/mobx-state-tree-abort-bug-forked-iy6893?file=/src/App.js

I set up an event listener on the abortController.signal, and the callback on that listener fires when you would expect it to:

mounting APP! 
mounting title 
starting action! 
mounting APP! 
unmounting title and cancelling, this should cause a catch above and run the code inside before the component re-mounts! 
We heard about the abort signal # This is from the added abort listener.
mounting title 
starting action! 
mounting APP! 
this should be reached before the second "starting action" 
this failed! Aborted

This timing holds even if you comment out the fetch call to JSON placeholder and the title setting action.

I thought maybe this was specific to flow and how the generators/yield work, but then I converted the action to an async action and unprotected the tree. Same behavior: https://codesandbox.io/s/mobx-state-tree-abort-bug-forked-rqmjop?file=/src/App.js

Maybe this has something to do with keeping the AbortController in volatile state? Does something go wrong in the reconciliation phase or closures?

coolsoftwaretyler commented 1 year ago

I think we should leave this issue open, it seems like it's probably a bug. I still don't understand the root cause, though.