Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Computed / Reaction comparer #951

Merged
merged 20 commits into from
Jul 11, 2017
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
f09df1f
Merge remote-tracking branch 'refs/remotes/mobxjs/master'
jamiewinder Apr 13, 2017
4348fe1
add comparers
jamiewinder Apr 15, 2017
d4a8af0
add equals option to api
jamiewinder Apr 15, 2017
f6666d8
fixes for tests, workaround for build issue
jamiewinder Apr 15, 2017
f8115d6
fix 'correct api' test
jamiewinder Apr 15, 2017
e650e51
remove valueDidChange, seems to fix build issue
jamiewinder Apr 15, 2017
61939a9
expose @computed.equals
jamiewinder Apr 15, 2017
8abf3d1
update computedvalue constructor jsdoc
jamiewinder Apr 15, 2017
6dc991d
fixes equals option with computed boxed values. equals function not i…
jamiewinder Apr 15, 2017
aef91c7
expand on the computed equals invocation test
jamiewinder Apr 15, 2017
e9b7c25
no need for undefined checks on computed.equals tests any more
jamiewinder Apr 15, 2017
2380ffc
fix reaction use of equals; only invoke after first time
jamiewinder Apr 16, 2017
180cf30
another reaction-with-equals test
jamiewinder Apr 16, 2017
1884840
Merge remote-tracking branch 'refs/remotes/mobxjs/master' into comput…
jamiewinder Jun 13, 2017
a10e414
namespace comparers
jamiewinder Jun 27, 2017
61c5905
add extendObservable with custom comparer test
jamiewinder Jun 27, 2017
5f468f9
Merge remote-tracking branch 'refs/remotes/mobxjs/master' into comput…
jamiewinder Jun 27, 2017
3fde349
fix import order, add comparer to 'everything' export
jamiewinder Jun 29, 2017
7ef703f
Merge branch 'master' into computed-comparator
jamiewinder Jul 6, 2017
ab38024
Merge branch 'master' into computed-comparator
mweststrate Jul 11, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions src/api/autorun.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import {Lambda, getNextId, invariant, valueDidChange, fail} from "../utils/utils";
import {Lambda, getNextId, invariant, fail} from "../utils/utils";
import {isModifierDescriptor} from "../types/modifiers";
import {Reaction, IReactionPublic, IReactionDisposer} from "../core/reaction";
import {untrackedStart, untrackedEnd} from "../core/derivation";
import {action, isAction} from "../api/action";
import {IEqualsComparer, defaultComparer, structuralComparer} from "../types/comparer";
import {getMessage} from "../utils/messages";

/**
Expand Down Expand Up @@ -156,6 +157,7 @@ export interface IReactionOptions {
compareStructural?: boolean;
/** alias for compareStructural */
struct?: boolean;
equals?: IEqualsComparer<any>;
name?: string;
}

