Fix #3271 flow.bound (#3273)

.changeset/fuzzy-coins-beam.md

@@ -0,0 +1,5 @@
+---
+"mobx": patch
+---
+
+fix `flow.bound` #3271

packages/mobx/__tests__/v5/base/make-observable.ts

@@ -378,7 +378,7 @@ test("makeAutoObservable allows overrides", () => {
 })
 
 test("makeAutoObservable cannot be used on subclasses", () => {
-    class A { }
+    class A {}
 
     class B extends A {
         constructor() {
@@ -1232,10 +1232,10 @@ test("subclass - cannot re-annotate", () => {
                 computed: computed
             })
         }
-        action() { }
-        actionBound() { }
-        *flow() { }
-        *flowBound() { }
+        action() {}
+        actionBound() {}
+        *flow() {}
+        *flowBound() {}
         get computed() {
             return this
         }
@@ -1248,7 +1248,7 @@ test("subclass - cannot re-annotate", () => {
                 action: action
             })
         }
-        action() { }
+        action() {}
     }
 
     class ChildActionBound extends Parent {
@@ -1258,7 +1258,7 @@ test("subclass - cannot re-annotate", () => {
                 actionBound: action.bound
             })
         }
-        actionBound() { }
+        actionBound() {}
     }
 
     class ChildFlow extends Parent {
@@ -1268,7 +1268,7 @@ test("subclass - cannot re-annotate", () => {
                 flow: flow
             })
         }
-        *flow() { }
+        *flow() {}
     }
 
     class ChildFlowBound extends Parent {
@@ -1278,7 +1278,7 @@ test("subclass - cannot re-annotate", () => {
                 flowBound: flow.bound
             })
         }
-        *flowBound() { }
+        *flowBound() {}
     }
 
     class ChildObservable extends Parent {
@@ -1319,13 +1319,13 @@ test("subclass - cannot re-decorate", () => {
             makeObservable(this)
         }
         @action
-        action() { }
+        action() {}
         @action.bound
-        actionBound() { }
+        actionBound() {}
         @flow
-        *flow() { }
+        *flow() {}
         @flow.bound
-        *flowBound() { }
+        *flowBound() {}
         @computed
         get computed() {
             return this
@@ -1339,7 +1339,7 @@ test("subclass - cannot re-decorate", () => {
                 makeObservable(this)
             }
             @action
-            action() { }
+            action() {}
         }
     }).toThrow(/^\[MobX\] Cannot apply/)
 
@@ -1350,7 +1350,7 @@ test("subclass - cannot re-decorate", () => {
                 makeObservable(this)
             }
             @action.bound
-            actionBound() { }
+            actionBound() {}
         }
     }).toThrow(/^\[MobX\] Cannot apply/)
 
@@ -1361,7 +1361,7 @@ test("subclass - cannot re-decorate", () => {
                 makeObservable(this)
             }
             @flow
-            *flow() { }
+            *flow() {}
         }
     }).toThrow(/^\[MobX\] Cannot apply/)
 
@@ -1372,7 +1372,7 @@ test("subclass - cannot re-decorate", () => {
                 makeObservable(this)
             }
             @flow.bound
-            *flowBound() { }
+            *flowBound() {}
         }
     }).toThrow(/^\[MobX\] Cannot apply/)
 
@@ -1411,14 +1411,14 @@ test("subclass - cannot redefine property", () => {
                 computed: computed
             })
         }
