Skip to content

Commit

Permalink
Merge pull request #1249 from Ulmanb/master
Browse files Browse the repository at this point in the history
make deep NaN === deep NaN for computed values when using compareStructural
  • Loading branch information
mweststrate authored Nov 23, 2017
2 parents 7fdf5f3 + 6052a7c commit 307405a
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 8 deletions.
10 changes: 2 additions & 8 deletions src/types/comparer.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { deepEqual } from "../utils/utils"
import { deepEqual, areBothNaN } from "../utils/utils"

export interface IEqualsComparer<T> {
(a: T, b: T): boolean
Expand All @@ -9,17 +9,11 @@ function identityComparer(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 deepEqual(a, b)
}

function defaultComparer(a: any, b: any): boolean {
if (typeof a === "number" && typeof b === "number" && isNaN(a) && isNaN(b)) {
return true
}
return identityComparer(a, b)
return areBothNaN(a,b) || identityComparer(a, b)
}

export const comparer = {
Expand Down
5 changes: 5 additions & 0 deletions src/utils/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ export function getEnumerableKeys(obj) {
export function deepEqual(a, b) {
if (a === null && b === null) return true
if (a === undefined && b === undefined) return true
if (areBothNaN(a, b)) return true
if (typeof a !== "object") return a === b
const aIsArray = isArrayLike(a)
const aIsMap = isMapLike(a)
Expand Down Expand Up @@ -191,6 +192,10 @@ export function createInstanceofPredicate<T>(
} as any
}

export function areBothNaN(a: any, b: any): boolean {
return (typeof a === "number" && typeof b === "number" && isNaN(a) && isNaN(b));
}

/**
* Returns whether the argument is an array, disregarding observability.
*/
Expand Down
25 changes: 25 additions & 0 deletions test/observables.js
Original file line number Diff line number Diff line change
Expand Up @@ -1223,6 +1223,31 @@ test("computed values believe NaN === NaN", function(t) {
t.end()
})

test("computed values believe deep NaN === deep NaN when using compareStructural", function(t) {
var a = observable({ b: { a: 1 } })
var c = computed(function() {
return a.b
}, {compareStructural: true})

var buf = new buffer()
c.observe((newValue) => {
buf(newValue)
});

a.b = { a: NaN }
a.b = { a: NaN }
a.b = { a: NaN }
a.b = { a: 2 }
a.b = { a: NaN }

var bufArray = buf.toArray()
t.equal(isNaN(bufArray[0].b), true)
t.deepEqual(bufArray[1], { a: 2 })
t.deepEqual(isNaN(bufArray[2].b), true)
t.equal(bufArray.length, 3)
t.end()
})

test.skip("issue 65; transaction causing transaction", function(t) {
// MWE: disabled, bad test; depends on transaction being tracked, transaction should not be used in computed!
var x = mobx.observable({
Expand Down

0 comments on commit 307405a

Please sign in to comment.