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: clean up VFragments when resetting component root #3360

Merged
merged 3 commits into from
Mar 1, 2023
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
32 changes: 22 additions & 10 deletions packages/@lwc/engine-core/src/framework/vm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import { ReactiveObserver } from './mutation-tracker';
import { connectWireAdapters, disconnectWireAdapters, installWireAdapters } from './wiring';
import { AccessorReactiveObserver } from './accessor-reactive-observer';
import { removeActiveVM } from './hot-swaps';
import { VNodes, VCustomElement, VNode, VNodeType, VBaseElement } from './vnodes';
import { VNodes, VCustomElement, VNode, VNodeType, VBaseElement, isVFragment } from './vnodes';

type ShadowRootMode = 'open' | 'closed';

Expand Down Expand Up @@ -672,23 +672,35 @@ function recursivelyDisconnectChildren(vnodes: VNodes) {
// into snabbdom. Especially useful when the reset is a consequence of an error, in which case the
// children VNodes might not be representing the current state of the DOM.
export function resetComponentRoot(vm: VM) {
recursivelyRemoveChildren(vm.children, vm);
vm.children = EmptyArray;

runChildNodesDisconnectedCallback(vm);
vm.velements = EmptyArray;
}

// Helper function to remove all children of the root node.
// If the set of children includes VFragment nodes, we need to remove the children of those nodes too.
// Since VFragments can contain other VFragments, we need to traverse the entire of tree of VFragments.
// If the set contains no VFragment nodes, no traversal is needed.
function recursivelyRemoveChildren(vnodes: VNodes, vm: VM) {
const {
children,
renderRoot,
renderer: { remove },
} = vm;

for (let i = 0, len = children.length; i < len; i++) {
const child = children[i];
for (let i = 0, len = vnodes.length; i < len; i += 1) {
const vnode = vnodes[i];

if (!isNull(child) && !isUndefined(child.elm)) {
remove(child.elm, renderRoot);
if (!isNull(vnode)) {
// VFragments are special; their .elm property does not point to the root element since they have no single root.
if (isVFragment(vnode)) {
recursivelyRemoveChildren(vnode.children, vm);
} else if (!isUndefined(vnode.elm)) {
remove(vnode.elm, renderRoot);
}
}
}
vm.children = EmptyArray;

runChildNodesDisconnectedCallback(vm);
vm.velements = EmptyArray;
}

export function scheduleRehydration(vm: VM) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import ShadowLightParent from 'x/shadowLightParent';
import LightParent from 'x/lightParent';
import LightShadowParent from 'x/lightShadowParent';
import ToggleContainer from 'x/toggleContainer';
import MultiTemplateConditionals from 'x/multiTemplateConditionals';

function resetTimingBuffer() {
window.timingBuffer = [];
Expand Down Expand Up @@ -228,3 +229,71 @@ it('should invoke callbacks on the right order (issue #1199 and #1198)', () => {
);
});
});

it('should invoke callbacks on the right order when multiple templates are used with lwc:if', () => {
const elm = createElement('x-multi-template-conditionals', { is: MultiTemplateConditionals });
elm.show = true;
document.body.appendChild(elm);

// initial load is x-shadow-parent
expect(window.timingBuffer).toEqual([
'leaf:T1-1:connectedCallback',
'leaf:T1-2:connectedCallback',
'leaf:T1-3:connectedCallback',
'leaf:T1-4:connectedCallback',
'leaf:T1-5:connectedCallback',
'leaf:T1-6:connectedCallback',
]);

resetTimingBuffer();
elm.next();

return Promise.resolve()
.then(() => {
// disconnect x-shadow-parent +
// connect x-shadow-container with 2 parents, 'a' and 'b'
expect(window.timingBuffer).toEqual(
window.lwcRuntimeFlags.ENABLE_NATIVE_CUSTOM_ELEMENT_LIFECYCLE
? [
'leaf:T1-1:disconnectedCallback',
'leaf:T1-2:disconnectedCallback',
'leaf:T1-3:disconnectedCallback',
'leaf:T1-4:disconnectedCallback',
'leaf:T1-5:disconnectedCallback',
'leaf:T1-6:disconnectedCallback',
'leaf:T2-1:connectedCallback',
'leaf:T2-2:connectedCallback',
'leaf:T2-3:connectedCallback',
'leaf:T2-4:connectedCallback',
'leaf:T2-5:connectedCallback',
'leaf:T2-6:connectedCallback',
]
: [
'leaf:T1-6:disconnectedCallback',
'leaf:T1-5:disconnectedCallback',
'leaf:T1-4:disconnectedCallback',
'leaf:T1-3:disconnectedCallback',
'leaf:T1-2:disconnectedCallback',
'leaf:T1-1:disconnectedCallback',
'leaf:T2-1:connectedCallback',
'leaf:T2-2:connectedCallback',
'leaf:T2-3:connectedCallback',
'leaf:T2-4:connectedCallback',
'leaf:T2-5:connectedCallback',
'leaf:T2-6:connectedCallback',
]
);
resetTimingBuffer();
elm.show = false;
})
.then(() => {
expect(window.timingBuffer).toEqual([
'leaf:T2-1:disconnectedCallback',
'leaf:T2-2:disconnectedCallback',
'leaf:T2-3:disconnectedCallback',
'leaf:T2-4:disconnectedCallback',
'leaf:T2-5:disconnectedCallback',
'leaf:T2-6:disconnectedCallback',
]);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { LightningElement, api } from 'lwc';
import template from './template.html';
import template2 from './template2.html';

export default class MultiTemplateConditionals extends LightningElement {
templateIndex = 0;
templateMapping = {
0: template,
1: template2,
};

@api
show = false;

render() {
return this.templateMapping[this.templateIndex];
}

@api
next() {
if (this.templateIndex < 1) {
this.templateIndex++;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<template>
<template lwc:if={show}>
<x-leaf name="T1-1"></x-leaf>
<template lwc:if={show}>
<x-leaf name="T1-2"></x-leaf>
<template lwc:if={show}>
<x-leaf name="T1-3"></x-leaf>
<x-leaf name="T1-4"></x-leaf>
</template>
<x-leaf name="T1-5"></x-leaf>
</template>
<x-leaf name="T1-6"></x-leaf>
</template>
</template>
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<template>
<template lwc:if={show}>
<x-leaf name="T2-1"></x-leaf>
<template lwc:if={show}>
<x-leaf name="T2-2"></x-leaf>
<template lwc:if={show}>
<x-leaf name="T2-3"></x-leaf>
<x-leaf name="T2-4"></x-leaf>
</template>
<x-leaf name="T2-5"></x-leaf>
</template>
<x-leaf name="T2-6"></x-leaf>
</template>
</template>
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { createElement } from 'lwc';
import CustomRender from 'x/customRender';

describe('using custom renderer with lwc:if', () => {
it('should replace the entire template when switching templates in a custom render function', async function () {
const elm = createElement('x-custom-render', { is: CustomRender });
document.body.appendChild(elm);
elm.next();

return Promise.resolve().then(() => {
expect(elm.shadowRoot.textContent).toBe('TEMPLATE 2T2 nested 1T2 nested 2');
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { LightningElement, api } from 'lwc';
import template from './template.html';
import template2 from './template2.html';

export default class CustomRender extends LightningElement {
templateIndex = 0;
templateMapping = {
0: template,
1: template2,
};
showHeader = true;

render() {
return this.templateMapping[this.templateIndex];
}

@api
next() {
if (this.templateIndex < 1) {
this.templateIndex++;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<template>
<h1 lwc:if={showHeader}>TEMPLATE 1</h1>
<template lwc:if={showHeader}>
<template lwc:if={showHeader}>
<div>T1 nested 1</div>
<template lwc:if={showHeader}>
<div>T1 nested 2</div>
</template>
</template>
</template>
</template>
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<template>
<h1 lwc:if={showHeader}>TEMPLATE 2</h1>
<template lwc:if={showHeader}>
<template lwc:if={showHeader}>
<div>T2 nested 1</div>
<template lwc:if={showHeader}>
<div>T2 nested 2</div>
</template>
</template>
</template>
</template>