From a1cf2c63ef92d3d42a5b42a23ff6c7a745664cfd Mon Sep 17 00:00:00 2001 From: James Zhan Date: Tue, 17 Sep 2024 02:12:53 +1000 Subject: [PATCH] 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 | 5 ++ docs/enabling-decorators.md | 10 ++-- .../decorators_20223/stage3-decorators.ts | 54 ++++++++++++++----- packages/mobx/src/types/actionannotation.ts | 19 ++++--- 4 files changed, 66 insertions(+), 22 deletions(-) create mode 100644 .changeset/soft-beans-dream.md diff --git a/.changeset/soft-beans-dream.md b/.changeset/soft-beans-dream.md new file mode 100644 index 0000000000..c11821bc51 --- /dev/null +++ b/.changeset/soft-beans-dream.md @@ -0,0 +1,5 @@ +--- +"mobx": patch +--- + +Fix 2022.3 @action decorators on fields no longer require makeObservable diff --git a/docs/enabling-decorators.md b/docs/enabling-decorators.md index e864f07ad9..aaa2ad9768 100644 --- a/docs/enabling-decorators.md +++ b/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 { } } ``` +
Migrating from legacy decorators @@ -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.
diff --git a/packages/mobx/__tests__/decorators_20223/stage3-decorators.ts b/packages/mobx/__tests__/decorators_20223/stage3-decorators.ts index e76c8f9fb7..9f2420d6ea 100644 --- a/packages/mobx/__tests__/decorators_20223/stage3-decorators.ts +++ b/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) +}) diff --git a/packages/mobx/src/types/actionannotation.ts b/packages/mobx/src/types/actionannotation.ts index 831cf52369..ab9d354bb3 100644 --- a/packages/mobx/src/types/actionannotation.ts +++ b/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") {