perf(runtime-dom/vModel): remove looseHas if model is Set (#2236)

vuejs · Oct 13, 2020 · 6a554fe · 6a554fe
1 parent 6b8cf99
commit 6a554fe
Show file tree

Hide file tree

Showing 3 changed files with 208 additions and 18 deletions.
diff --git a/packages/runtime-dom/__tests__/directives/vModel.spec.ts b/packages/runtime-dom/__tests__/directives/vModel.spec.ts
@@ -619,7 +619,7 @@ describe('vModel', () => {
  expect(bar.selected).toEqual(true)
  })
 
- it('should work with multiple select', async () => {
+ it('multiple select (model is Array)', async () => {
  const component = defineComponent({
  data() {
  return { value: [] }
@@ -783,6 +783,206 @@ describe('vModel', () => {
  expect(two.selected).toEqual(true)
  })
 
+ it('multiple select (model is Array, option value is object)', async () => {
+ const fooValue = { foo: 1 }
+ const barValue = { bar: 1 }
+
+ const component = defineComponent({
+ data() {
+ return { value: [] }
+ },
+ render() {
+ return [
+ withVModel(
+ h(
+ 'select',
+ {
+ value: null,
+ multiple: true,
+ 'onUpdate:modelValue': setValue.bind(this)
+ },
+ [
+ h('option', { value: fooValue }),
+ h('option', { value: barValue })
+ ]
+ ),
+ this.value
+ )
+ ]
+ }
+ })
+ render(h(component), root)
+
+ await nextTick()
+
+ const input = root.querySelector('select')
+ const [foo, bar] = root.querySelectorAll('option')
+ const data = root._vnode.component.data
+
+ foo.selected = true
+ triggerEvent('change', input)
+ await nextTick()
+ expect(data.value).toMatchObject([fooValue])
+
+ foo.selected = false
+ bar.selected = true
+ triggerEvent('change', input)
+ await nextTick()
+ expect(data.value).toMatchObject([barValue])
+
+ foo.selected = true
+ bar.selected = true
+ triggerEvent('change', input)
+ await nextTick()
+ expect(data.value).toMatchObject([fooValue, barValue])
+
+ foo.selected = false
+ bar.selected = false
+ data.value = [fooValue, barValue]
+ await nextTick()
+ expect(foo.selected).toEqual(true)
+ expect(bar.selected).toEqual(true)
+
+ foo.selected = false
+ bar.selected = false
+ data.value = [{ foo: 1 }, { bar: 1 }]
+ await nextTick()
+ // looseEqual
+ expect(foo.selected).toEqual(true)
+ expect(bar.selected).toEqual(true)
+ })
+
+ it('multiple select (model is Set)', async () => {
+ const component = defineComponent({
+ data() {
+ return { value: new Set() }
+ },
+ render() {
+ return [
+ withVModel(
+ h(
+ 'select',
+ {
+ value: null,
+ multiple: true,
+ 'onUpdate:modelValue': setValue.bind(this)
+ },
+ [h('option', { value: 'foo' }), h('option', { value: 'bar' })]
+ ),
+ this.value
+ )
+ ]
+ }
+ })
+ render(h(component), root)
+
+ const input = root.querySelector('select')
+ const foo = root.querySelector('option[value=foo]')
+ const bar = root.querySelector('option[value=bar]')
+ const data = root._vnode.component.data
+
+ foo.selected = true
+ triggerEvent('change', input)
+ await nextTick()
+ expect(data.value).toMatchObject(new Set(['foo']))
+
+ foo.selected = false
+ bar.selected = true
+ triggerEvent('change', input)
+ await nextTick()
+ expect(data.value).toMatchObject(new Set(['bar']))
+
+ foo.selected = true
+ bar.selected = true
+ triggerEvent('change', input)
+ await nextTick()
+ expect(data.value).toMatchObject(new Set(['foo', 'bar']))
+
+ foo.selected = false
+ bar.selected = false
+ data.value = new Set(['foo'])
+ await nextTick()
+ expect(input.value).toEqual('foo')
+ expect(foo.selected).toEqual(true)
+ expect(bar.selected).toEqual(false)
+
+ foo.selected = false
+ bar.selected = false
+ data.value = new Set(['foo', 'bar'])
+ await nextTick()
+ expect(foo.selected).toEqual(true)
+ expect(bar.selected).toEqual(true)
+ })
+
+ it('multiple select (model is Set, option value is object)', async () => {
+ const fooValue = { foo: 1 }
+ const barValue = { bar: 1 }
+
+ const component = defineComponent({
+ data() {
+ return { value: new Set() }
+ },
+ render() {
+ return [
+ withVModel(
+ h(
+ 'select',
+ {
+ value: null,
+ multiple: true,
+ 'onUpdate:modelValue': setValue.bind(this)
+ },
+ [
+ h('option', { value: fooValue }),
+ h('option', { value: barValue })
+ ]
+ ),
+ this.value
+ )
+ ]
+ }
+ })
+ render(h(component), root)
+
+ await nextTick()
+
+ const input = root.querySelector('select')
+ const [foo, bar] = root.querySelectorAll('option')
+ const data = root._vnode.component.data
+
+ foo.selected = true
+ triggerEvent('change', input)
+ await nextTick()
+ expect(data.value).toMatchObject(new Set([fooValue]))
+
+ foo.selected = false
+ bar.selected = true
+ triggerEvent('change', input)
+ await nextTick()
+ expect(data.value).toMatchObject(new Set([barValue]))
+
+ foo.selected = true
+ bar.selected = true
+ triggerEvent('change', input)
+ await nextTick()
+ expect(data.value).toMatchObject(new Set([fooValue, barValue]))
+
+ foo.selected = false
+ bar.selected = false
+ data.value = new Set([fooValue, barValue])
+ await nextTick()
+ expect(foo.selected).toEqual(true)
+ expect(bar.selected).toEqual(true)
+
+ foo.selected = false
+ bar.selected = false
+ data.value = new Set([{ foo: 1 }, { bar: 1 }])
+ await nextTick()
+ // whithout looseEqual, here is different from Array
+ expect(foo.selected).toEqual(false)
+ expect(bar.selected).toEqual(false)
+ })
+
  it('should work with composition session', async () => {
  const component = defineComponent({
  data() {

diff --git a/packages/runtime-dom/src/directives/vModel.ts b/packages/runtime-dom/src/directives/vModel.ts
@@ -12,8 +12,7 @@ import {
  looseIndexOf,
  invokeArrayFns,
  toNumber,
- isSet,
- looseHas
+ isSet
 } from '@vue/shared'
 
 type AssignerFn = (value: any) => void
@@ -119,12 +118,10 @@ export const vModelCheckbox: ModelDirective<HTMLInputElement> = {
  assign(filtered)
  }
  } else if (isSet(modelValue)) {
- const found = modelValue.has(elementValue)
- if (checked && !found) {
- assign(modelValue.add(elementValue))
- } else if (!checked && found) {
+ if (checked) {
+ modelValue.add(elementValue)
+ } else {
  modelValue.delete(elementValue)
- assign(modelValue)
  }
  } else {
  assign(getCheckboxValue(el, checked))
@@ -148,7 +145,7 @@ function setChecked(
  if (isArray(value)) {
  el.checked = looseIndexOf(value, vnode.props!.value) > -1
  } else if (isSet(value)) {
- el.checked = looseHas(value, vnode.props!.value)
+ el.checked = value.has(vnode.props!.value)
  } else if (value !== oldValue) {
  el.checked = looseEqual(value, getCheckboxValue(el, true))
  }
@@ -213,7 +210,7 @@ function setSelected(el: HTMLSelectElement, value: any) {
  if (isArray(value)) {
  option.selected = looseIndexOf(value, optionValue) > -1
  } else {
- option.selected = looseHas(value, optionValue)
+ option.selected = value.has(optionValue)
  }
  } else {
  if (looseEqual(getValue(option), value)) {
@@ -305,7 +302,7 @@ if (__NODE_JS__) {
  return { checked: true }
  }
  } else if (isSet(value)) {
- if (vnode.props && looseHas(value, vnode.props.value)) {
+ if (vnode.props && value.has(vnode.props.value)) {
  return { checked: true }
  }
  } else if (value) {

diff --git a/packages/shared/src/looseEqual.ts b/packages/shared/src/looseEqual.ts
@@ -51,10 +51,3 @@ export function looseEqual(a: any, b: any): boolean {
 export function looseIndexOf(arr: any[], val: any): number {
  return arr.findIndex(item => looseEqual(item, val))
 }
-
-export function looseHas(set: Set<any>, val: any): boolean {
- for (let item of set) {
- if (looseEqual(item, val)) return true
- }
- return false
-}