Skip to content

Commit

Permalink
refactor(reactivity): refactor iteration key trigger logic + use more…
Browse files Browse the repository at this point in the history
… robust Map/Set check
  • Loading branch information
yyx990803 committed Sep 14, 2020
1 parent cf1b6c6 commit 0124eac
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 28 deletions.
12 changes: 7 additions & 5 deletions packages/reactivity/src/collectionHandlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import {
capitalize,
hasOwn,
hasChanged,
toRawType
toRawType,
isMap
} from '@vue/shared'

export type CollectionTypes = IterableCollections | WeakCollections
Expand Down Expand Up @@ -129,7 +130,7 @@ function clear(this: IterableCollections) {
const target = toRaw(this)
const hadItems = target.size !== 0
const oldTarget = __DEV__
? target instanceof Map
? isMap(target)
? new Map(target)
: new Set(target)
: undefined
Expand Down Expand Up @@ -185,9 +186,10 @@ function createIterableMethod(
): Iterable & Iterator {
const target = (this as any)[ReactiveFlags.RAW]
const rawTarget = toRaw(target)
const isMap = rawTarget instanceof Map
const isPair = method === 'entries' || (method === Symbol.iterator && isMap)
const isKeyOnly = method === 'keys' && isMap
const targetIsMap = isMap(rawTarget)
const isPair =
method === 'entries' || (method === Symbol.iterator && targetIsMap)
const isKeyOnly = method === 'keys' && targetIsMap
const innerIterator = target[method](...args)
const wrap = isReadonly ? toReadonly : isShallow ? toShallow : toReactive
!isReadonly &&
Expand Down
40 changes: 27 additions & 13 deletions packages/reactivity/src/effect.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { TrackOpTypes, TriggerOpTypes } from './operations'
import { EMPTY_OBJ, isArray, isIntegerKey } from '@vue/shared'
import { EMPTY_OBJ, isArray, isIntegerKey, isMap } from '@vue/shared'

// The main WeakMap that stores {target -> key -> dep} connections.
// Conceptually, it's easier to think of a dependency as a Dep class
Expand Down Expand Up @@ -197,19 +197,33 @@ export function trigger(
if (key !== void 0) {
add(depsMap.get(key))
}

// also run for iteration key on ADD | DELETE | Map.SET
const shouldTriggerIteration =
(type === TriggerOpTypes.ADD &&
(!isArray(target) || isIntegerKey(key))) ||
(type === TriggerOpTypes.DELETE && !isArray(target))
if (
shouldTriggerIteration ||
(type === TriggerOpTypes.SET && target instanceof Map)
) {
add(depsMap.get(isArray(target) ? 'length' : ITERATE_KEY))
}
if (shouldTriggerIteration && target instanceof Map) {
add(depsMap.get(MAP_KEY_ITERATE_KEY))
switch (type) {
case TriggerOpTypes.ADD:
if (!isArray(target)) {
add(depsMap.get(ITERATE_KEY))
if (isMap(target)) {
add(depsMap.get(MAP_KEY_ITERATE_KEY))
}
} else if (isIntegerKey(key)) {
// new index added to array -> length changes
add(depsMap.get('length'))
}
break
case TriggerOpTypes.DELETE:
if (!isArray(target)) {
add(depsMap.get(ITERATE_KEY))
if (isMap(target)) {
add(depsMap.get(MAP_KEY_ITERATE_KEY))
}
}
break
case TriggerOpTypes.SET:
if (isMap(target)) {
add(depsMap.get(ITERATE_KEY))
}
break
}
}

Expand Down
10 changes: 6 additions & 4 deletions packages/runtime-core/src/apiWatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ import {
isString,
hasChanged,
NOOP,
remove
remove,
isMap,
isSet
} from '@vue/shared'
import {
currentInstance,
Expand Down Expand Up @@ -335,12 +337,12 @@ function traverse(value: unknown, seen: Set<unknown> = new Set()) {
for (let i = 0; i < value.length; i++) {
traverse(value[i], seen)
}
} else if (value instanceof Map) {
value.forEach((v, key) => {
} else if (isMap(value)) {
value.forEach((_, key) => {
// to register mutation dep for existing keys
traverse(value.get(key), seen)
})
} else if (value instanceof Set) {
} else if (isSet(value)) {
value.forEach(v => {
traverse(v, seen)
})
Expand Down
8 changes: 5 additions & 3 deletions packages/shared/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,11 @@ export const hasOwn = (
): key is keyof typeof val => hasOwnProperty.call(val, key)

export const isArray = Array.isArray
export const isSet = (val: any): boolean => {
return toRawType(val) === 'Set'
}
export const isMap = (val: unknown): val is Map<any, any> =>
toTypeString(val) === '[object Map]'
export const isSet = (val: unknown): val is Set<any> =>
toTypeString(val) === '[object Set]'

This comment has been minimized.

Copy link
@jods4

jods4 Sep 16, 2020

Contributor

Out of curiosity: is this faster than val instanceof Set?

The other reason I've seen people use this trick is to avoid a dependency on the type (i.e. Set) where it may be absent, e.g. it would have to be polyfilled in IE9.

Given Vue 3 targets IE11 at least, Set would always been defined, so I'm curious if there's another reason.

This comment has been minimized.

Copy link
@07akioni

07akioni May 3, 2021

Contributor

It is much slower than instanceof.

Given:

const s = new Set()
const x = ''

function f1 (st) {
	return st instanceof Set
}

const toString = Object.prototype.toString

function f2 (st) {
	return toString.call(st) === '[object Set]'
}

image

You can see instanceof is much faster.


export const isDate = (val: unknown): val is Date => val instanceof Date
export const isFunction = (val: unknown): val is Function =>
typeof val === 'function'
Expand Down
6 changes: 3 additions & 3 deletions packages/shared/src/toDisplayString.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { isArray, isObject, isPlainObject } from './index'
import { isArray, isMap, isObject, isPlainObject, isSet } from './index'

/**
* For converting {{ interpolation }} values to displayed strings.
Expand All @@ -13,14 +13,14 @@ export const toDisplayString = (val: unknown): string => {
}

const replacer = (_key: string, val: any) => {
if (val instanceof Map) {
if (isMap(val)) {
return {
[`Map(${val.size})`]: [...val.entries()].reduce((entries, [key, val]) => {
;(entries as any)[`${key} =>`] = val
return entries
}, {})
}
} else if (val instanceof Set) {
} else if (isSet(val)) {
return {
[`Set(${val.size})`]: [...val.values()]
}
Expand Down

0 comments on commit 0124eac

Please sign in to comment.