Expand Down Expand Up @@ -194,8 +196,9 @@ export function reaction<T>(expression: (r: IReactionPublic) => T, effect: (arg:

let firstTime = true;
let isScheduled = false;
let nextValue: T;
let value: T;

const equals = opts.equals ? opts.equals : (opts.compareStructural || opts.struct) ? structuralComparer : defaultComparer;
const r = new Reaction(opts.name, () => {
if (firstTime || (opts.delay as any) < 1) {
reactionRunner();
Expand All @@ -213,14 +216,14 @@ export function reaction<T>(expression: (r: IReactionPublic) => T, effect: (arg:
return;
let changed = false;
r.track(() => {
const v = expression(r);
changed = valueDidChange(opts.compareStructural!, nextValue, v);
nextValue = v;
const nextValue = expression(r);
changed = firstTime || !equals(value, nextValue);
value = nextValue;
});
if (firstTime && opts.fireImmediately!)
effect(nextValue, r);
effect(value, r);
if (!firstTime && (changed as boolean) === true)
effect(nextValue, r);
effect(value, r);
if (firstTime)
firstTime = false;
}
Expand Down
17 changes: 11 additions & 6 deletions src/api/computed.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import {IEqualsComparer, defaultComparer, structuralComparer} from "../types/comparer";
import {asObservableObject, defineComputedProperty} from "../types/observableobject";
import {invariant} from "../utils/utils";
import {createClassPropertyDecorator} from "../utils/decorators";
Expand All @@ -7,6 +8,7 @@ import {getMessage} from "../utils/messages";
export interface IComputedValueOptions<T> {
compareStructural?: boolean;
struct?: boolean;
equals?: IEqualsComparer<T>;
name?: string;
setter?: (value: T) => void;
context?: any;
Expand All @@ -17,17 +19,17 @@ export interface IComputed {
<T>(func: () => T, options: IComputedValueOptions<T>): IComputedValue<T>;
(target: Object, key: string | symbol, baseDescriptor?: PropertyDescriptor): void;
struct(target: Object, key: string | symbol, baseDescriptor?: PropertyDescriptor): void;
equals(equals: IEqualsComparer<any>): PropertyDecorator;
}


function createComputedDecorator(compareStructural) {
function createComputedDecorator(equals: IEqualsComparer<any>) {
return createClassPropertyDecorator(
(target, name, _, __, originalDescriptor) => {
invariant(typeof originalDescriptor !== "undefined", getMessage("m009"));
invariant(typeof originalDescriptor.get === "function", getMessage("m010"));

const adm = asObservableObject(target, "");
defineComputedProperty(adm, name, originalDescriptor.get, originalDescriptor.set, compareStructural, false);
defineComputedProperty(adm, name, originalDescriptor.get, originalDescriptor.set, equals, false);
},
function (name) {
const observable = this.$mobx.values[name];
Expand All @@ -43,8 +45,8 @@ function createComputedDecorator(compareStructural) {
);
}

const computedDecorator = createComputedDecorator(false);
const computedStructDecorator = createComputedDecorator(true);
const computedDecorator = createComputedDecorator(defaultComparer);
const computedStructDecorator = createComputedDecorator(structuralComparer);

/**
* Decorator for class properties: @computed get value() { return expr; }.
Expand All @@ -59,8 +61,11 @@ export var computed: IComputed = (
invariant(arguments.length < 3, getMessage("m012"));
const opts: IComputedValueOptions<any> = typeof arg2 === "object" ? arg2 : {};
opts.setter = typeof arg2 === "function" ? arg2 : opts.setter;
return new ComputedValue(arg1, opts.context, opts.compareStructural || opts.struct || false, opts.name || arg1.name || "", opts.setter);

const equals = opts.equals ? opts.equals : (opts.compareStructural || opts.struct) ? structuralComparer : defaultComparer;
return new ComputedValue(arg1, opts.context, equals, opts.name || arg1.name || "", opts.setter);
}
) as any;

computed.struct = computedStructDecorator;
computed.equals = createComputedDecorator;
5 changes: 3 additions & 2 deletions src/api/createtransformer.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {ComputedValue} from "../core/computedvalue";
import {invariant, getNextId, addHiddenProp} from "../utils/utils";
import {globalState} from "../core/globalstate";
import {defaultComparer} from "../types/comparer";
import {invariant, getNextId, addHiddenProp} from "../utils/utils";

export type ITransformer<A, B> = (object: A) => B;

Expand All @@ -17,7 +18,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), undefined, false, `Transformer-${(<any>transformer).name}-${sourceIdentifier}`, undefined);
super(() => transformer(sourceObject), undefined, defaultComparer, `Transformer-${(<any>transformer).name}-${sourceIdentifier}`, undefined);
}
onBecomeUnobserved() {
const lastValue = this.value;
Expand Down
21 changes: 14 additions & 7 deletions src/core/computedvalue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@ import {IObservable, reportObserved, propagateMaybeChanged, propagateChangeConfi
import {IDerivation, IDerivationState, trackDerivedFunction, clearObserving, untrackedStart, untrackedEnd, shouldCompute, CaughtException, isCaughtException} from "./derivation";
import {globalState} from "./globalstate";
import {createAction} from "./action";
import {createInstanceofPredicate, getNextId, valueDidChange, invariant, Lambda, unique, joinStrings, primitiveSymbol, toPrimitive} from "../utils/utils";
import {createInstanceofPredicate, getNextId, invariant, Lambda, unique, joinStrings, primitiveSymbol, toPrimitive} from "../utils/utils";
import {isSpyEnabled, spyReport} from "../core/spy";
import {autorun} from "../api/autorun";
import {IEqualsComparer} from "../types/comparer";
import {IValueDidChange} from "../types/observablevalue";
import {getMessage} from "../utils/messages";

Expand Down Expand Up @@ -46,7 +47,7 @@ export class ComputedValue<T> implements IObservable, IComputedValue<T>, IDeriva
lowestObserverState = IDerivationState.UP_TO_DATE;
unboundDepsCount = 0;
__mapid = "#" + getNextId();
protected value: T | undefined | CaughtException = undefined;
protected value: T | undefined | CaughtException = new CaughtException(null);
name: string;
isComputing: boolean = false; // to check for cycles
isRunningSetter: boolean = false;
Expand All @@ -57,12 +58,14 @@ export class ComputedValue<T> implements IObservable, IComputedValue<T>, IDeriva
*
* The `name` property is for debug purposes only.
*
* The `compareStructural` property indicates whether the return values should be compared structurally.
* Normally, a computed value will not notify an upstream observer if a newly produced value is strictly equal to the previously produced value.
* However, enabling compareStructural can be convenient if you always produce an new aggregated object and don't want to notify observers if it is structurally the same.
* The `equals` property specifies the comparer function to use to determine if a newly produced
* value differs from the previous value. Two comparers are provided in the library; `defaultComparer`
* compares based on identity comparison (===), and `structualComparer` deeply compares the structure.
* Structural comparison can be convenient 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, public scope: Object | undefined, private compareStructural: boolean, name: string, setter?: (v: T) => void) {
constructor(public derivation: () => T, public scope: Object | undefined, private equals: IEqualsComparer<any>, name: string, setter?: (v: T) => void) {
this.name = name || "ComputedValue@" + getNextId();
if (setter)
this.setter = createAction(name + "-setter", setter) as any;
Expand Down Expand Up @@ -135,7 +138,11 @@ export class ComputedValue<T> implements IObservable, IComputedValue<T>, IDeriva
}
const oldValue = this.value;
const newValue = this.value = this.computeValue(true);
return isCaughtException(newValue) || valueDidChange(this.compareStructural, newValue, oldValue);
return (
isCaughtException(oldValue) ||
isCaughtException(newValue) ||
!this.equals(oldValue, newValue)
);
}

computeValue(track: boolean) {
Expand Down
2 changes: 2 additions & 0 deletions src/mobx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@
*
*/

import { IEqualsComparer, defaultComparer, structuralComparer } from "./types/comparer";
import {registerGlobals} from "./core/globalstate";
registerGlobals();

export { IEqualsComparer, defaultComparer, structuralComparer } from "./types/comparer";
export { IAtom, Atom, BaseAtom } from "./core/atom";
export { IObservable, IDepTreeNode } from "./core/observable";
export { Reaction, IReactionPublic, IReactionDisposer } from "./core/reaction";
Expand Down
23 changes: 23 additions & 0 deletions src/types/comparer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { deepEqual } from '../utils/utils';

export interface IEqualsComparer<T> {
(a: T, b: T): boolean;
}

export function identityComparer(a: any, b: any): boolean {
return a === b;
}

export function structuralComparer(a: any, b: any): boolean {
if (typeof a === 'number' && typeof b === 'number' && isNaN(a) && isNaN(b)) {
return true;
}
return deepEqual(a, b);
}

export function defaultComparer(a: any, b: any): boolean {
if (typeof a === 'number' && typeof b === 'number' && isNaN(a) && isNaN(b)) {
return true;
}
return identityComparer(a, b);
}
7 changes: 4 additions & 3 deletions src/types/observableobject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ 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 {IEqualsComparer, defaultComparer} from "../types/comparer";
import {IEnhancer, isModifierDescriptor, IModifierDescriptor} from "../types/modifiers";
import {isAction, defineBoundAction} from "../api/action";
import {getMessage} from "../utils/messages";
Expand Down Expand Up @@ -100,7 +101,7 @@ export function defineObservablePropertyFromDescriptor(adm: ObservableObjectAdmi
}
} else {
// get x() { return 3 } set x(v) { }
defineComputedProperty(adm, propName, descriptor.get, descriptor.set, false, true);
defineComputedProperty(adm, propName, descriptor.get, descriptor.set, defaultComparer, true);
}
}

Expand Down Expand Up @@ -135,13 +136,13 @@ export function defineComputedProperty(
propName: string,
getter,
setter,
compareStructural: boolean,
equals: IEqualsComparer<any>,
asInstanceProperty: boolean
) {
if (asInstanceProperty)
assertPropertyConfigurable(adm.target, propName);

adm.values[propName] = new ComputedValue(getter, adm.target, compareStructural, `${adm.name}.${propName}`, setter);
adm.values[propName] = new ComputedValue(getter, adm.target, equals, `${adm.name}.${propName}`, setter);
if (asInstanceProperty) {
Object.defineProperty(adm.target, propName, generateComputedPropConfig(propName));
}
Expand Down
10 changes: 1 addition & 9 deletions src/utils/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,15 +94,6 @@ export function objectAssign() {
return res;
}

export function valueDidChange(compareStructural: boolean, oldValue, newValue): boolean {
if (typeof oldValue === 'number' && isNaN(oldValue)) {
return typeof newValue !== 'number' || !isNaN(newValue);
}
return compareStructural
? !deepEqual(oldValue, newValue)
: oldValue !== newValue;
}

const prototypeHasOwnProperty = Object.prototype.hasOwnProperty;
export function hasOwnProperty(object: Object, propName: string) {
return prototypeHasOwnProperty.call(object, propName);
Expand Down Expand Up @@ -242,5 +233,6 @@ export function toPrimitive(value) {

import {globalState} from "../core/globalstate";
import {IObservableArray, isObservableArray} from "../types/observablearray";
import {IEqualsComparer, defaultComparer} from "../types/comparer";
import {isObservableMap} from "../types/observablemap";
import {observable} from "../api/observable";
2 changes: 2 additions & 0 deletions test/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ test('correct api should be exposed', function(t) {
'computed',
'createTransformer',
'default',
'defaultComparer',
'expr',
'extendObservable',
'extendShallowObservable',
Expand All @@ -40,6 +41,7 @@ test('correct api should be exposed', function(t) {
'reaction',
'runInAction',
'spy',
'structuralComparer',
'toJS',
'transaction',
'untracked',
Expand Down
35 changes: 35 additions & 0 deletions test/babel/babel-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -873,3 +873,38 @@ test("action.bound binds (Babel)", t=> {

t.end();
})

test("@computed.equals (Babel)", t => {
const sameTime = (from, to) => from.hour === to.hour && from.minute === to.minute;
class Time {
constructor(hour, minute) {
this.hour = hour;
this.minute = minute;
}

@observable hour: number;
@observable minute: number;

@computed.equals(sameTime) get time() {
return { hour: this.hour, minute: this.minute };
}
}
const time = new Time(9, 0);

const changes = [];
const disposeAutorun = autorun(() => changes.push(time.time));

t.deepEqual(changes, [ { hour: 9, minute: 0 }]);
time.hour = 9;
t.deepEqual(changes, [ { hour: 9, minute: 0 }]);
time.minute = 0;
t.deepEqual(changes, [ { hour: 9, minute: 0 }]);
time.hour = 10;
t.deepEqual(changes, [ { hour: 9, minute: 0 }, { hour: 10, minute: 0 }]);
time.minute = 30;
t.deepEqual(changes, [ { hour: 9, minute: 0 }, { hour: 10, minute: 0 }, { hour: 10, minute: 30 }]);

disposeAutorun();

t.end();
});
48 changes: 48 additions & 0 deletions test/observables.js
Original file line number Diff line number Diff line change
Expand Up @@ -1730,3 +1730,51 @@ test('observables should not fail when ES6 Map is missing', t => {
global.Map = globalMapFunction;
t.end();
})

test("computed equals function only invoked when necessary", t => {
const comparisons = [];
const loggingComparer = (from, to) => {
comparisons.push({ from, to });
return from === to;
};

const left = mobx.observable("A");
const right = mobx.observable("B");
const combinedToLowerCase = mobx.computed(
() => left.get().toLowerCase() + right.get().toLowerCase(),
{ equals: loggingComparer }
);

const values = [];
const disposeAutorun = mobx.autorun(() => values.push(combinedToLowerCase.get()));

// No comparison should be made on the first value
t.deepEqual(comparisons, []);

// First change will cause a comparison
left.set("C");
t.deepEqual(comparisons, [{ from: "ab", to: "cb" }]);

// Transition *to* CaughtException in the computed won't cause a comparison
left.set(null);
t.deepEqual(comparisons, [{ from: "ab", to: "cb" }]);

// Transition *between* CaughtException-s in the computed won't cause a comparison
right.set(null);
t.deepEqual(comparisons, [{ from: "ab", to: "cb" }]);

// Transition *from* CaughtException in the computed won't cause a comparison
left.set("D");
right.set("E");
t.deepEqual(comparisons, [{ from: "ab", to: "cb" }]);

// Another value change will cause a comparison
right.set("F");
t.deepEqual(comparisons, [{ from: "ab", to: "cb" }, { from: "de", to: "df" }]);

t.deepEqual(values, ["ab", "cb", "de", "df"]);

disposeAutorun();

t.end();
});
Loading