fix: Allow action field decorators without makeObservable (#3879) (#3902

)

* fix action on field

* Create soft-beans-dream.md

* cleanup

* update doc

* tidy up

* minor fix

* add doc and unit test

.changeset/soft-beans-dream.md

@@ -0,0 +1,5 @@
+---
+"mobx": patch
+---
+
+Fix 2022.3 @action decorators on fields no longer require makeObservable

docs/enabling-decorators.md

@@ -14,8 +14,9 @@ After years of alterations, ES decorators have finally reached Stage 3 in the TC
 With modern decorators, it is no longer needed to call `makeObservable` / `makeAutoObservable`.
 
 2022.3 Decorators are supported in:
-* TypeScript (5.0 and higher, make sure that the `experimentalDecorators` flag is NOT enabled). [Example commit](https://github.com/mweststrate/currencies-demo/commit/acb9ac8c148e8beef88042c847bb395131e85d60).
-* For Babel make sure the plugin [`proposal-decorators`](https://babeljs.io/docs/babel-plugin-proposal-decorators) is enabled with the highest version (currently `2023-05`). [Example commit](https://github.com/mweststrate/currencies-demo/commit/4999d2228208f3e1e10bc00a272046eaefde8585).
+
+-   TypeScript (5.0 and higher, make sure that the `experimentalDecorators` flag is NOT enabled). [Example commit](https://github.com/mweststrate/currencies-demo/commit/acb9ac8c148e8beef88042c847bb395131e85d60).
+-   For Babel make sure the plugin [`proposal-decorators`](https://babeljs.io/docs/babel-plugin-proposal-decorators) is enabled with the highest version (currently `2023-05`). [Example commit](https://github.com/mweststrate/currencies-demo/commit/4999d2228208f3e1e10bc00a272046eaefde8585).
 
 ```js
 // tsconfig.json
@@ -106,6 +107,7 @@ class TodoList {
     }
 }
 ```
+
 </details>
 
 <details id="migrate-decorators"><summary>Migrating from legacy decorators</summary>
@@ -126,7 +128,9 @@ MobX' 2022.3 Decorators are very similar to the MobX 5 decorators, so usage is m
     The main cases for enumerability seem to have been around serialization and rest destructuring.
     -   Regarding serialization, implicitly serializing all properties probably isn't ideal in an OOP-world anyway, so this doesn't seem like a substantial issue (consider implementing `toJSON` or using `serializr` as possible alternatives)
     -   Addressing rest-destructuring, such is an anti-pattern in MobX - doing so would (likely unwantedly) touch all observables and make the observer overly-reactive).
--   `@action some_field = () => {}` was and is valid usage (_if_ `makeObservable()` is also used). However, `@action accessor some_field = () => {}` is never valid.
+-   `@action some_field = () => {}` was and is valid usage. However, inheritance is different between legacy decorators and modern decorators.
+    -   In legacy decorators, if superclass has a field decorated by `@action`, and subclass tries to override the same field, it will throw a `TypeError: Cannot redefine property`.
+    -   In modern decorators, if superclass has a field decorated by `@action`, and subclass tries to override the same field, it's allowed to override the field. However, the field on subclass is not an action unless it's also decorated with `@action` in subclass declaration.
 
 </details>
 

packages/mobx/__tests__/decorators_20223/stage3-decorators.ts

@@ -334,9 +334,7 @@ function normalizeSpyEvents(events: any[]) {
 
 test("action decorator (2022.3)", () => {
     class Store {
-        constructor(private multiplier: number) {
-            makeObservable(this)
-        }
+        constructor(private multiplier: number) {}
 
         @action
         add(a: number, b: number): number {
@@ -366,9 +364,7 @@ test("action decorator (2022.3)", () => {
 
 test("custom action decorator (2022.3)", () => {
     class Store {
-        constructor(private multiplier: number) {
-            makeObservable(this)
-        }
+        constructor(private multiplier: number) {}
 
         @action("zoem zoem")
         add(a: number, b: number): number {
@@ -416,9 +412,7 @@ test("custom action decorator (2022.3)", () => {
 
 test("action decorator on field (2022.3)", () => {
     class Store {
-        constructor(private multiplier: number) {
-            makeObservable(this)
-        }
+        constructor(private multiplier: number) {}
 
         @action
         add = (a: number, b: number) => {
@@ -450,9 +444,7 @@ test("action decorator on field (2022.3)", () => {
 
 test("custom action decorator on field (2022.3)", () => {
     class Store {
-        constructor(private multiplier: number) {
-            makeObservable(this)
-        }
+        constructor(private multiplier: number) {}
 
         @action("zoem zoem")
         add = (a: number, b: number) => {
@@ -893,6 +885,22 @@ test("action.bound binds (2022.3)", () => {
     t.equal(a.x, 2)
 })
 
+test("action.bound binds property function (2022.3)", () => {
+    class A {
+        @observable accessor x = 0
+        @action.bound
+        inc = function (value: number) {
+            this.x += value
+        }
+    }
+
+    const a = new A()
+    const runner = a.inc
+    runner(2)
+
+    t.equal(a.x, 2)
+})
+
 test("@computed.equals (2022.3)", () => {
     const sameTime = (from: Time, to: Time) => from.hour === to.hour && from.minute === to.minute
     class Time {
@@ -1085,3 +1093,25 @@ test("#2159 - computed property keys", () => {
         "original string value" // original string
     ])
 })
+
+test(`decorated field can be inherited, but doesn't inherite the effect of decorator`, () => {
+    class Store {
+        @action
+        action = () => {
+            return
+        }
+    }
+
+    class SubStore extends Store {
+        action = () => {
+            // should not be a MobX action
+            return
+        }
+    }
+
+    const store = new Store()
+    expect(isAction(store.action)).toBe(true)
+
+    const subStore = new SubStore()
+    expect(isAction(subStore.action)).toBe(false)
+})

packages/mobx/src/types/actionannotation.ts

@@ -8,8 +8,7 @@ import {
     Annotation,
     globalState,
     MakeResult,
-    assert20223DecoratorType,
-    storeAnnotation
+    assert20223DecoratorType
 } from "../internal"
 
 export function createActionAnnotation(name: string, options?: object): Annotation {
@@ -73,12 +72,18 @@ function decorate_20223_(this: Annotation, mthd, context: DecoratorContext) {
     const _createAction = m =>
         createAction(ann.options_?.name ?? name!.toString(), m, ann.options_?.autoAction ?? false)
 
-    // Backwards/Legacy behavior, expects makeObservable(this)
     if (kind == "field") {
-        addInitializer(function () {
-            storeAnnotation(this, name, ann)
-        })
-        return
+        return function (initMthd) {
+            let mthd = initMthd
+            if (!isAction(mthd)) {
+                mthd = _createAction(mthd)
+            }
+            if (ann.options_?.bound) {
+                mthd = mthd.bind(this)
+                mthd.isMobxAction = true
+            }
+            return mthd
+        }
     }
 
     if (kind == "method") {