From: Evan You Date: Fri, 7 Feb 2025 09:04:05 +0000 (+0800) Subject: wip(vapor): fix component unmount when not at block root level X-Git-Tag: v3.6.0-alpha.1~16^2~88 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bcd2eb7fd8d5892c26c2ab40e0a1bd99aac0384e;p=thirdparty%2Fvuejs%2Fcore.git wip(vapor): fix component unmount when not at block root level --- diff --git a/packages/runtime-vapor/__tests__/apiLifecycle.spec.ts b/packages/runtime-vapor/__tests__/apiLifecycle.spec.ts index 7c941d254a..d3bbeb8e7a 100644 --- a/packages/runtime-vapor/__tests__/apiLifecycle.spec.ts +++ b/packages/runtime-vapor/__tests__/apiLifecycle.spec.ts @@ -21,8 +21,10 @@ import { } from '@vue/runtime-dom' import { createComponent, + createFor, createIf, createTextNode, + insert, renderEffect, setText, template, @@ -526,4 +528,90 @@ describe('api: lifecycle hooks', () => { expect(handleUpdated).toHaveBeenCalledTimes(1) expect(handleUpdatedChild).toHaveBeenCalledTimes(1) }) + + test('unmount hooks when nested in if block', async () => { + const toggle = ref(true) + const fn = vi.fn(() => { + expect(host.innerHTML).toBe('
') + }) + const fn2 = vi.fn(() => { + expect(host.innerHTML).toBe('') + }) + const { render, host } = define({ + setup() { + const n0 = createIf( + () => toggle.value, + () => { + const n1 = document.createElement('div') + const n2 = createComponent(Child) + insert(n2, n1) + return n1 + }, + ) + return n0 + }, + }) + + const Child = { + setup() { + onBeforeUnmount(fn) + onUnmounted(fn2) + + const t0 = template('') + const n0 = t0() + return n0 + }, + } + + render() + + toggle.value = false + await nextTick() + expect(fn).toHaveBeenCalledTimes(1) + expect(fn2).toHaveBeenCalledTimes(1) + expect(host.innerHTML).toBe('') + }) + + test('unmount hooks when nested in for blocks', async () => { + const list = ref([1]) + const fn = vi.fn(() => { + expect(host.innerHTML).toBe('
') + }) + const fn2 = vi.fn(() => { + expect(host.innerHTML).toBe('') + }) + const { render, host } = define({ + setup() { + const n0 = createFor( + () => list.value, + () => { + const n1 = document.createElement('div') + const n2 = createComponent(Child) + insert(n2, n1) + return n1 + }, + ) + return n0 + }, + }) + + const Child = { + setup() { + onBeforeUnmount(fn) + onUnmounted(fn2) + + const t0 = template('') + const n0 = t0() + return n0 + }, + } + + render() + + list.value.pop() + await nextTick() + expect(fn).toHaveBeenCalledTimes(1) + expect(fn2).toHaveBeenCalledTimes(1) + expect(host.innerHTML).toBe('') + }) }) diff --git a/packages/runtime-vapor/src/apiCreateFor.ts b/packages/runtime-vapor/src/apiCreateFor.ts index 07b1e34bb3..1bce641635 100644 --- a/packages/runtime-vapor/src/apiCreateFor.ts +++ b/packages/runtime-vapor/src/apiCreateFor.ts @@ -75,7 +75,7 @@ export const createFor = ( let newBlocks: ForBlock[] let parent: ParentNode | undefined | null const parentAnchor = __DEV__ ? createComment('for') : createTextNode() - const ref = new VaporFragment(oldBlocks) + const frag = new VaporFragment(oldBlocks) const instance = currentInstance! if (__DEV__ && !instance) { @@ -265,9 +265,9 @@ export const createFor = ( } } - ref.nodes = [(oldBlocks = newBlocks)] + frag.nodes = [(oldBlocks = newBlocks)] if (parentAnchor) { - ref.nodes.push(parentAnchor) + frag.nodes.push(parentAnchor) } } @@ -338,12 +338,12 @@ export const createFor = ( } const unmount = ({ nodes, scope }: ForBlock) => { - removeBlock(nodes, parent!) scope && scope.stop() + removeBlock(nodes, parent!) } once ? renderList() : renderEffect(renderList) - return ref + return frag } export function createForSlots( diff --git a/packages/runtime-vapor/src/block.ts b/packages/runtime-vapor/src/block.ts index d363489054..b33e33a029 100644 --- a/packages/runtime-vapor/src/block.ts +++ b/packages/runtime-vapor/src/block.ts @@ -21,7 +21,7 @@ export class VaporFragment { nodes: Block anchor?: Node insert?: (parent: ParentNode, anchor: Node | null) => void - remove?: () => void + remove?: (parent?: ParentNode) => void constructor(nodes: Block) { this.nodes = nodes @@ -139,16 +139,12 @@ export function prepend(parent: ParentNode, ...blocks: Block[]): void { * during each root remove call, and update their children list by filtering * unmounted children */ -export let parentsWithUnmountedChildren: Set | null = - null +// export let parentsWithUnmountedChildren: Set | null = +// null -export function remove(block: Block, parent: ParentNode): void { - const isRoot = !parentsWithUnmountedChildren - if (isRoot) { - parentsWithUnmountedChildren = new Set() - } +export function remove(block: Block, parent?: ParentNode): void { if (block instanceof Node) { - parent.removeChild(block) + parent && parent.removeChild(block) } else if (isVaporComponent(block)) { unmountComponent(block, parent) } else if (isArray(block)) { @@ -158,7 +154,7 @@ export function remove(block: Block, parent: ParentNode): void { } else { // fragment if (block.remove) { - block.remove() + block.remove(parent) } else { remove(block.nodes, parent) } @@ -167,12 +163,6 @@ export function remove(block: Block, parent: ParentNode): void { ;(block as DynamicFragment).scope!.stop() } } - if (isRoot) { - for (const i of parentsWithUnmountedChildren!) { - i.children = i.children.filter(n => !n.isUnmounted) - } - parentsWithUnmountedChildren = null - } } /** diff --git a/packages/runtime-vapor/src/component.ts b/packages/runtime-vapor/src/component.ts index e46390f461..3c39612bb8 100644 --- a/packages/runtime-vapor/src/component.ts +++ b/packages/runtime-vapor/src/component.ts @@ -1,5 +1,4 @@ import { - type ComponentInternalInstance, type ComponentInternalOptions, type ComponentPropsOptions, EffectScope, @@ -26,29 +25,17 @@ import { unregisterHMR, warn, } from '@vue/runtime-dom' -import { - type Block, - insert, - isBlock, - parentsWithUnmountedChildren, - remove, -} from './block' +import { type Block, insert, isBlock, remove } from './block' import { type ShallowRef, markRaw, + onScopeDispose, pauseTracking, proxyRefs, resetTracking, unref, } from '@vue/reactivity' -import { - EMPTY_ARR, - EMPTY_OBJ, - invokeArrayFns, - isFunction, - isString, - remove as removeItem, -} from '@vue/shared' +import { EMPTY_OBJ, invokeArrayFns, isFunction, isString } from '@vue/shared' import { type DynamicPropsSource, type RawProps, @@ -264,6 +251,8 @@ export function createComponent( endMeasure(instance, 'init') } + onScopeDispose(() => unmountComponent(instance), true) + return instance } @@ -300,8 +289,6 @@ export class VaporComponentInstance implements GenericComponentInstance { type: VaporComponent root: GenericComponentInstance | null parent: GenericComponentInstance | null - children: VaporComponentInstance[] - vdomChildren?: ComponentInternalInstance[] appContext: GenericAppContext block: Block @@ -379,12 +366,8 @@ export class VaporComponentInstance implements GenericComponentInstance { this.type = comp this.parent = currentInstance this.root = currentInstance ? currentInstance.root : this - this.children = [] if (currentInstance) { - if (isVaporComponent(currentInstance)) { - currentInstance.children.push(this) - } this.appContext = currentInstance.appContext this.provides = currentInstance.provides this.ids = currentInstance.ids @@ -523,40 +506,13 @@ export function unmountComponent( instance.scope.stop() - for (const c of instance.children) { - unmountComponent(c) - } - instance.children = EMPTY_ARR as any - - if (instance.vdomChildren) { - const unmount = instance.appContext.vapor!.vdomUnmount - for (const c of instance.vdomChildren) { - unmount(c, null) - } - instance.vdomChildren = EMPTY_ARR as any - } - - if (parentNode) { - // root remove: need to both remove this instance's DOM nodes - // and also remove it from the parent's children list. - remove(instance.block, parentNode) - const parentInstance = instance.parent - instance.parent = null - if (isVaporComponent(parentInstance)) { - if (parentsWithUnmountedChildren) { - // for optimize children removal - parentsWithUnmountedChildren.add(parentInstance) - } else { - removeItem(parentInstance.children, instance) - } - } - } - if (instance.um) { queuePostFlushCb(() => invokeArrayFns(instance.um!)) } instance.isUnmounted = true - } else if (parentNode) { + } + + if (parentNode) { remove(instance.block, parentNode) } } diff --git a/packages/runtime-vapor/src/vdomInterop.ts b/packages/runtime-vapor/src/vdomInterop.ts index d258066d63..76fecff67d 100644 --- a/packages/runtime-vapor/src/vdomInterop.ts +++ b/packages/runtime-vapor/src/vdomInterop.ts @@ -24,7 +24,7 @@ import { unmountComponent, } from './component' import { type Block, VaporFragment, insert, remove } from './block' -import { extend, isFunction, remove as removeItem } from '@vue/shared' +import { extend, isFunction } from '@vue/shared' import { type RawProps, rawPropsProxyHandlers } from './componentProps' import type { RawSlots, VaporSlot } from './componentSlots' import { renderEffect } from './renderEffect' @@ -116,17 +116,14 @@ function createVDOMComponent( undefined, false, ) - ;(parentInstance.vdomChildren || (parentInstance.vdomChildren = [])).push( - vnode.component!, - ) + // TODO register unmount with onScopeDispose isMounted = true } else { // TODO move } } - frag.remove = () => { - internals.umt(vnode.component!, null, true) - removeItem(parentInstance.vdomChildren!, vnode.component) + frag.remove = parentNode => { + internals.umt(vnode.component!, null, !!parentNode) } return frag @@ -144,11 +141,9 @@ function renderVDOMSlot( let isMounted = false let fallbackNodes: Block | undefined - let parentNode: ParentNode let oldVNode: VNode | null = null - frag.insert = (parent, anchor) => { - parentNode = parent + frag.insert = (parentNode, anchor) => { if (!isMounted) { renderEffect(() => { const vnode = renderSlot( @@ -169,7 +164,7 @@ function renderVDOMSlot( if (oldVNode) { internals.um(oldVNode, parentComponent as any, null, true) } - insert((fallbackNodes = fallback(props)), parent, anchor) + insert((fallbackNodes = fallback(props)), parentNode, anchor) } oldVNode = null } @@ -179,7 +174,7 @@ function renderVDOMSlot( // TODO move } - frag.remove = () => { + frag.remove = parentNode => { if (fallbackNodes) { remove(fallbackNodes, parentNode) } else if (oldVNode) {