mobxjs / mobx-react-lite

Lightweight React bindings for MobX based on React 16.8 and Hooks
https://mobx.js.org/react-integration.html
MIT License
2.13k stars 90 forks source link

fix: render reaction would dispose too early if observable data was changed before first `useEffect` #328

Closed mweststrate closed 3 years ago

mweststrate commented 3 years ago

Fixed an issue where, if an observable changed between initial render and useEffect, the render reaction would be disposed. This caused any computed values to be cleaned up as well, and become untracked.

It was a bit tricky to reproduce in a unit tests, as they don't run effects async, but see the following screenshots of before and after, and the unit test.

In the screenshot, observerHeader uses amount of todo's left. Then directly after mount, but before the useEffect happened (create HEADER), we would first change the collection (causing the render reaction to be disposed), and then set up an unrelated reaction. Since the computed value is suspended at that point, it will recomputed.

Screenshot 2020-10-15 at 09 57 33

After the fix, todo's is computed only once, as should be done.

Screenshot 2020-10-15 at 10 18 47

It seem that in the code somebody already thought about this case before, as a field changedBeforeMount was introduced earlier, but it was never set or read.

I've made those fields non optional; a) it would probably have prevented this bug, and b) this is in general faster as as JS engines optimize objects with a fixed shape better.

changeset-bot[bot] commented 3 years ago

🦋 Changeset detected

Latest commit: cabaa0ad00f5fb24b201ec4ead7db7fc10d68293

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package | Name | Type | | --------------- | ----- | | mobx-react-lite | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

coveralls commented 3 years ago

Coverage Status

Coverage increased (+0.03%) to 93.717% when pulling cabaa0ad00f5fb24b201ec4ead7db7fc10d68293 on fix-observable-change-before-mount into c6c78b355d0208c609e31e46583d9e54fb06c7a3 on master.