feat(runtime-vapor): reset old props when setting dynamic props (#131)

Co-authored-by: 三咲智子 Kevin Deng <[email protected]>

Author: ygj6

packages/runtime-vapor/__tests__/dom/patchProp.spec.ts

@@ -5,6 +5,7 @@ import {
   setAttr,
   setClass,
   setDOMProp,
+  setDynamicProps,
   setHtml,
   setStyle,
   setText,
@@ -414,6 +415,54 @@ describe('patchProp', () => {
     test.todo('should be able to set something on SVG')
   })
 
+  describe('setDynamicProps', () => {
+    test('basic set dynamic props', () => {
+      const el = document.createElement('div')
+      setDynamicProps(el, { foo: 'val' }, { bar: 'val' })
+      expect(el.getAttribute('foo')).toBe('val')
+      expect(el.getAttribute('bar')).toBe('val')
+    })
+
+    test('should merge props', () => {
+      const el = document.createElement('div')
+      setDynamicProps(el, { foo: 'val' }, { foo: 'newVal' })
+      expect(el.getAttribute('foo')).toBe('newVal')
+    })
+
+    test('should reset old props', () => {
+      const el = document.createElement('div')
+
+      setDynamicProps(el, { foo: 'val' })
+      expect(el.attributes.length).toBe(1)
+      expect(el.getAttribute('foo')).toBe('val')
+
+      setDynamicProps(el, { bar: 'val' })
+      expect(el.attributes.length).toBe(1)
+      expect(el.getAttribute('bar')).toBe('val')
+      expect(el.getAttribute('foo')).toBeNull()
+    })
+
+    test('should reset old modifier props', () => {
+      const el = document.createElement('div')
+
+      setDynamicProps(el, { ['.foo']: 'val' })
+      expect((el as any).foo).toBe('val')
+
+      setDynamicProps(el, { ['.bar']: 'val' })
+      expect((el as any).bar).toBe('val')
+      expect((el as any).foo).toBe('')
+
+      setDynamicProps(el, { ['^foo']: 'val' })
+      expect(el.attributes.length).toBe(1)
+      expect(el.getAttribute('foo')).toBe('val')
+
+      setDynamicProps(el, { ['^bar']: 'val' })
+      expect(el.attributes.length).toBe(1)
+      expect(el.getAttribute('bar')).toBe('val')
+      expect(el.getAttribute('foo')).toBeNull()
+    })
+  })
+
   describe('setText', () => {
     test('should set textContent', () => {
       const el = document.createElement('div')

packages/runtime-vapor/src/dom/prop.ts

@@ -9,20 +9,28 @@ import {
   normalizeStyle,
   toDisplayString,
 } from '@vue/shared'
-import { currentInstance } from '../component'
+import { type ElementMetadata, currentInstance } from '../component'
 import { warn } from '../warning'
 import { setStyle } from './style'
 
-export function recordPropMetadata(el: Node, key: string, value: any): any {
+function getMetadata(el: Node): ElementMetadata {
+  const EMPTY_METADATA = { props: {} }
+
   if (!currentInstance) {
     // TODO implement error handling
     if (__DEV__) throw new Error('cannot be used out of component')
-    return
+    return EMPTY_METADATA
   }
+
   let metadata = currentInstance.metadata.get(el)
   if (!metadata) {
-    currentInstance.metadata.set(el, (metadata = { props: {} }))
+    currentInstance.metadata.set(el, (metadata = EMPTY_METADATA))
   }
+  return metadata
+}
+
+export function recordPropMetadata(el: Node, key: string, value: any): any {
+  const metadata = getMetadata(el)
   const prev = metadata.props[key]
   metadata.props[key] = value
   return prev
@@ -140,9 +148,21 @@ export function setDynamicProp(el: Element, key: string, value: any) {
 }
 
 export function setDynamicProps(el: Element, ...args: any) {
+  const oldProps = getMetadata(el).props
   const props = args.length > 1 ? mergeProps(...args) : args[0]
 
-  // TODO remove all of old props before set new props since there is containing dynamic key
+  for (const key in oldProps) {
+    // TODO should these keys be allowed as dynamic keys? The current logic of the runtime-core will throw an error
+    if (key === 'textContent' || key === 'innerHTML') {
+      continue
+    }
+
+    const hasNewValue = props[key] || props['.' + key] || props['^' + key]
+    if (oldProps[key] && !hasNewValue) {
+      setDynamicProp(el, key, null)
+    }
+  }
+
   for (const key in props) {
     setDynamicProp(el, key, props[key])
   }

playground/src/v-bind.vue renamed to playground/src/prop.vue

@@ -11,7 +11,7 @@ const handleClick = () => {
 </script>
 
 <template>
-  <button @click="handleClick">{{ count }}</button>
+  <button @click="handleClick">click me to update props</button>
 
   <!-- prop id's value should update reactively  -->
   <button :id="'before'" :[key]="'dynamic key after' + count">
@@ -34,4 +34,7 @@ const handleClick = () => {
   <!-- prop id's value should update only once since the prop id in object props was override -->
   <button v-bind="obj" :id="'after'">{{ count }}</button>
   <button v-bind="obj" :[key]="'dynamic key after'">{{ count }}</button>
+
+  <!-- old props will be reset after dynamic key changed -->
+  <button :[`key${count}`]="'dynamic key'">{{ count }}</button>
 </template>