From 79dc06fdf870a54c30d53677074fb6dbf79d34d8 Mon Sep 17 00:00:00 2001 From: linzhe Date: Mon, 17 Jun 2024 14:59:06 +0800 Subject: [PATCH 1/9] fix: When patching, elements that use innerHTML and textContent should first `unmountChildren` --- packages/runtime-core/src/renderer.ts | 12 +++++++ packages/runtime-dom/src/modules/props.ts | 39 ++++++++++------------- packages/runtime-dom/src/patchProp.ts | 10 +----- 3 files changed, 29 insertions(+), 32 deletions(-) diff --git a/packages/runtime-core/src/renderer.ts b/packages/runtime-core/src/renderer.ts index 088d1565cbb..08175ecd7ed 100644 --- a/packages/runtime-core/src/renderer.ts +++ b/packages/runtime-core/src/renderer.ts @@ -832,6 +832,18 @@ function baseCreateRenderer( optimized = false dynamicChildren = null } + // the following args are passed only due to potential innerHTML/textContent + // overriding existing VNodes, in which case the old tree must be properly + // unmounted. + if ( + oldProps.innerHTML !== undefined || + oldProps.textContent !== undefined + ) { + hostSetElementText(n1.el!, '') + if (n1.children) { + unmountChildren(n1.children as VNode[], parentComponent, parentSuspense) + } + } if (dynamicChildren) { patchBlockChildren( diff --git a/packages/runtime-dom/src/modules/props.ts b/packages/runtime-dom/src/modules/props.ts index 2eb83ea39f7..5255c408479 100644 --- a/packages/runtime-dom/src/modules/props.ts +++ b/packages/runtime-dom/src/modules/props.ts @@ -9,20 +9,13 @@ import { includeBooleanAttr } from '@vue/shared' export function patchDOMProp( el: any, key: string, - value: any, - // the following args are passed only due to potential innerHTML/textContent - // overriding existing VNodes, in which case the old tree must be properly - // unmounted. - prevChildren: any, + prevValue: any, + nextValue: any, parentComponent: any, - parentSuspense: any, - unmountChildren: any, ) { if (key === 'innerHTML' || key === 'textContent') { - if (prevChildren) { - unmountChildren(prevChildren, parentComponent, parentSuspense) - } - el[key] = value == null ? '' : value + if (prevValue && nextValue == null) return + el[key] = nextValue == null ? '' : nextValue return } @@ -38,38 +31,38 @@ export function patchDOMProp( // compare against its attribute value instead. const oldValue = tag === 'OPTION' ? el.getAttribute('value') || '' : el.value - const newValue = value == null ? '' : String(value) + const newValue = nextValue == null ? '' : String(nextValue) if (oldValue !== newValue || !('_value' in el)) { el.value = newValue } - if (value == null) { + if (nextValue == null) { el.removeAttribute(key) } // store value as _value as well since // non-string values will be stringified. - el._value = value + el._value = nextValue return } let needRemove = false - if (value === '' || value == null) { + if (nextValue === '' || nextValue == null) { const type = typeof el[key] if (type === 'boolean') { // e.g. try { - el[key] = value + el[key] = nextValue } catch (e: any) { // do not warn if value is auto-coerced from nullish values if (__DEV__ && !needRemove) { warn( `Failed setting prop "${key}" on <${tag.toLowerCase()}>: ` + - `value ${value} is invalid.`, + `value ${nextValue} is invalid.`, e, ) } diff --git a/packages/runtime-dom/src/patchProp.ts b/packages/runtime-dom/src/patchProp.ts index e7b733c74af..b6afd57c025 100644 --- a/packages/runtime-dom/src/patchProp.ts +++ b/packages/runtime-dom/src/patchProp.ts @@ -43,15 +43,7 @@ export const patchProp: DOMRendererOptions['patchProp'] = ( ? ((key = key.slice(1)), false) : shouldSetAsProp(el, key, nextValue, isSVG) ) { - patchDOMProp( - el, - key, - nextValue, - prevChildren, - parentComponent, - parentSuspense, - unmountChildren, - ) + patchDOMProp(el, key, prevValue, nextValue, parentComponent) // #6007 also set form state as attributes so they work with // or libs / extensions that expect attributes // #11163 custom elements may use value as an prop and set it as object From 885ac02a162f05ffd4966ef099448cf1cd9bc901 Mon Sep 17 00:00:00 2001 From: linzhe Date: Mon, 17 Jun 2024 15:06:36 +0800 Subject: [PATCH 2/9] fix: update patchDOMProp --- packages/runtime-dom/src/modules/props.ts | 35 ++++++++++++----------- packages/runtime-dom/src/patchProp.ts | 2 +- 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/packages/runtime-dom/src/modules/props.ts b/packages/runtime-dom/src/modules/props.ts index 5255c408479..dbacdd2d5c6 100644 --- a/packages/runtime-dom/src/modules/props.ts +++ b/packages/runtime-dom/src/modules/props.ts @@ -9,14 +9,15 @@ import { includeBooleanAttr } from '@vue/shared' export function patchDOMProp( el: any, key: string, - prevValue: any, - nextValue: any, + value: any, parentComponent: any, ) { if (key === 'innerHTML' || key === 'textContent') { - if (prevValue && nextValue == null) return - el[key] = nextValue == null ? '' : nextValue - return + if (value === null) return + else { + el[key] = value + return + } } const tag = el.tagName @@ -31,38 +32,38 @@ export function patchDOMProp( // compare against its attribute value instead. const oldValue = tag === 'OPTION' ? el.getAttribute('value') || '' : el.value - const newValue = nextValue == null ? '' : String(nextValue) + const newValue = value == null ? '' : String(value) if (oldValue !== newValue || !('_value' in el)) { el.value = newValue } - if (nextValue == null) { + if (value == null) { el.removeAttribute(key) } // store value as _value as well since // non-string values will be stringified. - el._value = nextValue + el._value = value return } let needRemove = false - if (nextValue === '' || nextValue == null) { + if (value === '' || value == null) { const type = typeof el[key] if (type === 'boolean') { // e.g. try { - el[key] = nextValue + el[key] = value } catch (e: any) { // do not warn if value is auto-coerced from nullish values if (__DEV__ && !needRemove) { warn( `Failed setting prop "${key}" on <${tag.toLowerCase()}>: ` + - `value ${nextValue} is invalid.`, + `value ${value} is invalid.`, e, ) } diff --git a/packages/runtime-dom/src/patchProp.ts b/packages/runtime-dom/src/patchProp.ts index b6afd57c025..493f5791c23 100644 --- a/packages/runtime-dom/src/patchProp.ts +++ b/packages/runtime-dom/src/patchProp.ts @@ -43,7 +43,7 @@ export const patchProp: DOMRendererOptions['patchProp'] = ( ? ((key = key.slice(1)), false) : shouldSetAsProp(el, key, nextValue, isSVG) ) { - patchDOMProp(el, key, prevValue, nextValue, parentComponent) + patchDOMProp(el, key, nextValue, parentComponent) // #6007 also set form state as attributes so they work with // or libs / extensions that expect attributes // #11163 custom elements may use value as an prop and set it as object From 29775dcb9e483d3372a6f37aa7f50392af4c2f79 Mon Sep 17 00:00:00 2001 From: linzhe Date: Mon, 17 Jun 2024 15:24:57 +0800 Subject: [PATCH 3/9] test: update test --- .../runtime-dom/__tests__/patchProps.spec.ts | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/packages/runtime-dom/__tests__/patchProps.spec.ts b/packages/runtime-dom/__tests__/patchProps.spec.ts index 61dd98513ce..4082ebec5c1 100644 --- a/packages/runtime-dom/__tests__/patchProps.spec.ts +++ b/packages/runtime-dom/__tests__/patchProps.spec.ts @@ -1,5 +1,5 @@ import { patchProp } from '../src/patchProp' -import { h, render } from '../src' +import { h, nextTick, ref, render } from '../src' describe('runtime-dom: props patching', () => { test('basic', () => { @@ -133,6 +133,25 @@ describe('runtime-dom: props patching', () => { expect(fn).toHaveBeenCalled() }) + test('patch innerHTML porp', async () => { + const root = document.createElement('div') + const state = ref(false) + const Comp = { + render: () => { + if (state.value) { + return h('div', { key: 1 }, [h('del', null, 'baz')]) + } else { + return h('div', { key: 1, innerHTML: 'baz' }) + } + }, + } + render(h(Comp), root) + expect(root.innerHTML).toBe(`
baz
`) + state.value = true + await nextTick() + expect(root.innerHTML).toBe(`
baz
`) + }) + test('textContent unmount prev children', () => { const fn = vi.fn() const comp = { From 0ef0c14840cd8f6c403d888e89a438405db10c47 Mon Sep 17 00:00:00 2001 From: linzhe Date: Tue, 18 Jun 2024 14:56:49 +0800 Subject: [PATCH 4/9] fix: update --- packages/runtime-dom/src/modules/props.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/runtime-dom/src/modules/props.ts b/packages/runtime-dom/src/modules/props.ts index dbacdd2d5c6..36e321657c0 100644 --- a/packages/runtime-dom/src/modules/props.ts +++ b/packages/runtime-dom/src/modules/props.ts @@ -14,10 +14,8 @@ export function patchDOMProp( ) { if (key === 'innerHTML' || key === 'textContent') { if (value === null) return - else { - el[key] = value - return - } + el[key] = value + return } const tag = el.tagName From 00fd54a70f88dced17ed9846ded3a5bee589b8f6 Mon Sep 17 00:00:00 2001 From: linzhe Date: Tue, 18 Jun 2024 16:34:08 +0800 Subject: [PATCH 5/9] chore: update test --- packages/runtime-dom/__tests__/patchProps.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/runtime-dom/__tests__/patchProps.spec.ts b/packages/runtime-dom/__tests__/patchProps.spec.ts index 4082ebec5c1..7f418847f5f 100644 --- a/packages/runtime-dom/__tests__/patchProps.spec.ts +++ b/packages/runtime-dom/__tests__/patchProps.spec.ts @@ -139,9 +139,9 @@ describe('runtime-dom: props patching', () => { const Comp = { render: () => { if (state.value) { - return h('div', { key: 1 }, [h('del', null, 'baz')]) + return h('div', [h('del', null, 'baz')]) } else { - return h('div', { key: 1, innerHTML: 'baz' }) + return h('div', { innerHTML: 'baz' }) } }, } From d684be644893f0d633f59d365b0a71e76a5365bc Mon Sep 17 00:00:00 2001 From: linzhe Date: Wed, 19 Jun 2024 10:53:36 +0800 Subject: [PATCH 6/9] chore: update comment --- packages/runtime-core/src/renderer.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/runtime-core/src/renderer.ts b/packages/runtime-core/src/renderer.ts index 08175ecd7ed..754c81d118c 100644 --- a/packages/runtime-core/src/renderer.ts +++ b/packages/runtime-core/src/renderer.ts @@ -832,9 +832,8 @@ function baseCreateRenderer( optimized = false dynamicChildren = null } - // the following args are passed only due to potential innerHTML/textContent - // overriding existing VNodes, in which case the old tree must be properly - // unmounted. + // due to potential innerHTML/textContent overriding existing VNodes, + // in which case the old tree must be properly unmounted. if ( oldProps.innerHTML !== undefined || oldProps.textContent !== undefined From 707e04f8b15c8061f4efd0f512b046a464b0dff7 Mon Sep 17 00:00:00 2001 From: linzhe Date: Fri, 21 Jun 2024 11:06:05 +0800 Subject: [PATCH 7/9] chore: update --- packages/runtime-core/src/hydration.ts | 11 +------ packages/runtime-core/src/renderer.ts | 42 ++------------------------ packages/runtime-dom/src/patchProp.ts | 3 -- 3 files changed, 4 insertions(+), 52 deletions(-) diff --git a/packages/runtime-core/src/hydration.ts b/packages/runtime-core/src/hydration.ts index fe267f4fef6..c371542607d 100644 --- a/packages/runtime-core/src/hydration.ts +++ b/packages/runtime-core/src/hydration.ts @@ -465,15 +465,7 @@ export function createHydrationFunctions( // force hydrate v-bind with .prop modifiers key[0] === '.' ) { - patchProp( - el, - key, - null, - props[key], - undefined, - undefined, - parentComponent, - ) + patchProp(el, key, null, props[key], undefined, parentComponent) } } } else if (props.onClick) { @@ -485,7 +477,6 @@ export function createHydrationFunctions( null, props.onClick, undefined, - undefined, parentComponent, ) } else if (patchFlag & PatchFlags.STYLE && isReactive(props.style)) { diff --git a/packages/runtime-core/src/renderer.ts b/packages/runtime-core/src/renderer.ts index 754c81d118c..38c837ea8da 100644 --- a/packages/runtime-core/src/renderer.ts +++ b/packages/runtime-core/src/renderer.ts @@ -107,10 +107,7 @@ export interface RendererOptions< prevValue: any, nextValue: any, namespace?: ElementNamespace, - prevChildren?: VNode[], parentComponent?: ComponentInternalInstance | null, - parentSuspense?: SuspenseBoundary | null, - unmountChildren?: UnmountChildrenFn, ): void insert(el: HostNode, parent: HostElement, anchor?: HostNode | null): void remove(el: HostNode): void @@ -670,17 +667,7 @@ function baseCreateRenderer( if (props) { for (const key in props) { if (key !== 'value' && !isReservedProp(key)) { - hostPatchProp( - el, - key, - null, - props[key], - namespace, - vnode.children as VNode[], - parentComponent, - parentSuspense, - unmountChildren, - ) + hostPatchProp(el, key, null, props[key], namespace, parentComponent) } } /** @@ -919,17 +906,7 @@ function baseCreateRenderer( const next = newProps[key] // #1471 force patch value if (next !== prev || key === 'value') { - hostPatchProp( - el, - key, - prev, - next, - namespace, - n1.children as VNode[], - parentComponent, - parentSuspense, - unmountChildren, - ) + hostPatchProp(el, key, prev, next, namespace, parentComponent) } } } @@ -1026,10 +1003,7 @@ function baseCreateRenderer( oldProps[key], null, namespace, - vnode.children as VNode[], parentComponent, - parentSuspense, - unmountChildren, ) } } @@ -1041,17 +1015,7 @@ function baseCreateRenderer( const prev = oldProps[key] // defer patching value if (next !== prev && key !== 'value') { - hostPatchProp( - el, - key, - prev, - next, - namespace, - vnode.children as VNode[], - parentComponent, - parentSuspense, - unmountChildren, - ) + hostPatchProp(el, key, prev, next, namespace, parentComponent) } } if ('value' in newProps) { diff --git a/packages/runtime-dom/src/patchProp.ts b/packages/runtime-dom/src/patchProp.ts index 493f5791c23..f3ef14ee83c 100644 --- a/packages/runtime-dom/src/patchProp.ts +++ b/packages/runtime-dom/src/patchProp.ts @@ -21,10 +21,7 @@ export const patchProp: DOMRendererOptions['patchProp'] = ( prevValue, nextValue, namespace, - prevChildren, parentComponent, - parentSuspense, - unmountChildren, ) => { const isSVG = namespace === 'svg' if (key === 'class') { From 6c95b0ba7729862c97cec6b2e43fb12d165f92c1 Mon Sep 17 00:00:00 2001 From: Evan You Date: Wed, 17 Jul 2024 16:04:22 +0800 Subject: [PATCH 8/9] chore: update --- packages/runtime-core/src/renderer.ts | 34 +++++---------------------- 1 file changed, 6 insertions(+), 28 deletions(-) diff --git a/packages/runtime-core/src/renderer.ts b/packages/runtime-core/src/renderer.ts index 38c837ea8da..e1b79508d13 100644 --- a/packages/runtime-core/src/renderer.ts +++ b/packages/runtime-core/src/renderer.ts @@ -819,16 +819,12 @@ function baseCreateRenderer( optimized = false dynamicChildren = null } - // due to potential innerHTML/textContent overriding existing VNodes, - // in which case the old tree must be properly unmounted. + if ( - oldProps.innerHTML !== undefined || - oldProps.textContent !== undefined + (oldProps.innerHTML && newProps.innerHTML == null) || + (oldProps.textContent && newProps.textContent == null) ) { - hostSetElementText(n1.el!, '') - if (n1.children) { - unmountChildren(n1.children as VNode[], parentComponent, parentSuspense) - } + hostSetElementText(el, '') } if (dynamicChildren) { @@ -867,15 +863,7 @@ function baseCreateRenderer( // (i.e. at the exact same position in the source template) if (patchFlag & PatchFlags.FULL_PROPS) { // element props contain dynamic keys, full diff needed - patchProps( - el, - n2, - oldProps, - newProps, - parentComponent, - parentSuspense, - namespace, - ) + patchProps(el, oldProps, newProps, parentComponent, namespace) } else { // class // this flag is matched when the element has dynamic class bindings. @@ -921,15 +909,7 @@ function baseCreateRenderer( } } else if (!optimized && dynamicChildren == null) { // unoptimized, full diff - patchProps( - el, - n2, - oldProps, - newProps, - parentComponent, - parentSuspense, - namespace, - ) + patchProps(el, oldProps, newProps, parentComponent, namespace) } if ((vnodeHook = newProps.onVnodeUpdated) || dirs) { @@ -986,11 +966,9 @@ function baseCreateRenderer( const patchProps = ( el: RendererElement, - vnode: VNode, oldProps: Data, newProps: Data, parentComponent: ComponentInternalInstance | null, - parentSuspense: SuspenseBoundary | null, namespace: ElementNamespace, ) => { if (oldProps !== newProps) { From 69e1d40c486daeb7edad64806375cdae7cdd8cf0 Mon Sep 17 00:00:00 2001 From: Evan You Date: Wed, 17 Jul 2024 16:26:27 +0800 Subject: [PATCH 9/9] chore: comments --- packages/runtime-core/src/renderer.ts | 2 ++ packages/runtime-dom/src/modules/props.ts | 2 ++ 2 files changed, 4 insertions(+) diff --git a/packages/runtime-core/src/renderer.ts b/packages/runtime-core/src/renderer.ts index e1b79508d13..db674f987d8 100644 --- a/packages/runtime-core/src/renderer.ts +++ b/packages/runtime-core/src/renderer.ts @@ -820,6 +820,8 @@ function baseCreateRenderer( dynamicChildren = null } + // #9135 innerHTML / textContent unset needs to happen before possible + // new children mount if ( (oldProps.innerHTML && newProps.innerHTML == null) || (oldProps.textContent && newProps.textContent == null) diff --git a/packages/runtime-dom/src/modules/props.ts b/packages/runtime-dom/src/modules/props.ts index 36e321657c0..04f0d0e866d 100644 --- a/packages/runtime-dom/src/modules/props.ts +++ b/packages/runtime-dom/src/modules/props.ts @@ -13,6 +13,8 @@ export function patchDOMProp( parentComponent: any, ) { if (key === 'innerHTML' || key === 'textContent') { + // null value case is handled in renderer patchElement before patching + // children if (value === null) return el[key] = value return