From 18a349ce8c26d04ccd980938d87964ab2c5771e3 Mon Sep 17 00:00:00 2001 From: Mayness <1095346833@qq.com> Date: Wed, 23 Oct 2019 10:53:43 -0500 Subject: [PATCH] fix(reactivity): account for NaN in value change checks (#361) --- packages/reactivity/__tests__/collections/Map.spec.ts | 8 ++++++++ .../reactivity/__tests__/collections/WeakMap.spec.ts | 10 ++++++++++ packages/reactivity/__tests__/effect.spec.ts | 10 ++++++++++ packages/reactivity/src/baseHandlers.ts | 6 +++--- packages/reactivity/src/collectionHandlers.ts | 6 +++--- packages/runtime-core/src/apiWatch.ts | 11 +++++++++-- packages/shared/src/index.ts | 4 ++++ 7 files changed, 47 insertions(+), 8 deletions(-) diff --git a/packages/reactivity/__tests__/collections/Map.spec.ts b/packages/reactivity/__tests__/collections/Map.spec.ts index 0ae0965bb..b190da32f 100644 --- a/packages/reactivity/__tests__/collections/Map.spec.ts +++ b/packages/reactivity/__tests__/collections/Map.spec.ts @@ -311,5 +311,13 @@ describe('reactivity/collections', () => { map.get(key)!.foo++ expect(dummy).toBe(2) }) + + it('should not be trigger when the value and the old value both are NaN', () => { + const map = reactive(new Map([['foo', NaN]])) + const mapSpy = jest.fn(() => map.get('foo')) + effect(mapSpy) + map.set('foo', NaN) + expect(mapSpy).toHaveBeenCalledTimes(1) + }) }) }) diff --git a/packages/reactivity/__tests__/collections/WeakMap.spec.ts b/packages/reactivity/__tests__/collections/WeakMap.spec.ts index 01e729569..6c82b82dd 100644 --- a/packages/reactivity/__tests__/collections/WeakMap.spec.ts +++ b/packages/reactivity/__tests__/collections/WeakMap.spec.ts @@ -107,5 +107,15 @@ describe('reactivity/collections', () => { observed.get(key).a = 2 expect(dummy).toBe(2) }) + + it('should not be trigger when the value and the old value both are NaN', () => { + const map = new WeakMap() + const key = {} + map.set(key, NaN) + const mapSpy = jest.fn(() => map.get(key)) + effect(mapSpy) + map.set(key, NaN) + expect(mapSpy).toHaveBeenCalledTimes(1) + }) }) }) diff --git a/packages/reactivity/__tests__/effect.spec.ts b/packages/reactivity/__tests__/effect.spec.ts index 32a7fbdfe..9ddd8e059 100644 --- a/packages/reactivity/__tests__/effect.spec.ts +++ b/packages/reactivity/__tests__/effect.spec.ts @@ -691,4 +691,14 @@ describe('reactivity/effect', () => { obj.foo = { prop: 1 } expect(dummy).toBe(1) }) + + it('should not be trigger when the value and the old value both are NaN', () => { + const obj = reactive({ + foo: NaN + }) + const fnSpy = jest.fn(() => obj.foo) + effect(fnSpy) + obj.foo = NaN + expect(fnSpy).toHaveBeenCalledTimes(1) + }) }) diff --git a/packages/reactivity/src/baseHandlers.ts b/packages/reactivity/src/baseHandlers.ts index f75c5b74b..4f4864451 100644 --- a/packages/reactivity/src/baseHandlers.ts +++ b/packages/reactivity/src/baseHandlers.ts @@ -2,7 +2,7 @@ import { reactive, readonly, toRaw } from './reactive' import { OperationTypes } from './operations' import { track, trigger } from './effect' import { LOCKED } from './lock' -import { isObject, hasOwn, isSymbol } from '@vue/shared' +import { isObject, hasOwn, isSymbol, hasChanged } from '@vue/shared' import { isRef } from './ref' const builtInSymbols = new Set( @@ -52,13 +52,13 @@ function set( const extraInfo = { oldValue, newValue: value } if (!hadKey) { trigger(target, OperationTypes.ADD, key, extraInfo) - } else if (value !== oldValue) { + } else if (hasChanged(value, oldValue)) { trigger(target, OperationTypes.SET, key, extraInfo) } } else { if (!hadKey) { trigger(target, OperationTypes.ADD, key) - } else if (value !== oldValue) { + } else if (hasChanged(value, oldValue)) { trigger(target, OperationTypes.SET, key) } } diff --git a/packages/reactivity/src/collectionHandlers.ts b/packages/reactivity/src/collectionHandlers.ts index 7f208583e..9e315261e 100644 --- a/packages/reactivity/src/collectionHandlers.ts +++ b/packages/reactivity/src/collectionHandlers.ts @@ -2,7 +2,7 @@ import { toRaw, reactive, readonly } from './reactive' import { track, trigger } from './effect' import { OperationTypes } from './operations' import { LOCKED } from './lock' -import { isObject, capitalize, hasOwn } from '@vue/shared' +import { isObject, capitalize, hasOwn, hasChanged } from '@vue/shared' export type CollectionTypes = IterableCollections | WeakCollections @@ -73,13 +73,13 @@ function set(this: MapTypes, key: unknown, value: unknown) { const extraInfo = { oldValue, newValue: value } if (!hadKey) { trigger(target, OperationTypes.ADD, key, extraInfo) - } else if (value !== oldValue) { + } else if (hasChanged(value, oldValue)) { trigger(target, OperationTypes.SET, key, extraInfo) } } else { if (!hadKey) { trigger(target, OperationTypes.ADD, key) - } else if (value !== oldValue) { + } else if (hasChanged(value, oldValue)) { trigger(target, OperationTypes.SET, key) } } diff --git a/packages/runtime-core/src/apiWatch.ts b/packages/runtime-core/src/apiWatch.ts index f374940e9..4a8248aa0 100644 --- a/packages/runtime-core/src/apiWatch.ts +++ b/packages/runtime-core/src/apiWatch.ts @@ -7,7 +7,14 @@ import { ReactiveEffectOptions } from '@vue/reactivity' import { queueJob } from './scheduler' -import { EMPTY_OBJ, isObject, isArray, isFunction, isString } from '@vue/shared' +import { + EMPTY_OBJ, + isObject, + isArray, + isFunction, + isString, + hasChanged +} from '@vue/shared' import { recordEffect } from './apiReactivity' import { currentInstance, @@ -144,7 +151,7 @@ function doWatch( return } const newValue = runner() - if (deep || newValue !== oldValue) { + if (deep || hasChanged(newValue, oldValue)) { // cleanup before running cb again if (cleanup) { cleanup() diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index b83e08849..32836f53a 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -67,3 +67,7 @@ export const hyphenate = (str: string): string => { export const capitalize = (str: string): string => { return str.charAt(0).toUpperCase() + str.slice(1) } + +// compare whether a value has changed, accounting for NaN. +export const hasChanged = (value: any, oldValue: any): boolean => + value !== oldValue && (value === value || oldValue === oldValue)