From 968df808f38fdf72ce90ab0e3b3bc1d2228762e6 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Sat, 24 Dec 2016 22:06:16 +0100 Subject: [PATCH] Implemented bound actions, see #699 --- CHANGELOG.md | 9 +++ src/api/action.ts | 89 ++++++++++++++++++++++++----- src/core/action.ts | 1 + src/types/observableobject.ts | 15 ++--- test/action.js | 34 +++++++++++ test/babel/babel-tests.js | 71 ++++++++++++++++++++++- test/typescript/typescript-tests.ts | 18 ++++++ 7 files changed, 211 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a3e96fb5b..7fed75e6b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,10 +52,19 @@ Using `computed` to create boxed observables has been simplified, and `computed` * `computed(expr, setter)` * `computed(expr, options)`, where options is an object that can specify one or more of the following fields: `name`, `setter`, `compareStructural` or `context` (the "this"). +### Bound actions + +[#699](https://github.com/mobxjs/mobx/issues/699) + +`action.bound` / `@action.bound` + +doesn't support name decorators like `@action`, the name is always the targeted key + ### Other changes * It is now possible to pass ES6 Maps to `observable` / observable maps. The map will be converted to an observable map (if keys are string like) * Made `action` more debug friendly, it should now be easier to step through +* ObservableMap now has an additional method, `.replace(data)`, which is a combination of `clear()` and `merge(data)`. # 2.7.0 diff --git a/src/api/action.ts b/src/api/action.ts index 0fbb1883d..87e7a2ce7 100644 --- a/src/api/action.ts +++ b/src/api/action.ts @@ -2,9 +2,47 @@ import {invariant, addHiddenProp} from "../utils/utils"; import {createClassPropertyDecorator} from "../utils/decorators"; import {createAction, executeAction} from "../core/action"; +export interface IActionFactory { + // nameless actions + R>(fn: T): T; + R>(fn: T): T; + R>(fn: T): T; + R>(fn: T): T; + R>(fn: T): T; + R>(fn: T): T; + + // named actions + R>(name: string, fn: T): T; + R>(name: string, fn: T): T; + R>(name: string, fn: T): T; + R>(name: string, fn: T): T; + R>(name: string, fn: T): T; + R>(name: string, fn: T): T; + + // generic forms + (fn: T): T; + (name: string, fn: T): T; + + // named decorator + (customName: string): (target: Object, key: string, baseDescriptor?: PropertyDescriptor) => void; + + // unnamed decorator + (target: Object, propertyKey: string, descriptor?: PropertyDescriptor): void; + + // .bound + R>(fn: T): T; + R>(fn: T): T; + R>(fn: T): T; + R>(fn: T): T; + R>(fn: T): T; + R>(fn: T): T; + + // .bound decorator + bound(target: Object, propertyKey: string, descriptor?: PropertyDescriptor): void; +} + const actionFieldDecorator = createClassPropertyDecorator( function (target, key, value, args, originalDescriptor) { - debugger; const actionName = (args && args.length === 1) ? args[0] : (value.name || key || ""); const wrappedAction = action(actionName, value); addHiddenProp(target, key, wrappedAction); @@ -19,19 +57,22 @@ const actionFieldDecorator = createClassPropertyDecorator( true ); -export function action R>(fn: T): T; -export function action R>(fn: T): T; -export function action R>(fn: T): T; -export function action R>(fn: T): T; -export function action R>(name: string, fn: T): T; -export function action R>(name: string, fn: T): T; -export function action R>(name: string, fn: T): T; -export function action R>(name: string, fn: T): T; -export function action(fn: T): T; -export function action(name: string, fn: T): T; -export function action(customName: string): (target: Object, key: string, baseDescriptor?: PropertyDescriptor) => void; -export function action(target: Object, propertyKey: string, descriptor?: PropertyDescriptor): void; -export function action(arg1, arg2?, arg3?, arg4?): any { +const boundActionDecorator = createClassPropertyDecorator( + function (target, key, value, args, originalDescriptor) { + const actionName = (args && args.length === 1) ? args[0] : key; + defineBoundAction(target, key, value); + }, + function (key) { + return this[key]; + }, + function () { + invariant(false, "It is not allowed to assign new values to @action fields"); + }, + false, + false +); + +export const action: IActionFactory = function action(arg1, arg2?, arg3?, arg4?): any { if (arguments.length === 1 && typeof arg1 === "function") return createAction(arg1.name || "", arg1); if (arguments.length === 2 && typeof arg2 === "function") @@ -41,7 +82,17 @@ export function action(arg1, arg2?, arg3?, arg4?): any { return namedActionDecorator(arg1); return namedActionDecorator(arg2).apply(null, arguments); -} +} as any; + +action.bound = function boundAction(arg1, arg2?, arg3?) { + if (typeof arg1 === "function") { + const action = createAction("", arg1); + (action as any).autoBind = true; + return action; + } + + return boundActionDecorator.apply(null, arguments); +}; function namedActionDecorator(name: string) { return function (target, prop, descriptor) { @@ -76,3 +127,11 @@ export function isAction(thing: any) { return typeof thing === "function" && thing.isMobxAction === true; } +export function defineBoundAction(target: any, propertyName: string, fn: Function) { + const res = function () { + return executeAction(propertyName, fn, target, arguments); + }; + (res as any).name = propertyName; + (res as any).isMobxAction = true; + addHiddenProp(target, propertyName, res); +} diff --git a/src/core/action.ts b/src/core/action.ts index caaa6e9dc..c024974ca 100644 --- a/src/core/action.ts +++ b/src/core/action.ts @@ -12,6 +12,7 @@ export function createAction(actionName: string, fn: Function): Function { const res = function () { return executeAction(actionName, fn, this, arguments); }; + (res as any).originalFn = fn; (res as any).isMobxAction = true; return res; } diff --git a/src/types/observableobject.ts b/src/types/observableobject.ts index 440dcfb28..1278d2fb2 100644 --- a/src/types/observableobject.ts +++ b/src/types/observableobject.ts @@ -5,15 +5,9 @@ import {runLazyInitializers} from "../utils/decorators"; import {hasInterceptors, IInterceptable, registerInterceptor, interceptChange} from "./intercept-utils"; import {IListenable, registerListener, hasListeners, notifyListeners} from "./listen-utils"; import {isSpyEnabled, spyReportStart, spyReportEnd} from "../core/spy"; -import {IEnhancer, isModifierDescriptor, IModifierDescriptor, deepEnhancer} from "../types/modifiers"; +import {IEnhancer, isModifierDescriptor, IModifierDescriptor} from "../types/modifiers"; +import {isAction, defineBoundAction} from "../api/action"; -const COMPUTED_FUNC_DEPRECATED = ( -` -In MobX 2.* passing a function without arguments to (extend)observable will automatically be inferred to be a computed value. -This behavior is ambiguous and will change in MobX 3 to create just an observable reference to the value passed in. -To disambiguate, please pass the function wrapped with a modifier: use 'computed(fn)' (for current behavior; automatic conversion), or 'asReference(fn)' (future behavior, just store reference) or 'action(fn)'. -Note that the idiomatic way to write computed properties is 'observable({ get propertyName() { ... }})'. -For more details, see https://github.com/mobxjs/mobx/issues/532`); export interface IObservableObject { "observable-object": IObservableObject; @@ -93,8 +87,9 @@ export function defineObservablePropertyFromDescriptor(adm: ObservableObjectAdmi const modifierDescriptor = descriptor.value as IModifierDescriptor; defineObservableProperty(adm, propName, modifierDescriptor.initialValue, modifierDescriptor.enhancer); } - // TODO: if is action, name and bind - else if (isComputedValue(descriptor.value)) { + else if (isAction(descriptor.value) && descriptor.value.autoBind === true) { + defineBoundAction(adm.target, propName, descriptor.value.originalFn); + } else if (isComputedValue(descriptor.value)) { // x: computed(someExpr) defineComputedPropertyFromComputedValue(adm, propName, descriptor.value); } else { diff --git a/test/action.js b/test/action.js index 905defa6c..47f61c054 100644 --- a/test/action.js +++ b/test/action.js @@ -370,3 +370,37 @@ test('computed values and actions', t => { t.end() }) + +test('bound actions bind', t => { + var called = 0; + var x = mobx.observable({ + y: 0, + z: mobx.action.bound(function(v) { + this.y += v; + this.y += v; + }), + + get yValue() { + called++; + return this.y; + } + }) + + var d = mobx.autorun(() => { + x.yValue; + }) + var events = []; + var d2 = mobx.spy(e => events.push(e)); + + var runner = x.z; + runner(3); + t.equal(x.yValue, 6); + t.equal(called, 2); + + t.deepEqual(events.filter(e => e.type === "action").map(e => e.name), ["z"]) + t.deepEqual(Object.keys(x), ["y"]); + + d(); + d2(); + t.end(); +}) diff --git a/test/babel/babel-tests.js b/test/babel/babel-tests.js index d9987d26e..da5c28ab3 100644 --- a/test/babel/babel-tests.js +++ b/test/babel/babel-tests.js @@ -803,4 +803,73 @@ test('issue #701', t => { t.equal(mobx.isObservableObject(model), true); t.end() -}) \ No newline at end of file +}) + + +test("@observable.ref (Babel)", t => { + class A { + @observable.ref ref = { a: 3} + } + + const a = new A(); + t.equal(a.ref.a, 3); + t.equal(mobx.isObservable(a.ref), false); + t.equal(mobx.isObservable(a, "ref"), true); + + t.end(); +}) + +test("@observable.shallow (Babel)", t => { + class A { + @observable.shallow arr = [{ todo: 1 }] + } + + const a = new A(); + const todo2 = { todo: 2 }; + a.arr.push(todo2) + t.equal(mobx.isObservable(a.arr), true); + t.equal(mobx.isObservable(a, "arr"), true); + t.equal(mobx.isObservable(a.arr[0]), false); + t.equal(mobx.isObservable(a.arr[1]), false); + t.ok(a.arr[1] === todo2) + + t.end(); +}) + + +test("@observable.deep (Babel)", t => { + class A { + @observable.deep arr = [{ todo: 1 }] + } + + const a = new A(); + const todo2 = { todo: 2 }; + a.arr.push(todo2) + + t.equal(mobx.isObservable(a.arr), true); + t.equal(mobx.isObservable(a, "arr"), true); + t.equal(mobx.isObservable(a.arr[0]), true); + t.equal(mobx.isObservable(a.arr[1]), true); + t.ok(a.arr[1] !== todo2) + t.equal(isObservable(todo2), false); + + t.end(); +}) + +test("action.bound binds (Babel)", t=> { + class A { + @observable x = 0; + @action.bound + inc(value: number) { + this.x += value; + } + } + + const a = new A(); + const runner = a.inc; + runner(2); + + t.equal(a.x, 2); + + t.end(); +}) diff --git a/test/typescript/typescript-tests.ts b/test/typescript/typescript-tests.ts index a5aae57ad..f4be11b24 100644 --- a/test/typescript/typescript-tests.ts +++ b/test/typescript/typescript-tests.ts @@ -1076,3 +1076,21 @@ test("@observable.deep (TS)", t => { t.end(); }) + +test("action.bound binds (TS)", t=> { + class A { + @observable x = 0; + @action.bound + inc(value: number) { + this.x += value; + } + } + + const a = new A(); + const runner = a.inc; + runner(2); + + t.equal(a.x, 2); + + t.end(); +})