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
-}