Added support for setters on computed props, fixes #421, #463

CHANGELOG.md

@@ -1,5 +1,6 @@
 # 2.5.0
 
+* Introduced setters for computed properties, use `computed(expr, setter)` or `@computed name get() { return expr } set(value) { action }`, #421, #463
 * Introduced `isStrictModeEnabled()`, deprecated `useStrict()` without arguments, see #464
 * Fixed #505, accessing an observable property throws before it is initialized
 

src/api/computeddecorator.ts

@@ -13,22 +13,25 @@ const computedDecorator = createClassPropertyDecorator(
 	(target, name, _, decoratorArgs, originalDescriptor) => {
 		invariant(typeof originalDescriptor !== "undefined", "@computed can only be used on getter functions, like: '@computed get myProps() { return ...; }'. It looks like it was used on a property.");
 		const baseValue = originalDescriptor.get;
+		const setter = originalDescriptor.set;
 		invariant(typeof baseValue === "function", "@computed can only be used on getter functions, like: '@computed get myProps() { return ...; }'");
 
 		let compareStructural = false;
 		if (decoratorArgs && decoratorArgs.length === 1 && decoratorArgs[0].asStructure === true)
 			compareStructural = true;
 
 		const adm = asObservableObject(target, undefined, ValueMode.Recursive);
-		defineObservableProperty(adm, name, compareStructural ? asStructure(baseValue) : baseValue, false);
+		defineObservableProperty(adm, name, compareStructural ? asStructure(baseValue) : baseValue, false, setter);
 	},
 	function (name) {
 		const observable = this.$mobx.values[name];
 		if (observable === undefined) // See #505
 			return undefined;
 		return observable.get();
 	},
-	throwingComputedValueSetter,
+	function (name, value) {
+		this.$mobx.values[name].set(value);
+	},
 	false,
 	true
 );
