From 4348fe1c5e536b9b880883ca3e5846b1b75c698c Mon Sep 17 00:00:00 2001 From: jamiewinder Date: Sat, 15 Apr 2017 15:32:58 +0100 Subject: [PATCH 01/16] add comparers --- src/types/comparer.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 src/types/comparer.ts diff --git a/src/types/comparer.ts b/src/types/comparer.ts new file mode 100644 index 000000000..510e2febc --- /dev/null +++ b/src/types/comparer.ts @@ -0,0 +1,13 @@ +import { deepEqual } from '../utils/utils'; + +export type EqualityComparer = (a: T, b: T) => boolean; + +export const equalityComparer = (a: any, b: any) => a == b; +export const identityComparer = (a: any, b: any) => a === b; +export const structuralComparer = (a: any, b: any) => deepEqual(a, b); +export const defaultComparer = (a: any, b: any) => { + if (typeof a === 'number' && typeof b === 'number' && isNaN(a) && isNaN(b)) { + return true; + } + return identityComparer(a, b); +}; From d4a8af0bea4a69da21d6b916daf68499e65915c3 Mon Sep 17 00:00:00 2001 From: jamiewinder Date: Sat, 15 Apr 2017 16:14:22 +0100 Subject: [PATCH 02/16] add equals option to api --- src/api/autorun.ts | 6 +++++- src/api/computed.ts | 6 +++++- src/api/createtransformer.ts | 5 +++-- src/core/computedvalue.ts | 6 ++++-- src/mobx.ts | 2 ++ src/types/comparer.ts | 2 +- src/types/observableobject.ts | 7 ++++--- src/utils/utils.ts | 10 +++------- 8 files changed, 27 insertions(+), 17 deletions(-) diff --git a/src/api/autorun.ts b/src/api/autorun.ts index 9d5ab2006..1db9910a5 100644 --- a/src/api/autorun.ts +++ b/src/api/autorun.ts @@ -3,6 +3,7 @@ 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 {EqualsComparer, defaultComparer, structuralComparer} from "../types/comparer"; import {getMessage} from "../utils/messages"; /** @@ -156,6 +157,7 @@ export interface IReactionOptions { compareStructural?: boolean; /** alias for compareStructural */ struct?: boolean; + equals?: EqualsComparer; name?: string; } @@ -196,6 +198,8 @@ export function reaction(expression: (r: IReactionPublic) => T, effect: (arg: let isScheduled = false; let nextValue: T; + const equals = opts.equals || (opts.compareStructural || opts.struct) ? structuralComparer : defaultComparer; + const r = new Reaction(opts.name, () => { if (firstTime || (opts.delay as any) < 1) { reactionRunner(); @@ -214,7 +218,7 @@ export function reaction(expression: (r: IReactionPublic) => T, effect: (arg: let changed = false; r.track(() => { const v = expression(r); - changed = valueDidChange(opts.compareStructural!, nextValue, v); + changed = valueDidChange(nextValue, v, equals); nextValue = v; }); if (firstTime && opts.fireImmediately!) diff --git a/src/api/computed.ts b/src/api/computed.ts index 0386af753..76e985015 100644 --- a/src/api/computed.ts +++ b/src/api/computed.ts @@ -1,3 +1,4 @@ +import {EqualsComparer, defaultComparer, structuralComparer} from "../types/comparer"; import {asObservableObject, defineComputedProperty} from "../types/observableobject"; import {invariant} from "../utils/utils"; import {createClassPropertyDecorator} from "../utils/decorators"; @@ -7,6 +8,7 @@ import {getMessage} from "../utils/messages"; export interface IComputedValueOptions { compareStructural?: boolean; struct?: boolean; + equals?: EqualsComparer; name?: string; setter?: (value: T) => void; context?: any; @@ -59,7 +61,9 @@ export var computed: IComputed = ( invariant(arguments.length < 3, getMessage("m012")); const opts: IComputedValueOptions = 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.compareStructural || opts.struct) ? structuralComparer : defaultComparer; + return new ComputedValue(arg1, opts.context, equals, opts.name || arg1.name || "", opts.setter); } ) as any; diff --git a/src/api/createtransformer.ts b/src/api/createtransformer.ts index c3485ed81..33a64b639 100644 --- a/src/api/createtransformer.ts +++ b/src/api/createtransformer.ts @@ -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 = (object: A) => B; @@ -17,7 +18,7 @@ export function createTransformer(transformer: ITransformer, onClean // Local transformer class specifically for this transformer class Transformer extends ComputedValue { constructor(private sourceIdentifier: string, private sourceObject: A) { - super(() => transformer(sourceObject), undefined, false, `Transformer-${(transformer).name}-${sourceIdentifier}`, undefined); + super(() => transformer(sourceObject), undefined, defaultComparer, `Transformer-${(transformer).name}-${sourceIdentifier}`, undefined); } onBecomeUnobserved() { const lastValue = this.value; diff --git a/src/core/computedvalue.ts b/src/core/computedvalue.ts index fb48c6fe2..6c73a2e76 100644 --- a/src/core/computedvalue.ts +++ b/src/core/computedvalue.ts @@ -5,6 +5,7 @@ import {createAction} from "./action"; import {createInstanceofPredicate, getNextId, valueDidChange, invariant, Lambda, unique, joinStrings, primitiveSymbol, toPrimitive} from "../utils/utils"; import {isSpyEnabled, spyReport} from "../core/spy"; import {autorun} from "../api/autorun"; +import {EqualsComparer} from "../types/comparer"; import {IValueDidChange} from "../types/observablevalue"; import {getMessage} from "../utils/messages"; @@ -57,12 +58,13 @@ export class ComputedValue implements IObservable, IComputedValue, IDeriva * * The `name` property is for debug purposes only. * + * JW: Update * 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. * 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: EqualsComparer, name: string, setter?: (v: T) => void) { this.name = name || "ComputedValue@" + getNextId(); if (setter) this.setter = createAction(name + "-setter", setter) as any; @@ -135,7 +137,7 @@ export class ComputedValue implements IObservable, IComputedValue, IDeriva } const oldValue = this.value; const newValue = this.value = this.computeValue(true); - return isCaughtException(newValue) || valueDidChange(this.compareStructural, newValue, oldValue); + return isCaughtException(newValue) || valueDidChange(newValue, oldValue, this.equals); } computeValue(track: boolean) { diff --git a/src/mobx.ts b/src/mobx.ts index 6ec9ae773..752d57de5 100644 --- a/src/mobx.ts +++ b/src/mobx.ts @@ -57,6 +57,8 @@ export { Lambda, isArrayLike } from "./utils/ut export { Iterator } from "./utils/iterable"; export { IObserverTree, IDependencyTree } from "./api/extras"; +export { EqualsComparer } from "./types/comparer"; + import { resetGlobalState, shareGlobalState, getGlobalState } from "./core/globalstate"; import { IDerivation } from "./core/derivation"; import { IDepTreeNode } from "./core/observable"; diff --git a/src/types/comparer.ts b/src/types/comparer.ts index 510e2febc..698eedc11 100644 --- a/src/types/comparer.ts +++ b/src/types/comparer.ts @@ -1,6 +1,6 @@ import { deepEqual } from '../utils/utils'; -export type EqualityComparer = (a: T, b: T) => boolean; +export type EqualsComparer = (a: T, b: T) => boolean; export const equalityComparer = (a: any, b: any) => a == b; export const identityComparer = (a: any, b: any) => a === b; diff --git a/src/types/observableobject.ts b/src/types/observableobject.ts index 01061a65c..3ec081ca9 100644 --- a/src/types/observableobject.ts +++ b/src/types/observableobject.ts @@ -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 {EqualsComparer, defaultComparer} from "../types/comparer"; import {IEnhancer, isModifierDescriptor, IModifierDescriptor} from "../types/modifiers"; import {isAction, defineBoundAction} from "../api/action"; import {getMessage} from "../utils/messages"; @@ -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); } } @@ -135,13 +136,13 @@ export function defineComputedProperty( propName: string, getter, setter, - compareStructural: boolean, + equals: EqualsComparer, 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)); } diff --git a/src/utils/utils.ts b/src/utils/utils.ts index 28d02d26b..93e37244b 100644 --- a/src/utils/utils.ts +++ b/src/utils/utils.ts @@ -94,13 +94,8 @@ 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; +export function valueDidChange(oldValue: any, newValue: any, equals: EqualsComparer): boolean { + return !equals(oldValue, newValue); } const prototypeHasOwnProperty = Object.prototype.hasOwnProperty; @@ -242,5 +237,6 @@ export function toPrimitive(value) { import {globalState} from "../core/globalstate"; import {IObservableArray, isObservableArray} from "../types/observablearray"; +import {EqualsComparer} from "../types/comparer"; import {isObservableMap} from "../types/observablemap"; import {observable} from "../api/observable"; From f6666d8c37c4281d9fde6775395c442405fa07bb Mon Sep 17 00:00:00 2001 From: jamiewinder Date: Sat, 15 Apr 2017 18:30:24 +0100 Subject: [PATCH 03/16] fixes for tests, workaround for build issue --- src/api/autorun.ts | 5 ++--- src/api/computed.ts | 11 ++++++----- src/core/computedvalue.ts | 4 ++-- src/mobx.ts | 3 +-- src/types/comparer.ts | 24 +++++++++++++++--------- src/types/observableobject.ts | 4 ++-- src/utils/utils.ts | 4 ++-- test/api.js | 3 +++ 8 files changed, 33 insertions(+), 25 deletions(-) diff --git a/src/api/autorun.ts b/src/api/autorun.ts index 1db9910a5..7f75b8fd9 100644 --- a/src/api/autorun.ts +++ b/src/api/autorun.ts @@ -3,7 +3,7 @@ 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 {EqualsComparer, defaultComparer, structuralComparer} from "../types/comparer"; +import {IEqualsComparer, defaultComparer, structuralComparer} from "../types/comparer"; import {getMessage} from "../utils/messages"; /** @@ -157,7 +157,7 @@ export interface IReactionOptions { compareStructural?: boolean; /** alias for compareStructural */ struct?: boolean; - equals?: EqualsComparer; + equals?: IEqualsComparer; name?: string; } @@ -199,7 +199,6 @@ export function reaction(expression: (r: IReactionPublic) => T, effect: (arg: let nextValue: T; const equals = opts.equals || (opts.compareStructural || opts.struct) ? structuralComparer : defaultComparer; - const r = new Reaction(opts.name, () => { if (firstTime || (opts.delay as any) < 1) { reactionRunner(); diff --git a/src/api/computed.ts b/src/api/computed.ts index 76e985015..71a172cc4 100644 --- a/src/api/computed.ts +++ b/src/api/computed.ts @@ -1,4 +1,4 @@ -import {EqualsComparer, defaultComparer, structuralComparer} from "../types/comparer"; +import {IEqualsComparer, defaultComparer, structuralComparer} from "../types/comparer"; import {asObservableObject, defineComputedProperty} from "../types/observableobject"; import {invariant} from "../utils/utils"; import {createClassPropertyDecorator} from "../utils/decorators"; @@ -8,7 +8,7 @@ import {getMessage} from "../utils/messages"; export interface IComputedValueOptions { compareStructural?: boolean; struct?: boolean; - equals?: EqualsComparer; + equals?: IEqualsComparer; name?: string; setter?: (value: T) => void; context?: any; @@ -21,15 +21,16 @@ export interface IComputed { struct(target: Object, key: string | symbol, baseDescriptor?: PropertyDescriptor): void; } - -function createComputedDecorator(compareStructural) { +function createComputedDecorator(struct: boolean) { return createClassPropertyDecorator( (target, name, _, __, originalDescriptor) => { invariant(typeof originalDescriptor !== "undefined", getMessage("m009")); invariant(typeof originalDescriptor.get === "function", getMessage("m010")); + const equals = struct ? structuralComparer : defaultComparer; + 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]; diff --git a/src/core/computedvalue.ts b/src/core/computedvalue.ts index 6c73a2e76..c39aab44c 100644 --- a/src/core/computedvalue.ts +++ b/src/core/computedvalue.ts @@ -5,7 +5,7 @@ import {createAction} from "./action"; import {createInstanceofPredicate, getNextId, valueDidChange, invariant, Lambda, unique, joinStrings, primitiveSymbol, toPrimitive} from "../utils/utils"; import {isSpyEnabled, spyReport} from "../core/spy"; import {autorun} from "../api/autorun"; -import {EqualsComparer} from "../types/comparer"; +import {IEqualsComparer} from "../types/comparer"; import {IValueDidChange} from "../types/observablevalue"; import {getMessage} from "../utils/messages"; @@ -64,7 +64,7 @@ export class ComputedValue implements IObservable, IComputedValue, IDeriva * 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. * This is useful for working with vectors, mouse coordinates etc. */ - constructor(public derivation: () => T, public scope: Object | undefined, private equals: EqualsComparer, name: string, setter?: (v: T) => void) { + constructor(public derivation: () => T, public scope: Object | undefined, private equals: IEqualsComparer, name: string, setter?: (v: T) => void) { this.name = name || "ComputedValue@" + getNextId(); if (setter) this.setter = createAction(name + "-setter", setter) as any; diff --git a/src/mobx.ts b/src/mobx.ts index 752d57de5..cd6244ce9 100644 --- a/src/mobx.ts +++ b/src/mobx.ts @@ -19,6 +19,7 @@ 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"; @@ -57,8 +58,6 @@ export { Lambda, isArrayLike } from "./utils/ut export { Iterator } from "./utils/iterable"; export { IObserverTree, IDependencyTree } from "./api/extras"; -export { EqualsComparer } from "./types/comparer"; - import { resetGlobalState, shareGlobalState, getGlobalState } from "./core/globalstate"; import { IDerivation } from "./core/derivation"; import { IDepTreeNode } from "./core/observable"; diff --git a/src/types/comparer.ts b/src/types/comparer.ts index 698eedc11..33f87cf3c 100644 --- a/src/types/comparer.ts +++ b/src/types/comparer.ts @@ -1,13 +1,19 @@ -import { deepEqual } from '../utils/utils'; - -export type EqualsComparer = (a: T, b: T) => boolean; +export interface IEqualsComparer { + (a: T, b: T): boolean; +} -export const equalityComparer = (a: any, b: any) => a == b; -export const identityComparer = (a: any, b: any) => a === b; -export const structuralComparer = (a: any, b: any) => deepEqual(a, b); -export const defaultComparer = (a: any, b: any) => { - if (typeof a === 'number' && typeof b === 'number' && isNaN(a) && isNaN(b)) { +export var identityComparer: IEqualsComparer = (a: any, b: any) => a === b; +export var structuralComparer: IEqualsComparer = (a: any, b: any) => { + if (typeof a === 'number' && typeof b === 'number' && isNaN(a) && isNaN(b)) { + return true; + } + return deepEqual(a, b); +}; +export var defaultComparer: IEqualsComparer = (a: any, b: any) => { + if (typeof a === 'number' && typeof b === 'number' && isNaN(a) && isNaN(b)) { return true; - } + } return identityComparer(a, b); }; + +import { deepEqual } from '../utils/utils'; diff --git a/src/types/observableobject.ts b/src/types/observableobject.ts index 3ec081ca9..8daf7ac8e 100644 --- a/src/types/observableobject.ts +++ b/src/types/observableobject.ts @@ -5,7 +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 {EqualsComparer, defaultComparer} from "../types/comparer"; +import {IEqualsComparer, defaultComparer} from "../types/comparer"; import {IEnhancer, isModifierDescriptor, IModifierDescriptor} from "../types/modifiers"; import {isAction, defineBoundAction} from "../api/action"; import {getMessage} from "../utils/messages"; @@ -136,7 +136,7 @@ export function defineComputedProperty( propName: string, getter, setter, - equals: EqualsComparer, + equals: IEqualsComparer, asInstanceProperty: boolean ) { if (asInstanceProperty) diff --git a/src/utils/utils.ts b/src/utils/utils.ts index 93e37244b..8777f2379 100644 --- a/src/utils/utils.ts +++ b/src/utils/utils.ts @@ -94,7 +94,7 @@ export function objectAssign() { return res; } -export function valueDidChange(oldValue: any, newValue: any, equals: EqualsComparer): boolean { +export function valueDidChange(oldValue: any, newValue: any, equals: IEqualsComparer): boolean { return !equals(oldValue, newValue); } @@ -237,6 +237,6 @@ export function toPrimitive(value) { import {globalState} from "../core/globalstate"; import {IObservableArray, isObservableArray} from "../types/observablearray"; -import {EqualsComparer} from "../types/comparer"; +import {IEqualsComparer, defaultComparer} from "../types/comparer"; import {isObservableMap} from "../types/observablemap"; import {observable} from "../api/observable"; diff --git a/test/api.js b/test/api.js index 5f95ecb42..30408f1e5 100644 --- a/test/api.js +++ b/test/api.js @@ -6,6 +6,7 @@ test('correct api should be exposed', function(t) { 'Atom', 'BaseAtom', // TODO: remove somehow 'IDerivationState', + 'IEqualsComparer', 'IObservableFactories', 'ObservableMap', 'Reaction', @@ -19,6 +20,7 @@ test('correct api should be exposed', function(t) { 'computed', 'createTransformer', 'default', + 'defaultComparer', 'expr', 'extendObservable', 'extendShallowObservable', @@ -40,6 +42,7 @@ test('correct api should be exposed', function(t) { 'reaction', 'runInAction', 'spy', + 'structuralComparer', 'toJS', 'transaction', 'untracked', From f8115d673c1935bfb3c9e45d25ba59799a4ffb7f Mon Sep 17 00:00:00 2001 From: jamiewinder Date: Sat, 15 Apr 2017 18:43:33 +0100 Subject: [PATCH 04/16] fix 'correct api' test --- test/api.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/api.js b/test/api.js index 30408f1e5..3e413ee97 100644 --- a/test/api.js +++ b/test/api.js @@ -6,7 +6,6 @@ test('correct api should be exposed', function(t) { 'Atom', 'BaseAtom', // TODO: remove somehow 'IDerivationState', - 'IEqualsComparer', 'IObservableFactories', 'ObservableMap', 'Reaction', From e650e51a26165955eb52a597e7e59452a8b033c6 Mon Sep 17 00:00:00 2001 From: jamiewinder Date: Sat, 15 Apr 2017 19:30:15 +0100 Subject: [PATCH 05/16] remove valueDidChange, seems to fix build issue --- src/api/autorun.ts | 4 ++-- src/api/computed.ts | 8 +++----- src/core/computedvalue.ts | 4 ++-- src/mobx.ts | 1 + src/types/comparer.ts | 18 +++++++++++------- src/utils/utils.ts | 4 ---- 6 files changed, 19 insertions(+), 20 deletions(-) diff --git a/src/api/autorun.ts b/src/api/autorun.ts index 7f75b8fd9..07c94c218 100644 --- a/src/api/autorun.ts +++ b/src/api/autorun.ts @@ -1,4 +1,4 @@ -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"; @@ -217,7 +217,7 @@ export function reaction(expression: (r: IReactionPublic) => T, effect: (arg: let changed = false; r.track(() => { const v = expression(r); - changed = valueDidChange(nextValue, v, equals); + changed = !equals(nextValue, v); nextValue = v; }); if (firstTime && opts.fireImmediately!) diff --git a/src/api/computed.ts b/src/api/computed.ts index 71a172cc4..de1eac4fb 100644 --- a/src/api/computed.ts +++ b/src/api/computed.ts @@ -21,14 +21,12 @@ export interface IComputed { struct(target: Object, key: string | symbol, baseDescriptor?: PropertyDescriptor): void; } -function createComputedDecorator(struct: boolean) { +function createComputedDecorator(equals: IEqualsComparer) { return createClassPropertyDecorator( (target, name, _, __, originalDescriptor) => { invariant(typeof originalDescriptor !== "undefined", getMessage("m009")); invariant(typeof originalDescriptor.get === "function", getMessage("m010")); - const equals = struct ? structuralComparer : defaultComparer; - const adm = asObservableObject(target, ""); defineComputedProperty(adm, name, originalDescriptor.get, originalDescriptor.set, equals, false); }, @@ -46,8 +44,8 @@ function createComputedDecorator(struct: boolean) { ); } -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; }. diff --git a/src/core/computedvalue.ts b/src/core/computedvalue.ts index c39aab44c..76f3a9ed0 100644 --- a/src/core/computedvalue.ts +++ b/src/core/computedvalue.ts @@ -2,7 +2,7 @@ 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"; @@ -137,7 +137,7 @@ export class ComputedValue implements IObservable, IComputedValue, IDeriva } const oldValue = this.value; const newValue = this.value = this.computeValue(true); - return isCaughtException(newValue) || valueDidChange(newValue, oldValue, this.equals); + return isCaughtException(newValue) || !this.equals(newValue, oldValue); } computeValue(track: boolean) { diff --git a/src/mobx.ts b/src/mobx.ts index cd6244ce9..7a6f75ab1 100644 --- a/src/mobx.ts +++ b/src/mobx.ts @@ -16,6 +16,7 @@ * */ +import { IEqualsComparer, defaultComparer, structuralComparer } from "./types/comparer"; import {registerGlobals} from "./core/globalstate"; registerGlobals(); diff --git a/src/types/comparer.ts b/src/types/comparer.ts index 33f87cf3c..4ec6eec57 100644 --- a/src/types/comparer.ts +++ b/src/types/comparer.ts @@ -1,19 +1,23 @@ +import { deepEqual } from '../utils/utils'; + export interface IEqualsComparer { (a: T, b: T): boolean; } -export var identityComparer: IEqualsComparer = (a: any, b: any) => a === b; -export var structuralComparer: IEqualsComparer = (a: any, b: any) => { +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 var defaultComparer: IEqualsComparer = (a: any, b: any) => { +} + +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); -}; - -import { deepEqual } from '../utils/utils'; +} diff --git a/src/utils/utils.ts b/src/utils/utils.ts index 8777f2379..abae38060 100644 --- a/src/utils/utils.ts +++ b/src/utils/utils.ts @@ -94,10 +94,6 @@ export function objectAssign() { return res; } -export function valueDidChange(oldValue: any, newValue: any, equals: IEqualsComparer): boolean { - return !equals(oldValue, newValue); -} - const prototypeHasOwnProperty = Object.prototype.hasOwnProperty; export function hasOwnProperty(object: Object, propName: string) { return prototypeHasOwnProperty.call(object, propName); From 61939a975a0ed61516aa74fdedff1f2185b9aa83 Mon Sep 17 00:00:00 2001 From: jamiewinder Date: Sat, 15 Apr 2017 19:39:26 +0100 Subject: [PATCH 06/16] expose @computed.equals --- src/api/computed.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/api/computed.ts b/src/api/computed.ts index de1eac4fb..786beba55 100644 --- a/src/api/computed.ts +++ b/src/api/computed.ts @@ -19,6 +19,7 @@ export interface IComputed { (func: () => T, options: IComputedValueOptions): IComputedValue; (target: Object, key: string | symbol, baseDescriptor?: PropertyDescriptor): void; struct(target: Object, key: string | symbol, baseDescriptor?: PropertyDescriptor): void; + equals(equals: IEqualsComparer): PropertyDecorator; } function createComputedDecorator(equals: IEqualsComparer) { @@ -67,3 +68,4 @@ export var computed: IComputed = ( ) as any; computed.struct = computedStructDecorator; +computed.equals = createComputedDecorator; \ No newline at end of file From 8abf3d1d3b4b8948d1e6e6f7377c71e3acdbc51f Mon Sep 17 00:00:00 2001 From: jamiewinder Date: Sat, 15 Apr 2017 19:58:41 +0100 Subject: [PATCH 07/16] update computedvalue constructor jsdoc --- src/core/computedvalue.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/core/computedvalue.ts b/src/core/computedvalue.ts index 76f3a9ed0..d2b85684a 100644 --- a/src/core/computedvalue.ts +++ b/src/core/computedvalue.ts @@ -58,10 +58,11 @@ export class ComputedValue implements IObservable, IComputedValue, IDeriva * * The `name` property is for debug purposes only. * - * JW: Update - * 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 equals: IEqualsComparer, name: string, setter?: (v: T) => void) { From 6dc991dfe925fffd6be3f45df88771dad5f775ad Mon Sep 17 00:00:00 2001 From: jamiewinder Date: Sat, 15 Apr 2017 21:43:45 +0100 Subject: [PATCH 08/16] fixes equals option with computed boxed values. equals function not invoked in first case, or from/to caught exceptions. added tests. --- src/api/computed.ts | 2 +- src/core/computedvalue.ts | 8 ++- test/babel/babel-tests.js | 39 ++++++++++++++ test/typescript/typescript-tests.ts | 83 +++++++++++++++++++++++++++++ 4 files changed, 129 insertions(+), 3 deletions(-) diff --git a/src/api/computed.ts b/src/api/computed.ts index 786beba55..22206256b 100644 --- a/src/api/computed.ts +++ b/src/api/computed.ts @@ -62,7 +62,7 @@ export var computed: IComputed = ( const opts: IComputedValueOptions = typeof arg2 === "object" ? arg2 : {}; opts.setter = typeof arg2 === "function" ? arg2 : opts.setter; - const equals = opts.equals || (opts.compareStructural || opts.struct) ? structuralComparer : defaultComparer; + 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; diff --git a/src/core/computedvalue.ts b/src/core/computedvalue.ts index d2b85684a..593d87cec 100644 --- a/src/core/computedvalue.ts +++ b/src/core/computedvalue.ts @@ -47,7 +47,7 @@ export class ComputedValue implements IObservable, IComputedValue, 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; @@ -138,7 +138,11 @@ export class ComputedValue implements IObservable, IComputedValue, IDeriva } const oldValue = this.value; const newValue = this.value = this.computeValue(true); - return isCaughtException(newValue) || !this.equals(newValue, oldValue); + return ( + isCaughtException(oldValue) || + isCaughtException(newValue) || + !this.equals(oldValue, newValue) + ); } computeValue(track: boolean) { diff --git a/test/babel/babel-tests.js b/test/babel/babel-tests.js index 45a3629a2..8f82f0860 100644 --- a/test/babel/babel-tests.js +++ b/test/babel/babel-tests.js @@ -873,3 +873,42 @@ test("action.bound binds (Babel)", t=> { t.end(); }) + +test("@computed.equals (Babel)", t => { + const sameTime = (a, b) => { + return (a != null && b != null) + ? a.hour === b.hour && a.minute === b.minute + : a === b; + } + 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(); +}); \ No newline at end of file diff --git a/test/typescript/typescript-tests.ts b/test/typescript/typescript-tests.ts index b4606a7bd..877cb2273 100644 --- a/test/typescript/typescript-tests.ts +++ b/test/typescript/typescript-tests.ts @@ -1120,3 +1120,86 @@ test("803 - action.bound and action preserve type info", t => { const bound2 = action.bound(function () {}) as (() => void) t.end() }) + +test("@computed.equals (TS)", t => { + const sameTime = (a: Time, b: Time) => { + return (a != null && b != null) + ? a.hour === b.hour && a.minute === b.minute + : a === b; + } + class Time { + constructor(hour: number, minute: number) { + this.hour = hour; + this.minute = minute; + } + + @observable public hour: number; + @observable public minute: number; + + @computed.equals(sameTime) public get time() { + return { hour: this.hour, minute: this.minute }; + } + } + const time = new Time(9, 0); + + const changes: Array<{ hour: number, minute: number }> = []; + 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(); +}); + + +test("computed equals comparer", t => { + const comparisons: Array<{ from: string, to: string }> = []; + const comparer = (from: string, to: string) => { + comparisons.push({ from, to }); + return from === to; + }; + + const left = observable("A"); + const right = observable("B"); + const combinedToLowerCase = computed( + () => left.get().toLowerCase() + right.get().toLowerCase(), + { equals: comparer } + ); + + const values: Array = []; + const disposeAutorun = 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 *from* CaughtException in the computed won't cause a comparison + left.set("D"); + t.deepEqual(comparisons, [{ from: "ab", to: "cb" }]); + + // Another value change will cause a comparison + right.set("E"); + t.deepEqual(comparisons, [{ from: "ab", to: "cb" }, { from: "db", to: "de" }]); + + t.deepEqual(values, ["ab", "cb", "db", "de"]); + + disposeAutorun(); + + t.end(); +}); \ No newline at end of file From aef91c7b7a8ed37ffb22c7e3b3009708891b63e1 Mon Sep 17 00:00:00 2001 From: jamiewinder Date: Sat, 15 Apr 2017 21:56:38 +0100 Subject: [PATCH 09/16] expand on the computed equals invocation test --- test/typescript/typescript-tests.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/test/typescript/typescript-tests.ts b/test/typescript/typescript-tests.ts index 877cb2273..2468aa5b1 100644 --- a/test/typescript/typescript-tests.ts +++ b/test/typescript/typescript-tests.ts @@ -1161,7 +1161,7 @@ test("@computed.equals (TS)", t => { }); -test("computed equals comparer", t => { +test("computed equals function only invoked when necessary", t => { const comparisons: Array<{ from: string, to: string }> = []; const comparer = (from: string, to: string) => { comparisons.push({ from, to }); @@ -1189,15 +1189,20 @@ test("computed equals comparer", t => { 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("E"); - t.deepEqual(comparisons, [{ from: "ab", to: "cb" }, { from: "db", to: "de" }]); + right.set("F"); + t.deepEqual(comparisons, [{ from: "ab", to: "cb" }, { from: "de", to: "df" }]); - t.deepEqual(values, ["ab", "cb", "db", "de"]); + t.deepEqual(values, ["ab", "cb", "de", "df"]); disposeAutorun(); From e9b7c25cf9a06f0bcc0275e457cbb1c08d8067e8 Mon Sep 17 00:00:00 2001 From: jamiewinder Date: Sat, 15 Apr 2017 22:18:57 +0100 Subject: [PATCH 10/16] no need for undefined checks on computed.equals tests any more --- test/babel/babel-tests.js | 6 +----- test/typescript/typescript-tests.ts | 6 +----- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/test/babel/babel-tests.js b/test/babel/babel-tests.js index 8f82f0860..2cc2bd13e 100644 --- a/test/babel/babel-tests.js +++ b/test/babel/babel-tests.js @@ -875,11 +875,7 @@ test("action.bound binds (Babel)", t=> { }) test("@computed.equals (Babel)", t => { - const sameTime = (a, b) => { - return (a != null && b != null) - ? a.hour === b.hour && a.minute === b.minute - : a === b; - } + const sameTime = (from, to) => from.hour === to.hour && from.minute === to.minute; class Time { constructor(hour, minute) { this.hour = hour; diff --git a/test/typescript/typescript-tests.ts b/test/typescript/typescript-tests.ts index 2468aa5b1..2719e60ed 100644 --- a/test/typescript/typescript-tests.ts +++ b/test/typescript/typescript-tests.ts @@ -1122,11 +1122,7 @@ test("803 - action.bound and action preserve type info", t => { }) test("@computed.equals (TS)", t => { - const sameTime = (a: Time, b: Time) => { - return (a != null && b != null) - ? a.hour === b.hour && a.minute === b.minute - : a === b; - } + const sameTime = (from: Time, to: Time) => from.hour === to.hour && from.minute === to.minute; class Time { constructor(hour: number, minute: number) { this.hour = hour; From 2380ffc9c3cbd76c3261a53d4ae9652d33e85eb3 Mon Sep 17 00:00:00 2001 From: jamiewinder Date: Sun, 16 Apr 2017 20:16:47 +0100 Subject: [PATCH 11/16] fix reaction use of equals; only invoke after first time --- src/api/autorun.ts | 14 ++++----- test/observables.js | 48 ++++++++++++++++++++++++++++ test/reaction.js | 21 +++++++++++++ test/typescript/typescript-tests.ts | 49 ----------------------------- 4 files changed, 76 insertions(+), 56 deletions(-) diff --git a/src/api/autorun.ts b/src/api/autorun.ts index 07c94c218..74bfc6702 100644 --- a/src/api/autorun.ts +++ b/src/api/autorun.ts @@ -196,9 +196,9 @@ export function reaction(expression: (r: IReactionPublic) => T, effect: (arg: let firstTime = true; let isScheduled = false; - let nextValue: T; + let value: T; - const equals = opts.equals || (opts.compareStructural || opts.struct) ? structuralComparer : defaultComparer; + 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(); @@ -216,14 +216,14 @@ export function reaction(expression: (r: IReactionPublic) => T, effect: (arg: return; let changed = false; r.track(() => { - const v = expression(r); - changed = !equals(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; } diff --git a/test/observables.js b/test/observables.js index fbe36c7a7..b337672f3 100644 --- a/test/observables.js +++ b/test/observables.js @@ -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 comparer = (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: comparer } + ); + + 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(); +}); diff --git a/test/reaction.js b/test/reaction.js index d3f663a41..d4dd68e86 100644 --- a/test/reaction.js +++ b/test/reaction.js @@ -284,3 +284,24 @@ test("do not rerun if prev & next expr output is NaN", t => { t.deepEqual(valuesS, [ 'a', 'NaN', 'b']); t.end(); }) + +test("reaction uses equals", t => { + const o = mobx.observable("a"); + const values = []; + const disposeReaction = mobx.reaction( + () => o.get(), + (value) => values.push(value.toLowerCase()), + { equals: (from, to) => from.toUpperCase() === to.toUpperCase(), fireImmediately: true } + ); + t.deepEqual(values, ["a"]); + o.set("A"); + t.deepEqual(values, ["a"]); + o.set("B"); + t.deepEqual(values, ["a", "b"]); + o.set("A"); + t.deepEqual(values, ["a", "b", "a"]); + + disposeReaction(); + + t.end(); +}); \ No newline at end of file diff --git a/test/typescript/typescript-tests.ts b/test/typescript/typescript-tests.ts index 2719e60ed..2662cfddf 100644 --- a/test/typescript/typescript-tests.ts +++ b/test/typescript/typescript-tests.ts @@ -1153,54 +1153,5 @@ test("@computed.equals (TS)", t => { disposeAutorun(); - t.end(); -}); - - -test("computed equals function only invoked when necessary", t => { - const comparisons: Array<{ from: string, to: string }> = []; - const comparer = (from: string, to: string) => { - comparisons.push({ from, to }); - return from === to; - }; - - const left = observable("A"); - const right = observable("B"); - const combinedToLowerCase = computed( - () => left.get().toLowerCase() + right.get().toLowerCase(), - { equals: comparer } - ); - - const values: Array = []; - const disposeAutorun = 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(); }); \ No newline at end of file From 180cf30e83c435ade5f7e5d760e785bb5bb33469 Mon Sep 17 00:00:00 2001 From: jamiewinder Date: Sun, 16 Apr 2017 20:51:38 +0100 Subject: [PATCH 12/16] another reaction-with-equals test --- test/observables.js | 4 ++-- test/reaction.js | 51 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/test/observables.js b/test/observables.js index b337672f3..03bdc6055 100644 --- a/test/observables.js +++ b/test/observables.js @@ -1733,7 +1733,7 @@ test('observables should not fail when ES6 Map is missing', t => { test("computed equals function only invoked when necessary", t => { const comparisons = []; - const comparer = (from, to) => { + const loggingComparer = (from, to) => { comparisons.push({ from, to }); return from === to; }; @@ -1742,7 +1742,7 @@ test("computed equals function only invoked when necessary", t => { const right = mobx.observable("B"); const combinedToLowerCase = mobx.computed( () => left.get().toLowerCase() + right.get().toLowerCase(), - { equals: comparer } + { equals: loggingComparer } ); const values = []; diff --git a/test/reaction.js b/test/reaction.js index d4dd68e86..37c893fc8 100644 --- a/test/reaction.js +++ b/test/reaction.js @@ -304,4 +304,53 @@ test("reaction uses equals", t => { disposeReaction(); t.end(); -}); \ No newline at end of file +}); + + +test("reaction 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 values = []; + const disposeReaction = mobx.reaction( + () => left.get().toLowerCase() + right.get().toLowerCase(), + (value) => values.push(value), + { equals: loggingComparer, fireImmediately: true } + ); + + // 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" }]); + + // Exception in the reaction expression won't cause a comparison + left.set(null); + t.deepEqual(comparisons, [{ from: "ab", to: "cb" }]); + + // Another exception in the reaction expression won't cause a comparison + right.set(null); + t.deepEqual(comparisons, [{ from: "ab", to: "cb" }]); + + // Transition from exception in the expression will cause a comparison with the last valid value + left.set("D"); + right.set("E"); + t.deepEqual(comparisons, [{ from: "ab", to: "cb" }, { from: 'cb', to: 'de' }]); + + // Another value change will cause a comparison + right.set("F"); + t.deepEqual(comparisons, [{ from: "ab", to: "cb" }, { from: 'cb', to: 'de' }, { from: "de", to: "df" }]); + + t.deepEqual(values, ["ab", "cb", "de", "df"]); + + disposeReaction(); + + t.end(); +}); From a10e414f2dfeb5d98c5b3aa0959547a99a109bc1 Mon Sep 17 00:00:00 2001 From: jamiewinder Date: Tue, 27 Jun 2017 11:54:28 +0100 Subject: [PATCH 13/16] namespace comparers --- src/api/autorun.ts | 9 +++++++-- src/api/computed.ts | 13 +++++++++---- src/api/createtransformer.ts | 4 ++-- src/mobx.ts | 5 ++--- src/types/comparer.ts | 24 +++++++++++++++--------- src/types/observableobject.ts | 5 ++--- src/utils/utils.ts | 1 - test/api.js | 3 +-- 8 files changed, 38 insertions(+), 26 deletions(-) diff --git a/src/api/autorun.ts b/src/api/autorun.ts index 573e4d81a..cad07fb6e 100644 --- a/src/api/autorun.ts +++ b/src/api/autorun.ts @@ -3,7 +3,7 @@ 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 {IEqualsComparer, comparer} from "../types/comparer"; import {getMessage} from "../utils/messages"; /** @@ -198,7 +198,12 @@ export function reaction(expression: (r: IReactionPublic) => T, effect: (arg: let isScheduled = false; let value: T; - const equals = opts.equals ? opts.equals : (opts.compareStructural || opts.struct) ? structuralComparer : defaultComparer; + const equals = opts.equals + ? opts.equals + : (opts.compareStructural || opts.struct) + ? comparer.structural + : comparer.default; + const r = new Reaction(opts.name, () => { if (firstTime || (opts.delay as any) < 1) { reactionRunner(); diff --git a/src/api/computed.ts b/src/api/computed.ts index 22206256b..6b5a4aa60 100644 --- a/src/api/computed.ts +++ b/src/api/computed.ts @@ -1,4 +1,4 @@ -import {IEqualsComparer, defaultComparer, structuralComparer} from "../types/comparer"; +import {IEqualsComparer, comparer} from "../types/comparer"; import {asObservableObject, defineComputedProperty} from "../types/observableobject"; import {invariant} from "../utils/utils"; import {createClassPropertyDecorator} from "../utils/decorators"; @@ -45,8 +45,8 @@ function createComputedDecorator(equals: IEqualsComparer) { ); } -const computedDecorator = createComputedDecorator(defaultComparer); -const computedStructDecorator = createComputedDecorator(structuralComparer); +const computedDecorator = createComputedDecorator(comparer.default); +const computedStructDecorator = createComputedDecorator(comparer.structural); /** * Decorator for class properties: @computed get value() { return expr; }. @@ -62,7 +62,12 @@ export var computed: IComputed = ( const opts: IComputedValueOptions = typeof arg2 === "object" ? arg2 : {}; opts.setter = typeof arg2 === "function" ? arg2 : opts.setter; - const equals = opts.equals ? opts.equals : (opts.compareStructural || opts.struct) ? structuralComparer : defaultComparer; + const equals = opts.equals + ? opts.equals + : (opts.compareStructural || opts.struct) + ? comparer.structural + : comparer.default; + return new ComputedValue(arg1, opts.context, equals, opts.name || arg1.name || "", opts.setter); } ) as any; diff --git a/src/api/createtransformer.ts b/src/api/createtransformer.ts index 33a64b639..4ee34fd0d 100644 --- a/src/api/createtransformer.ts +++ b/src/api/createtransformer.ts @@ -1,6 +1,6 @@ import {ComputedValue} from "../core/computedvalue"; import {globalState} from "../core/globalstate"; -import {defaultComparer} from "../types/comparer"; +import {comparer} from "../types/comparer"; import {invariant, getNextId, addHiddenProp} from "../utils/utils"; export type ITransformer = (object: A) => B; @@ -18,7 +18,7 @@ export function createTransformer(transformer: ITransformer, onClean // Local transformer class specifically for this transformer class Transformer extends ComputedValue { constructor(private sourceIdentifier: string, private sourceObject: A) { - super(() => transformer(sourceObject), undefined, defaultComparer, `Transformer-${(transformer).name}-${sourceIdentifier}`, undefined); + super(() => transformer(sourceObject), undefined, comparer.default, `Transformer-${(transformer).name}-${sourceIdentifier}`, undefined); } onBecomeUnobserved() { const lastValue = this.value; diff --git a/src/mobx.ts b/src/mobx.ts index 7a6f75ab1..faca0869a 100644 --- a/src/mobx.ts +++ b/src/mobx.ts @@ -16,11 +16,10 @@ * */ -import { IEqualsComparer, defaultComparer, structuralComparer } from "./types/comparer"; import {registerGlobals} from "./core/globalstate"; registerGlobals(); -export { IEqualsComparer, defaultComparer, structuralComparer } from "./types/comparer"; +export { IEqualsComparer, comparer } from "./types/comparer"; export { IAtom, Atom, BaseAtom } from "./core/atom"; export { IObservable, IDepTreeNode } from "./core/observable"; export { Reaction, IReactionPublic, IReactionDisposer } from "./core/reaction"; @@ -95,7 +94,7 @@ export const extras = { declare var __MOBX_DEVTOOLS_GLOBAL_HOOK__: { injectMobx: ((any) => void)}; declare var module: { exports: any }; if (typeof __MOBX_DEVTOOLS_GLOBAL_HOOK__ === "object") { - __MOBX_DEVTOOLS_GLOBAL_HOOK__.injectMobx(module.exports) + __MOBX_DEVTOOLS_GLOBAL_HOOK__.injectMobx(module.exports); } // TODO: remove in 4.0, temporarily incompatibility fix for mobx-react@4.1.0 which accidentally uses default exports diff --git a/src/types/comparer.ts b/src/types/comparer.ts index 4ec6eec57..9445fefc3 100644 --- a/src/types/comparer.ts +++ b/src/types/comparer.ts @@ -1,23 +1,29 @@ import { deepEqual } from '../utils/utils'; export interface IEqualsComparer { - (a: T, b: T): boolean; + (a: T, b: T): boolean; } -export function identityComparer(a: any, b: any): boolean { - return a === b; +function identityComparer(a: any, b: any): boolean { + return a === b; } -export function structuralComparer(a: any, b: any): boolean { +function structuralComparer(a: any, b: any): boolean { if (typeof a === 'number' && typeof b === 'number' && isNaN(a) && isNaN(b)) { - return true; + return true; } - return deepEqual(a, b); + return deepEqual(a, b); } -export function defaultComparer(a: any, b: any): boolean { +function defaultComparer(a: any, b: any): boolean { if (typeof a === 'number' && typeof b === 'number' && isNaN(a) && isNaN(b)) { - return true; + return true; } - return identityComparer(a, b); + return identityComparer(a, b); } + +export const comparer = { + identity: identityComparer, + structural: structuralComparer, + default: defaultComparer +}; diff --git a/src/types/observableobject.ts b/src/types/observableobject.ts index 8daf7ac8e..d2a6a6c8e 100644 --- a/src/types/observableobject.ts +++ b/src/types/observableobject.ts @@ -5,13 +5,12 @@ 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 {IEqualsComparer, comparer} from "../types/comparer"; import {IEnhancer, isModifierDescriptor, IModifierDescriptor} from "../types/modifiers"; import {isAction, defineBoundAction} from "../api/action"; import {getMessage} from "../utils/messages"; - export interface IObservableObject { "observable-object": IObservableObject; } @@ -101,7 +100,7 @@ export function defineObservablePropertyFromDescriptor(adm: ObservableObjectAdmi } } else { // get x() { return 3 } set x(v) { } - defineComputedProperty(adm, propName, descriptor.get, descriptor.set, defaultComparer, true); + defineComputedProperty(adm, propName, descriptor.get, descriptor.set, comparer.default, true); } } diff --git a/src/utils/utils.ts b/src/utils/utils.ts index abae38060..cfba77887 100644 --- a/src/utils/utils.ts +++ b/src/utils/utils.ts @@ -233,6 +233,5 @@ 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"; diff --git a/test/api.js b/test/api.js index 3e413ee97..e40caed87 100644 --- a/test/api.js +++ b/test/api.js @@ -17,9 +17,9 @@ test('correct api should be exposed', function(t) { 'autorun', 'autorunAsync', 'computed', + 'comparer', 'createTransformer', 'default', - 'defaultComparer', 'expr', 'extendObservable', 'extendShallowObservable', @@ -41,7 +41,6 @@ test('correct api should be exposed', function(t) { 'reaction', 'runInAction', 'spy', - 'structuralComparer', 'toJS', 'transaction', 'untracked', From 61c59053aa9a9189403a70b2c2ee1791cf4ca5a7 Mon Sep 17 00:00:00 2001 From: jamiewinder Date: Tue, 27 Jun 2017 12:09:47 +0100 Subject: [PATCH 14/16] add extendObservable with custom comparer test --- src/mobx.ts | 2 +- test/typescript/typescript-tests.ts | 39 +++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/src/mobx.ts b/src/mobx.ts index faca0869a..a71854b05 100644 --- a/src/mobx.ts +++ b/src/mobx.ts @@ -94,7 +94,7 @@ export const extras = { declare var __MOBX_DEVTOOLS_GLOBAL_HOOK__: { injectMobx: ((any) => void)}; declare var module: { exports: any }; if (typeof __MOBX_DEVTOOLS_GLOBAL_HOOK__ === "object") { - __MOBX_DEVTOOLS_GLOBAL_HOOK__.injectMobx(module.exports); + __MOBX_DEVTOOLS_GLOBAL_HOOK__.injectMobx(module.exports) } // TODO: remove in 4.0, temporarily incompatibility fix for mobx-react@4.1.0 which accidentally uses default exports diff --git a/test/typescript/typescript-tests.ts b/test/typescript/typescript-tests.ts index 2662cfddf..86891c3ae 100644 --- a/test/typescript/typescript-tests.ts +++ b/test/typescript/typescript-tests.ts @@ -1153,5 +1153,44 @@ test("@computed.equals (TS)", t => { disposeAutorun(); + t.end(); +}); + +test("computed comparer works with extendObservable (TS)", t => { + const sameTime = (from: Time, to: Time) => from.hour === to.hour && from.minute === to.minute; + class Time { + constructor(hour: number, minute: number) { + this.hour = hour; + this.minute = minute; + extendObservable(this, { + hour, + minute, + time: computed(() => { + return { hour: this.hour, minute: this.minute }; + }, { equals: sameTime }) + }) + } + + public hour: number; + public minute: number; + public time: { hour: number, minute: number }; + } + const time = new Time(9, 0); + + const changes: Array<{ hour: number, minute: number }> = []; + 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(); }); \ No newline at end of file From 3fde3499080066875154c2f380c940c6843f48f9 Mon Sep 17 00:00:00 2001 From: jamiewinder Date: Thu, 29 Jun 2017 17:23:15 +0100 Subject: [PATCH 15/16] fix import order, add comparer to 'everything' export --- src/mobx.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/mobx.ts b/src/mobx.ts index bb8ac774a..4633a2bd2 100644 --- a/src/mobx.ts +++ b/src/mobx.ts @@ -17,7 +17,6 @@ */ -export { IEqualsComparer, comparer } from "./types/comparer"; export { IObservable, IDepTreeNode } from "./core/observable"; export { Reaction, IReactionPublic, IReactionDisposer } from "./core/reaction"; export { IDerivation, untracked, IDerivationState } from "./core/derivation"; @@ -33,6 +32,7 @@ export { useStrict, isStrictModeEnabled, IAction } from "./core/act export { spy } from "./core/spy"; export { IComputedValue } from "./core/computedvalue"; +export { IEqualsComparer, comparer } from "./types/comparer"; export { asReference, asFlat, asStructure, asMap } from "./types/modifiers-old"; export { IModifierDescriptor, IEnhancer, isModifierDescriptor } from "./types/modifiers"; export { IInterceptable, IInterceptor } from "./types/intercept-utils"; @@ -115,6 +115,7 @@ import { IAtom, Atom, BaseAtom } from "./core/ato import { useStrict, isStrictModeEnabled, IAction } from "./core/action"; import { spy } from "./core/spy"; import { IComputedValue } from "./core/computedvalue"; +import { IEqualsComparer, comparer } from "./types/comparer"; import { asReference, asFlat, asStructure, asMap } from "./types/modifiers-old"; import { IModifierDescriptor, IEnhancer, isModifierDescriptor } from "./types/modifiers"; import { IInterceptable, IInterceptor } from "./types/intercept-utils"; @@ -146,6 +147,7 @@ const everything = { Atom, BaseAtom, useStrict, isStrictModeEnabled, spy, + comparer, asReference, asFlat, asStructure, asMap, isModifierDescriptor, isObservableObject, From a6b24c1070776f45f7dd569b8fdd268e35218703 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Tue, 11 Jul 2017 11:50:28 +0200 Subject: [PATCH 16/16] Fixed compile error, added deprecation notices --- src/api/autorun.ts | 4 ++-- src/api/computed.ts | 4 ++-- src/core/computedvalue.ts | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/api/autorun.ts b/src/api/autorun.ts index cfd47fe89..f7b5d0815 100644 --- a/src/api/autorun.ts +++ b/src/api/autorun.ts @@ -154,9 +154,9 @@ export interface IReactionOptions { context?: any; fireImmediately?: boolean; delay?: number; - compareStructural?: boolean; + compareStructural?: boolean; // TODO: remove in 4.0 in favor of equals /** alias for compareStructural */ - struct?: boolean; + struct?: boolean; // TODO: remove in 4.0 in favor of equals equals?: IEqualsComparer; name?: string; } diff --git a/src/api/computed.ts b/src/api/computed.ts index 6b5a4aa60..cf0396c08 100644 --- a/src/api/computed.ts +++ b/src/api/computed.ts @@ -6,8 +6,8 @@ import {ComputedValue, IComputedValue} from "../core/computedvalue"; import {getMessage} from "../utils/messages"; export interface IComputedValueOptions { - compareStructural?: boolean; - struct?: boolean; + compareStructural?: boolean; // TODO: remove in 4.0 in favor of equals + struct?: boolean; // TODO: remove in 4.0 in favor of equals equals?: IEqualsComparer; name?: string; setter?: (value: T) => void; diff --git a/src/core/computedvalue.ts b/src/core/computedvalue.ts index ddbcb3442..32bee5bd9 100644 --- a/src/core/computedvalue.ts +++ b/src/core/computedvalue.ts @@ -2,7 +2,7 @@ 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 "./spy"; import {autorun} from "../api/autorun"; import {IEqualsComparer} from "../types/comparer";