-        action = () => { }
+        action = () => {}
         get computed() {
             return this
         }
     }
 
     class ChildAction extends Parent {
-        action = () => { }
+        action = () => {}
     }
 
     class ChildObservable extends Parent {
@@ -1560,16 +1560,16 @@ test("makeAutoObservable + production build #2751", () => {
 test("inherited fields are assignable before makeObservable", () => {
     class Foo {
         constructor() {
-            this.action = () => { }
-            this.flow = function* flow() { }
+            this.action = () => {}
+            this.flow = function* flow() {}
             makeObservable(this, {
                 action,
                 flow
             })
         }
 
-        action() { }
-        *flow() { }
+        action() {}
+        *flow() {}
     }
 
     const foo1 = new Foo()
@@ -1592,7 +1592,7 @@ test("makeAutoObservable + symbolic keys", () => {
         get [computedSymbol]() {
             return this.observable
         }
-        [actionSymbol]() { }
+        [actionSymbol]() {}
 
         constructor() {
             makeAutoObservable(this)
@@ -1652,20 +1652,53 @@ test("makeObservable throws when mixing @decorators with annotations", () => {
         }
     }
 
-    expect(() => new Test()).toThrow(/makeObservable second arg must be nullish when using decorators/);
+    expect(() => new Test()).toThrow(
+        /makeObservable second arg must be nullish when using decorators/
+    )
 })
 
 test("makeAutoObservable + Object.create #3197", () => {
     const proto = {
-        action() { },
-        *flow() { },
-        get computed() { return null },
-    };
-    const o = Object.create(proto);
-    o.observable = 5;
-    makeAutoObservable(o);
-    expect(isAction(proto.action)).toBe(true);
-    expect(isFlow(proto.flow)).toBe(true);
-    expect(isComputedProp(o, "computed")).toBe(true);
-    expect(isObservableProp(o, "observable")).toBe(true);
-});
+        action() {},
+        *flow() {},
+        get computed() {
+            return null
+        }
+    }
+    const o = Object.create(proto)
+    o.observable = 5
+    makeAutoObservable(o)
+    expect(isAction(proto.action)).toBe(true)
+    expect(isFlow(proto.flow)).toBe(true)
+    expect(isComputedProp(o, "computed")).toBe(true)
+    expect(isObservableProp(o, "observable")).toBe(true)
+})
+
+test("flow.bound #3271", async () => {
+    class Test {
+        constructor() {
+            makeObservable(this, { flowBound: flow.bound })
+        }
+        *flowBound() {
+            return this
+        }
+    }
+
+    const t1 = new Test()
+    const t2 = new Test()
+
+    // Make sure flow is actually bindable
+    expect(
+        await flow(function* () {
+            return this
+        }).bind(t1)()
+    ).toBe(t1)
+
+    expect(t1.hasOwnProperty("flowBound")).toBe(true)
+    expect(t2.hasOwnProperty("flowBound")).toBe(true)
+
+    expect(t1.flowBound !== t2.flowBound).toBe(true)
+
+    expect(await t1.flowBound.call(null)).toBe(t1)
+    expect(await t2.flowBound.call(null)).toBe(t2)
+})

packages/mobx/src/types/flowannotation.ts

@@ -7,7 +7,8 @@ import {
     isFlow,
     isFunction,
     globalState,
-    MakeResult
+    MakeResult,
+    hasProp
 } from "../internal"
 
 export function createFlowAnnotation(name: string, options?: object): Annotation {
@@ -33,7 +34,7 @@ function make_(
     }
     // prototype
     // bound - must annotate protos to support super.flow()
-    if (this.options_?.bound && !isFlow(adm.target_[key])) {
+    if (this.options_?.bound && (!hasProp(adm.target_, key) || !isFlow(adm.target_[key]))) {
         if (this.extend_(adm, key, descriptor, false) === null) return MakeResult.Cancel
     }
     if (isFlow(descriptor.value)) {
@@ -81,11 +82,18 @@ function createFlowDescriptor(
 ): PropertyDescriptor {
     assertFlowDescriptor(adm, annotation, key, descriptor)
     let { value } = descriptor
+    // In case of flow.bound, the descriptor can be from already annotated prototype
+    if (!isFlow(value)) {
+        value = flow(value)
+    }
     if (bound) {
+        // We do not keep original function around, so we bind the existing flow
         value = value.bind(adm.proxy_ ?? adm.target_)
+        // This is normally set by `flow`, but `bind` returns new function...
+        value.isMobXFlow = true
     }
     return {
-        value: flow(value),
+        value,
         // Non-configurable for classes
         // prevents accidental field redefinition in subclass
         configurable: safeDescriptors ? adm.isPlainObject_ : true,