@@ -37,21 +40,21 @@ const computedDecorator = createClassPropertyDecorator(
  * Decorator for class properties: @computed get value() { return expr; }.
  * For legacy purposes also invokable as ES5 observable created: `computed(() => expr)`;
  */
+export function computed<T>(func: () => T, setter: (value: T) => void): IComputedValue<T>;
 export function computed<T>(func: () => T, scope?: any): IComputedValue<T>;
 export function computed(opts: IComputedValueOptions): (target: Object, key: string, baseDescriptor?: PropertyDescriptor) => void;
 export function computed(target: Object, key: string | symbol, baseDescriptor?: PropertyDescriptor): void;
-export function computed(targetOrExpr: any, keyOrScope?: any, baseDescriptor?: PropertyDescriptor, options?: IComputedValueOptions) {
-	if (arguments.length < 3 && typeof targetOrExpr === "function")
-		return computedExpr(targetOrExpr, keyOrScope);
-	invariant(!baseDescriptor || !baseDescriptor.set, `@observable properties cannot have a setter: ${keyOrScope}`);
+export function computed(targetOrExpr: any, keyOrScopeOrSetter?: any, baseDescriptor?: PropertyDescriptor, options?: IComputedValueOptions) {
+	if (typeof targetOrExpr === "function" && arguments.length < 3) {
+		if (typeof keyOrScopeOrSetter === "function")
+			return computedExpr(targetOrExpr, keyOrScopeOrSetter, undefined);
+		else
+			return computedExpr(targetOrExpr, undefined, keyOrScopeOrSetter);
+	}
 	return computedDecorator.apply(null, arguments);
 }
 
-function computedExpr<T>(expr: () => T, scope?: any) {
+function computedExpr<T>(expr: () => T, setter, scope: any) {
 	const [mode, value] = getValueModeFromValue(expr, ValueMode.Recursive);
-	return new ComputedValue(value, scope, mode === ValueMode.Structure, value.name);
-}
-
-export function throwingComputedValueSetter() {
-	throw new Error(`[ComputedValue] It is not allowed to assign new values to computed properties.`);
+	return new ComputedValue(value, scope, mode === ValueMode.Structure, value.name, setter);
 }

src/api/createtransformer.ts

@@ -17,7 +17,7 @@ export function createTransformer<A, B>(transformer: ITransformer<A, B>, onClean
 	// Local transformer class specifically for this transformer
 	class Transformer extends ComputedValue<B> {
 		constructor(private sourceIdentifier: string, private sourceObject: A) {
-			super(() => transformer(sourceObject), null, false, `Transformer-${(<any>transformer).name}-${sourceIdentifier}`);
+			super(() => transformer(sourceObject), null, false, `Transformer-${(<any>transformer).name}-${sourceIdentifier}`, undefined);
 		}
 		onBecomeUnobserved() {
 			const lastValue = this.value;

src/api/observabledecorator.ts

@@ -11,7 +11,7 @@ const decoratorImpl = createClassPropertyDecorator(
 		if (typeof baseValue === "function")
 			baseValue = asReference(baseValue);
 		const adm = asObservableObject(target, undefined, ValueMode.Recursive);
-		defineObservableProperty(adm, name, baseValue, true);
+		defineObservableProperty(adm, name, baseValue, true, undefined);
 		allowStateChangesEnd(prevA);
 	},
 	function (name) {

src/core/computedvalue.ts

@@ -1,7 +1,7 @@
 import {IObservable, reportObserved} from "./observable";
 import {IDerivation, trackDerivedFunction, isComputingDerivation, clearObserving, untrackedStart, untrackedEnd} from "./derivation";
 import {globalState} from "./globalstate";
-import {allowStateChangesStart, allowStateChangesEnd} from "./action";
+import {allowStateChangesStart, allowStateChangesEnd, createAction} from "./action";
 import {getNextId, valueDidChange, invariant, Lambda, unique, joinStrings} from "../utils/utils";
 import {isSpyEnabled, spyReport} from "../core/spy";
 import {autorun} from "../api/autorun";
@@ -34,6 +34,7 @@ export class ComputedValue<T> implements IObservable, IComputedValue<T>, IDeriva
 	dependencyStaleCount = 0;      // nr of nodes being observed that are currently not ready
 	protected value: T = undefined;
 	name: string;
+	setter: (value: T) => void;
 
 	/**
 	 * Create a new computed value based on a function expression.
@@ -45,8 +46,10 @@ export class ComputedValue<T> implements IObservable, IComputedValue<T>, IDeriva
 	 * However, enabling compareStructural can be convienent if you always produce an new aggregated object and don't want to notify observers if it is structurally the same.
 	 * This is useful for working with vectors, mouse coordinates etc.
 	 */
-	constructor(public derivation: () => T, private scope: Object, private compareStructural: boolean, name: string) {
+	constructor(public derivation: () => T, private scope: Object, private compareStructural: boolean, name: string, setter: (v: T) => void) {
 		this.name  = name || "ComputedValue@" + getNextId();
+		if (setter)
+			this.setter = createAction(name + "-setter", setter) as any;
 	}
 
 	peek() {
@@ -100,8 +103,11 @@ export class ComputedValue<T> implements IObservable, IComputedValue<T>, IDeriva
 		return this.value;
 	}
 
-	public set(_: T) {
-		throw new Error(`[ComputedValue '${name}'] It is not possible to assign a new value to a computed value.`);
+	public set(value: T) {
+		if (this.setter)
+			this.setter.call(this.scope, value);
+		else
+			throw new Error(`[ComputedValue '${this.name}'] It is not possible to assign a new value to a computed value.`);
 	}
 
 	private trackAndCompute(): boolean {

src/types/observableobject.ts

@@ -4,7 +4,6 @@ import {isAction} from "../api/action";
 import {ValueMode, AsStructure} from "./modifiers";
 import {Lambda, getNextId, invariant, assertPropertyConfigurable, isPlainObject, addHiddenFinalProp} from "../utils/utils";
 import {runLazyInitializers} from "../utils/decorators";
-import {throwingComputedValueSetter} from "../api/computeddecorator";
 import {hasInterceptors, IInterceptable, registerInterceptor, interceptChange} from "./intercept-utils";
 import {IListenable, registerListener, hasListeners, notifyListeners} from "./listen-utils";
 import {isSpyEnabled, spyReportStart, spyReportEnd} from "../core/spy";
@@ -70,22 +69,31 @@ export function setObservableObjectInstanceProperty(adm: ObservableObjectAdminis
 	if (adm.values[propName])
 		adm.target[propName] = value; // the property setter will make 'value' reactive if needed.
 	else
-		defineObservableProperty(adm, propName, value, true);
+		defineObservableProperty(adm, propName, value, true, undefined);
 }
 
-export function defineObservableProperty(adm: ObservableObjectAdministration, propName: string, newValue, asInstanceProperty: boolean) {
+export function defineObservableProperty(adm: ObservableObjectAdministration, propName: string, newValue, asInstanceProperty: boolean, setter) {
 	if (asInstanceProperty)
 		assertPropertyConfigurable(adm.target, propName);
 
 	let observable: ComputedValue<any>|ObservableValue<any>;
 	let name = `${adm.name}.${propName}`;
 	let isComputed = true;
 
-	if (typeof newValue === "function" && newValue.length === 0 && !isAction(newValue))
-		observable = new ComputedValue(newValue, adm.target, false, name);
-	else if (newValue instanceof AsStructure && typeof newValue.value === "function" && newValue.value.length === 0)
-		observable = new ComputedValue(newValue.value, adm.target, true, name);
-	else {
+	if (newValue instanceof ObservableValue) {
+		observable = newValue;
+		isComputed = false;
+	} else if (newValue instanceof ComputedValue) {
+		observable = newValue;
+		if (!newValue.scope)
+			newValue.scope = adm.target;
+	} else if (typeof newValue === "function" && newValue.length === 0 && !isAction(newValue)) {
+		// TODO: add warning in 2.6, see https://github.com/mobxjs/mobx/issues/421
+		// TODO: remove in 3.0
+		observable = new ComputedValue(newValue, adm.target, false, name, setter);
+	} else if (newValue instanceof AsStructure && typeof newValue.value === "function" && newValue.value.length === 0) {
+		observable = new ComputedValue(newValue.value, adm.target, true, name, setter);
+	} else {
 		isComputed = false;
 		if (hasInterceptors(adm)) {
 			const change = interceptChange<IObjectWillChange>(adm, {
@@ -139,11 +147,12 @@ export function generateComputedPropConfig(propName) {
 		get: function() {
 			return this.$mobx.values[propName].get();
 		},
-		set: throwingComputedValueSetter
+		set: function(v) {
+			return this.$mobx.values[propName].set(v);
+		}
 	};
 }
 
-
 export function setPropertyValue(instance, name: string, newValue) {
 	const adm = instance.$mobx;
 	const observable = adm.values[name];

test/babel/babel-tests.js

@@ -707,3 +707,22 @@ test("505, don't throw when accessing subclass fields in super constructor (babe
 	t.deepEqual(values, { a: 1, b: 2}) // In the TS test b is undefined, which is actually the expected behavior?
 	t.end()
 })
+
+test('computed setter should succeed (babel)', function(t) {
+	class Bla {
+		@observable a = 3;
+		@computed get propX() {
+			return this.a * 2;
+		}
+		set propX(v) {
+			this.a = v
+		}
+	}
+
+	const b = new Bla();
+	t.equal(b.propX, 6);
+	b.propX = 4;
+	t.equal(b.propX, 8);
+
+	t.end();
+});

test/observables.js

@@ -1,3 +1,5 @@
+"use strict"
+
 var test = require('tape');
 var mobx = require('..');
 var m = mobx;
@@ -1742,4 +1744,43 @@ test('atom events #427', t => {
 	d()
 	t.equal(stop, 2)
 	t.end()
-})
+})
+
+test("support computed property getters / setters", t => {
+	let a = observable({
+		size: 1,
+		volume: mobx.computed(function() {
+			return this.size * this.size
+		})
+	})
+
+	t.equal(a.volume, 1)
+	a.size = 3
+	t.equal(a.volume, 9)
+
+	t.throws(() => a.volume = 9, /It is not possible to assign a new value to a computed value/)
+
+	a = {}
+	mobx.extendObservable(a, {
+		size: 2,
+		volume: mobx.computed(
+			function() { return this.size * this.size },
+			function(v) { this.size = Math.sqrt(v) }
+		)
+	})
+
+	const values = []
+	const d = mobx.autorun(() => values.push(a.volume))
+
+	a.volume = 9
+	mobx.transaction(() => {
+		a.volume = 100
+		a.volume = 64
+	})
+
+	t.deepEqual(values, [4, 9, 64])
+	t.deepEqual(a.size, 8)
+
+	d()
+	t.end()
+})

test/typescript-tests.ts

@@ -233,17 +233,22 @@ test('box', function(t) {
     t.end();
 })
 
-test('observable setter should fail', function(t) {
-	t.throws(() => {
-		class Bla {
-			@computed get propX() {
-				return 3;
-			}
-			set propX(v) {
-
-			}
+test('computed setter should succeed', function(t) {
+	class Bla {
+		@observable a = 3;
+		@computed get propX() {
+			return this.a * 2;
+		}
+		set propX(v) {
+			this.a = v
 		}
-	}, 'propX');
+	}
+
+	const b = new Bla();
+	t.equal(b.propX, 6);
+	b.propX = 4;
+	t.equal(b.propX, 8);
+
 	t.end();
 });