Merge d9f435f into 592e6e9

.changeset/proud-tips-bathe.md

@@ -0,0 +1,5 @@
+---
+"mobx": patch
+---
+
+fix: `onBecomeObserved` was not triggered correctly for computed dependencies of computeds. Fixes #2686, #2667

packages/mobx/__tests__/v5/base/become-observed.ts

@@ -5,7 +5,9 @@ import {
     computed,
     action,
     makeObservable,
-    onBecomeUnobserved
+    onBecomeUnobserved,
+    runInAction,
+    makeAutoObservable
 } from "../../../src/mobx"
 
 describe("become-observed", () => {
@@ -269,3 +271,241 @@ describe("#2309 onBecomeObserved inconsistencies", () => {
         expect(events).toEqual(["cb observed", "cb unobserved"])
     })
 })
+
+describe("nested computes don't trigger hooks #2686", () => {
+    let events: string[] = []
+
+    class Lower {
+        public lowerValue$ = -1
+
+        public isObserved = false
+
+        constructor() {
+            makeObservable(this, {
+                lowerValue$: observable
+            })
+
+            onBecomeObserved(
+                this,
+                "lowerValue$",
+                action(() => {
+                    events.push("onBecomeObserved")
+                    this.isObserved = true
+                })
+            )
+            onBecomeUnobserved(
+                this,
+                "lowerValue$",
+                action(() => {
+                    events.push("onBecomeUnobserved")
+                    this.isObserved = false
+                })
+            )
+        }
+    }
+
+    class UpperComputed {
+        constructor() {
+            makeObservable(this, {
+                upperValue$: computed,
+                lower$: observable.ref
+            })
+        }
+
+        public lower$: Lower | undefined
+
+        public get upperValue$() {
+            events.push("upperValue$")
+            const lower = this.lower$
+            return lower ? lower.lowerValue$ : -Infinity
+        }
+    }
+
+    const upperComputed = new UpperComputed()
+    const lowerForComputed = new Lower()
+
+    // Set up observers
+    const d = autorun(() => {
+        events.push("value read through computed: " + upperComputed.upperValue$)
+    })
+
+    // Provide the 'lower' values
+    runInAction(() => {
+        upperComputed.lower$ = lowerForComputed
+    })
+
+    // Check if the lower values are being observed.
+    expect(lowerForComputed.isObserved).toBe(true)
+
+    d()
+    expect(lowerForComputed.isObserved).toBe(false)
+
+    expect(events).toEqual([
+        "upperValue$",
+        "value read through computed: -Infinity",
+        "upperValue$",
+        "onBecomeObserved",
+        "value read through computed: -1",
+        "onBecomeUnobserved"
+    ])
+})
+
+test("#2686 - 2", () => {
+    const events: string[] = []
+    const options = { useColors: false }
+    makeAutoObservable(options)
+    const selection = { color: "red" }
+    makeAutoObservable(selection)
+
+    const blue = computed(() => {
+        let val
+        if (options.useColors) {
+            const isSelected = computed(() => selection.color === "blue")
+            onBecomeObserved(isSelected, () => events.push("observing"))
+            onBecomeUnobserved(isSelected, () => events.push("unobserving"))
+            val = isSelected.get()
+        }
+        return { isSelected: val }
+    })
+
+    const d = autorun(() => events.push(blue.get().isSelected ? "selected" : "unselected"))
+
+    runInAction(() => {
+        options.useColors = true
+        selection.color = "blue"
+    })
+    d()
+    expect(events).toEqual(["unselected", "observing", "selected", "unobserving"])
+})
+
+test("#2686 - 3", () => {
+    const events: string[] = []
+
+    // half first element of array
+    function halfFirst(data) {
+        const first = computed(() => {
+            events.push("recalculating")
+            return Math.round(data.elements[0] / 2) + data.suffix
+        })
+
+        onBecomeObserved(first, () => {
+            events.push("observing first")
+        })
+
+        return first
+    }
+
+    // APP
+
+    const network = observable({ model: null as any })
+
+    // load
+    const load = computed(() => {
+        // wait to load it
+        if (network.model) {
+            return halfFirst(network.model)
+        }
+        return undefined
+    })
+
+    // display
+    const result = computed(() => (load.get() ? load.get()!.get() : "loading"))
+    autorun(() => {
+        events.push("Current result: " + result.get())
+    })
+
+    runInAction(() => (network.model = observable({ suffix: "$", elements: [2, 4, 5] })))
+    runInAction(() => (network.model.elements[0] = 3))
+    runInAction(() => (network.model.elements[0] = 4))
+    runInAction(() => (network.model.elements[0] = 5))
+    expect(events).toEqual([
+        "Current result: loading",
+        "observing first",
+        "recalculating",
+        "Current result: 1$",
+        "recalculating",
+        "Current result: 2$",
+        "recalculating",
+        "recalculating",
+        "Current result: 3$"
+    ])
+})
+
+test("#2667", () => {
+    const events: any[] = []
+    class LazyInitializedList {
+        @observable
+        public items: string[] | undefined
+
+        @observable
+        public listName
+
+        public constructor(listName: string, lazyItems: string[]) {
+            makeObservable(this)
+            this.listName = listName
+            onBecomeObserved(
+                this,
+                "items",
+                action(() => {
+                    this.items = lazyItems
+                    events.push("onBecomeObserved" + listName)
+                })
+            )
+            onBecomeUnobserved(
+                this,
+                "items",
+                action(() => {
+                    this.items = undefined
+                    events.push("onBecomeUnobserved" + listName)
+                })
+            )
+        }
+    }
+
+    class ItemsStore {
+        @observable
+        private list: LazyInitializedList
+
+        public constructor() {
+            this.list = new LazyInitializedList("initial", ["a, b, c"])
+            makeObservable(this)
+        }
+
+        @action
+        public changeList = () => {
+            this.list = new LazyInitializedList("new", ["b, c, a"])
+        }
+
+        @computed
+        public get items(): string[] | undefined {
+            return this.list.items
+        }
+
+        @computed
+        public get activeListName(): string {
+            return this.list.listName
+        }
+    }
+
+    const store = new ItemsStore()
+
+    const d = autorun(() => {
+        events.push(store.items?.length ?? "-")
+        events.push(store.activeListName)
+    })
+
+    store.changeList()
+
+    d()
+
+    expect(events).toEqual([
+        "onBecomeObservedinitial",
+        1,
+        "initial",
+        "onBecomeObservednew",
+        1,
+        "new",
+        "onBecomeUnobservedinitial",
+        "onBecomeUnobservednew"
+    ])
+})

packages/mobx/src/core/reaction.ts

@@ -94,6 +94,8 @@ export class Reaction implements IDerivation, IReactionPublic {
         if (!this.isDisposed_) {
             startBatch()
             this.isScheduled_ = false
+            const prev = globalState.trackingContext
+            globalState.trackingContext = this
             if (shouldCompute(this)) {
                 this.isTrackPending_ = true
 
@@ -110,6 +112,7 @@ export class Reaction implements IDerivation, IReactionPublic {
                     this.reportExceptionInDerivation_(e)
                 }
             }
+            globalState.trackingContext = prev
             endBatch()
         }
     }