Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: When patching the elements that use innerHTML or textContent should first unmountChildren #11159

Merged
merged 9 commits into from
Jul 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 1 addition & 10 deletions packages/runtime-core/src/hydration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -485,7 +477,6 @@ export function createHydrationFunctions(
null,
props.onClick,
undefined,
undefined,
parentComponent,
)
} else if (patchFlag & PatchFlags.STYLE && isReactive(props.style)) {
Expand Down
73 changes: 14 additions & 59 deletions packages/runtime-core/src/renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,7 @@ export interface RendererOptions<
prevValue: any,
nextValue: any,
namespace?: ElementNamespace,
prevChildren?: VNode<HostNode, HostElement>[],
parentComponent?: ComponentInternalInstance | null,
parentSuspense?: SuspenseBoundary | null,
unmountChildren?: UnmountChildrenFn,
): void
insert(el: HostNode, parent: HostElement, anchor?: HostNode | null): void
remove(el: HostNode): void
Expand Down Expand Up @@ -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)
}
}
/**
Expand Down Expand Up @@ -833,6 +820,15 @@ 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)
) {
hostSetElementText(el, '')
}

if (dynamicChildren) {
patchBlockChildren(
n1.dynamicChildren!,
Expand Down Expand Up @@ -869,15 +865,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.
Expand Down Expand Up @@ -908,17 +896,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)
}
}
}
Expand All @@ -933,15 +911,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) {
Expand Down Expand Up @@ -998,11 +968,9 @@ function baseCreateRenderer(

const patchProps = (
el: RendererElement,
vnode: VNode,
oldProps: Data,
newProps: Data,
parentComponent: ComponentInternalInstance | null,
parentSuspense: SuspenseBoundary | null,
namespace: ElementNamespace,
) => {
if (oldProps !== newProps) {
Expand All @@ -1015,10 +983,7 @@ function baseCreateRenderer(
oldProps[key],
null,
namespace,
vnode.children as VNode[],
parentComponent,
parentSuspense,
unmountChildren,
)
}
}
Expand All @@ -1030,17 +995,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) {
Expand Down
21 changes: 20 additions & 1 deletion packages/runtime-dom/__tests__/patchProps.spec.ts
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand Down Expand Up @@ -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', [h('del', null, 'baz')])
} else {
return h('div', { innerHTML: 'baz' })
}
},
}
render(h(Comp), root)
expect(root.innerHTML).toBe(`<div>baz</div>`)
state.value = true
await nextTick()
expect(root.innerHTML).toBe(`<div><del>baz</del></div>`)
})

test('textContent unmount prev children', () => {
const fn = vi.fn()
const comp = {
Expand Down
14 changes: 4 additions & 10 deletions packages/runtime-dom/src/modules/props.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,13 @@ 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,
parentComponent: any,
parentSuspense: any,
unmountChildren: any,
) {
if (key === 'innerHTML' || key === 'textContent') {
if (prevChildren) {
unmountChildren(prevChildren, parentComponent, parentSuspense)
}
el[key] = value == null ? '' : value
// null value case is handled in renderer patchElement before patching
// children
if (value === null) return
el[key] = value
return
}

Expand Down
13 changes: 1 addition & 12 deletions packages/runtime-dom/src/patchProp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,7 @@ export const patchProp: DOMRendererOptions['patchProp'] = (
prevValue,
nextValue,
namespace,
prevChildren,
parentComponent,
parentSuspense,
unmountChildren,
) => {
const isSVG = namespace === 'svg'
if (key === 'class') {
Expand All @@ -43,15 +40,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, nextValue, parentComponent)
// #6007 also set form state as attributes so they work with
// <input type="reset"> or libs / extensions that expect attributes
// #11163 custom elements may use value as an prop and set it as object
Expand Down