Skip to content

Commit abbef02

Browse files
VarixoVarixo
committed
perf: use replace children for truncating nodes
1 parent d368215 commit abbef02

File tree

3 files changed

+147
-6
lines changed

3 files changed

+147
-6
lines changed

‎packages/qwik/src/core/client/vnode-diff.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ import {
3232
dangerouslySetInnerHTML,
3333
} from '../shared/utils/markers';
3434
import { isPromise } from '../shared/utils/promises';
35-
import { type ValueOrPromise } from '../shared/utils/types';
35+
import { isArray, type ValueOrPromise } from '../shared/utils/types';
3636
import {
3737
getEventNameFromJsxEvent,
3838
getEventNameScopeFromJsxEvent,
@@ -308,7 +308,7 @@ export const vnode_diff = (
308308
* is an array produced by the `map` function.
309309
*/
310310
function descend(children: JSXChildren, descendVNode: boolean) {
311-
if (children == null) {
311+
if (children == null || (descendVNode && isArray(children) && children.length === 0)) {
312312
expectNoChildren();
313313
return;
314314
}

‎packages/qwik/src/core/client/vnode.ts‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -183,8 +183,9 @@ export const enum VNodeJournalOpCode {
183183
SetText = 1, // ------ [SetAttribute, target, text]
184184
SetAttribute = 2, // - [SetAttribute, target, ...(key, values)]]
185185
HoistStyles = 3, // -- [HoistStyles, document]
186-
Remove = 4, // ------- [Insert, target(parent), ...nodes]
187-
Insert = 5, // ------- [Insert, target(parent), reference, ...nodes]
186+
Remove = 4, // ------- [Remove, target(parent), ...nodes]
187+
RemoveAll = 5, // ------- [RemoveAll, target(parent)]
188+
Insert = 6, // ------- [Insert, target(parent), reference, ...nodes]
188189
}
189190

190191
export type VNodeJournal = Array<
@@ -968,6 +969,15 @@ export const vnode_applyJournal = (journal: VNodeJournal) => {
968969
idx++;
969970
}
970971
break;
972+
case VNodeJournalOpCode.RemoveAll:
973+
const removeAllParent = journal[idx++] as Element;
974+
if (removeAllParent.replaceChildren) {
975+
removeAllParent.replaceChildren();
976+
} else {
977+
// fallback if replaceChildren is not supported
978+
removeAllParent.textContent = '';
979+
}
980+
break;
971981
case VNodeJournalOpCode.Insert:
972982
const insertParent = journal[idx++] as Element;
973983
const insertBefore = journal[idx++] as Element | Text | null;
@@ -1220,8 +1230,14 @@ export const vnode_truncate = (
12201230
) => {
12211231
assertDefined(vDelete, 'Missing vDelete.');
12221232
const parent = vnode_getDomParent(vParent);
1223-
const children = vnode_getDOMChildNodes(journal, vDelete);
1224-
parent && children.length && journal.push(VNodeJournalOpCode.Remove, parent, ...children);
1233+
if (parent) {
1234+
if (vnode_isElementVNode(vParent)) {
1235+
journal.push(VNodeJournalOpCode.RemoveAll, parent);
1236+
} else {
1237+
const children = vnode_getDOMChildNodes(journal, vParent);
1238+
children.length && journal.push(VNodeJournalOpCode.Remove, parent, ...children);
1239+
}
1240+
}
12251241
const vPrevious = vDelete.previousSibling;
12261242
if (vPrevious) {
12271243
vPrevious.nextSibling = null;

‎packages/qwik/src/core/tests/component.spec.tsx‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2410,6 +2410,131 @@ describe.each([
24102410
await expect(document.querySelector('div')).toMatchDOM(<div />);
24112411
});
24122412

2413+
it('should correctly remove all children for empty array', async () => {
2414+
const Cmp = component$(() => {
2415+
const list = useSignal([1, 2, 3]);
2416+
return (
2417+
<main>
2418+
<button onClick$={() => (list.value = [])}>Remove</button>
2419+
{list.value.map((item) => (
2420+
<div>{item}</div>
2421+
))}
2422+
</main>
2423+
);
2424+
});
2425+
const { vNode, document } = await render(<Cmp />, { debug });
2426+
expect(vNode).toMatchVDOM(
2427+
<Component>
2428+
<main>
2429+
<button>Remove</button>
2430+
<div>1</div>
2431+
<div>2</div>
2432+
<div>3</div>
2433+
</main>
2434+
</Component>
2435+
);
2436+
await trigger(document.body, 'button', 'click');
2437+
expect(vNode).toMatchVDOM(
2438+
<Component>
2439+
<main>
2440+
<button>Remove</button>
2441+
</main>
2442+
</Component>
2443+
);
2444+
expect(document.querySelector('main')).toMatchDOM(
2445+
<main>
2446+
<button>Remove</button>
2447+
</main>
2448+
);
2449+
});
2450+
2451+
it('should correctly remove all children for empty array - case 2', async () => {
2452+
const Cmp = component$(() => {
2453+
const list = useSignal([1, 2, 3]);
2454+
return (
2455+
<main>
2456+
<button onClick$={() => (list.value = [])}>Remove</button>
2457+
<div>
2458+
{list.value.map((item) => (
2459+
<div>{item}</div>
2460+
))}
2461+
</div>
2462+
</main>
2463+
);
2464+
});
2465+
const { vNode, document } = await render(<Cmp />, { debug });
2466+
expect(vNode).toMatchVDOM(
2467+
<Component>
2468+
<main>
2469+
<button>Remove</button>
2470+
<div>
2471+
<div>1</div>
2472+
<div>2</div>
2473+
<div>3</div>
2474+
</div>
2475+
</main>
2476+
</Component>
2477+
);
2478+
await trigger(document.body, 'button', 'click');
2479+
expect(vNode).toMatchVDOM(
2480+
<Component>
2481+
<main>
2482+
<button>Remove</button>
2483+
<div></div>
2484+
</main>
2485+
</Component>
2486+
);
2487+
expect(document.querySelector('main')).toMatchDOM(
2488+
<main>
2489+
<button>Remove</button>
2490+
<div></div>
2491+
</main>
2492+
);
2493+
});
2494+
2495+
it('should correctly remove all children for empty array within virtual node', async () => {
2496+
const Cmp = component$(() => {
2497+
const list = useSignal([1, 2, 3]);
2498+
return (
2499+
<main>
2500+
<button onClick$={() => (list.value = [])}>Remove</button>
2501+
<>
2502+
{list.value.map((item) => (
2503+
<div>{item}</div>
2504+
))}
2505+
</>
2506+
</main>
2507+
);
2508+
});
2509+
const { vNode, document } = await render(<Cmp />, { debug });
2510+
expect(vNode).toMatchVDOM(
2511+
<Component>
2512+
<main>
2513+
<button>Remove</button>
2514+
<Fragment ssr-required>
2515+
<div>1</div>
2516+
<div>2</div>
2517+
<div>3</div>
2518+
</Fragment>
2519+
</main>
2520+
</Component>
2521+
);
2522+
await trigger(document.body, 'button', 'click');
2523+
expect(vNode).toMatchVDOM(
2524+
<Component>
2525+
<main>
2526+
<button>Remove</button>
2527+
<Fragment ssr-required></Fragment>
2528+
</main>
2529+
</Component>
2530+
);
2531+
await expect(document.querySelector('main')).toMatchDOM(
2532+
<main>
2533+
<button>Remove</button>
2534+
</main>
2535+
);
2536+
});
2537+
24132538
describe('regression', () => {
24142539
it('#3643', async () => {
24152540
const Issue3643 = component$(() => {

0 commit comments

Comments
 (0)