From 86089b00312aa90c703b43f04c41deee9067a1aa Mon Sep 17 00:00:00 2001 From: "andrii.holubenko" Date: Fri, 28 Aug 2026 13:41:55 +0200 Subject: [PATCH 1/2] fix(core): keep the identity of a headless component's output around the useOn placeholder --- .changeset/lucky-moons-remount.md | 40 ++++ .../src/core/shared/component-execution.ts | 67 +++--- .../src/core/tests/use-visible-task.spec.tsx | 216 +++++++++++++++++- 3 files changed, 290 insertions(+), 33 deletions(-) create mode 100644 .changeset/lucky-moons-remount.md diff --git a/.changeset/lucky-moons-remount.md b/.changeset/lucky-moons-remount.md new file mode 100644 index 00000000000..851b41200fb --- /dev/null +++ b/.changeset/lucky-moons-remount.md @@ -0,0 +1,40 @@ +--- +'@qwik.dev/core': patch +--- + +fix: a component which renders no DOM element of its own and registers a document or window `useOn` +event - `useVisibleTask$` among them - no longer destroys everything it rendered on every re-render, +and no longer loses that event on some outputs. + +Such a component gets a placeholder ` - - {'run'} - ); }); @@ -904,6 +936,182 @@ describe.each([ }); describe('regression', () => { + it('should not re-create the root child component on re-render', async () => { + const Child = component$<{ text: string }>((props) => { + // no `track()`, so this runs once per instance and records what the instance was born with + const initial = useSignal(''); + useTask$(() => { + initial.value = props.text; + }); + return ( + + {props.text}|{initial.value} + + ); + }); + + /** + * The component renders no host element of its own, so `useVisibleTask$` makes it render a + * placeholder ` + + + + {'shared'} + + + + ); + }); + + it('should keep every instance of a headless component when there are many of them', async () => { + // every one of them renders the same keyed wrapper around its placeholder, and the key is + // only ever matched against siblings, so the instances must not be mixed up + const Child = component$<{ n: number; text: string }>((props) => { + const initial = useSignal(''); + useTask$(() => { + initial.value = props.text; + }); + return ( + + [{String(props.n)}:{props.text}|{initial.value}] + + ); + }); + + const Headless = component$<{ n: number }>((props) => { + const isLoaded = useSignal(false); + useVisibleTask$(() => { + isLoaded.value = true; + }); + return ; + }); + + const COUNT = 10; + const Cmp = component$(() => ( +
+ {Array.from({ length: COUNT }, (_, i) => ( + + ))} + {Array.from({ length: COUNT }, (_, i) => ( + + ))} +
+ )); + + const { document } = await render(, { debug }); + if (render === ssrRenderToDom) { + await trigger(document.body, 'script', 'd:qinit'); + } + await getTestPlatform().flush(); + + const texts = Array.from(document.querySelectorAll('span')).map((s) => s.textContent); + // every instance kept the value it was born with, and kept its own number + expect(texts).toHaveLength(COUNT * 2); + expect(new Set(texts).size).toBe(COUNT * 2); + expect(texts.filter((t) => !t!.endsWith('|vfalse]'))).toEqual([]); + expect(texts[0]).toBe('[0:vtrue|vfalse]'); + expect(texts[COUNT]).toBe('[100:vtrue|vfalse]'); + // one placeholder per instance, they do not share or steal each other's + expect(document.querySelectorAll('script[q-d\\:qinit]').length).toBe(COUNT * 2); + }); + it('should not double-register events on component re-render', async () => { const Cmp = component$(() => { const count = useSignal(0); From 8436357a1607f4cb251b1d28fb82c00ca8c5038a Mon Sep 17 00:00:00 2001 From: "andrii.holubenko" Date: Fri, 28 Aug 2026 20:32:14 +0200 Subject: [PATCH 2/2] fix(core): stop wrapping a headless component's output in a fragment --- .changeset/lucky-moons-remount.md | 37 +--- .../src/core/shared/component-execution.ts | 50 +---- .../qwik/src/core/shared/utils/markers.ts | 1 + packages/qwik/src/core/tests/use-on.spec.tsx | 56 +++--- .../src/core/tests/use-visible-task.spec.tsx | 187 ++++++++---------- 5 files changed, 115 insertions(+), 216 deletions(-) diff --git a/.changeset/lucky-moons-remount.md b/.changeset/lucky-moons-remount.md index 851b41200fb..167992954c1 100644 --- a/.changeset/lucky-moons-remount.md +++ b/.changeset/lucky-moons-remount.md @@ -2,39 +2,4 @@ '@qwik.dev/core': patch --- -fix: a component which renders no DOM element of its own and registers a document or window `useOn` -event - `useVisibleTask$` among them - no longer destroys everything it rendered on every re-render, -and no longer loses that event on some outputs. - -Such a component gets a placeholder ` + Count: {'123'}! + ); @@ -372,8 +373,9 @@ describe.each([ expect(vNode).toMatchVDOM( - Count: {'124'}! + Count: {'124'}! + ); }); @@ -412,28 +414,24 @@ describe.each([ const { vNode, container } = await render(, { debug }); expect(vNode).toMatchVDOM( - - - - Count: {'123'}! - - - + Count: {'123'}! + ); @@ -634,8 +633,9 @@ describe.each([ expect(vNode).toMatchVDOM( - Count: {'124'}! + Count: {'124'}! + ); }); @@ -1059,11 +1059,9 @@ describe.each([ - -
test
-
- +
test
+
); @@ -1089,12 +1087,10 @@ describe.each([ expect(vNode).toMatchVDOM( - - -
test
-
- -
+ +
test
+
+
); diff --git a/packages/qwik/src/core/tests/use-visible-task.spec.tsx b/packages/qwik/src/core/tests/use-visible-task.spec.tsx index a374636b72b..95816dccb1b 100644 --- a/packages/qwik/src/core/tests/use-visible-task.spec.tsx +++ b/packages/qwik/src/core/tests/use-visible-task.spec.tsx @@ -37,11 +37,6 @@ import type { JSXNodeInternal, JSXOutput } from '../shared/jsx/types/jsx-node'; const debug = false; //true; Error.stackTraceLimit = 100; -/** A function call is left eager by the optimizer, so reading a signal through it re-renders. */ -function stringify(value: unknown) { - return `v${value}`; -} - export function useDelay(value: string) { const ready = useSignal('---'); useVisibleTask$(() => { @@ -279,8 +274,8 @@ describe.each([ {'run'} - + ); }); @@ -320,18 +315,15 @@ describe.each([ if (render === ssrRenderToDom) { await trigger(document.body, 'script', 'd:qinit'); } - // the placeholder joins the list as a sibling inside the stable wrapper expect(vNode).toMatchVDOM( - - {'run'} - - - {'run'} - - + {'run'} + + + {'run'} + ); }); @@ -350,9 +342,8 @@ describe.each([ } expect(vNode).toMatchVDOM( - - - + + ); }); @@ -372,10 +363,8 @@ describe.each([ expect((globalThis as any).log).toEqual(['task']); expect(vNode).toMatchVDOM( - - {''} - - + {''} + ); }); @@ -395,10 +384,8 @@ describe.each([ expect((globalThis as any).log).toEqual(['task']); expect(vNode).toMatchVDOM( - - {''} - - + {''} + ); }); @@ -937,31 +924,24 @@ describe.each([ describe('regression', () => { it('should not re-create the root child component on re-render', async () => { + (globalThis as any).childRenderCounter = 0; + (globalThis as any).childInstanceCounter = 0; const Child = component$<{ text: string }>((props) => { - // no `track()`, so this runs once per instance and records what the instance was born with - const initial = useSignal(''); + (globalThis as any).childRenderCounter++; useTask$(() => { - initial.value = props.text; + // no `track()`, so this runs once per instance + (globalThis as any).childInstanceCounter++; }); - return ( - - {props.text}|{initial.value} - - ); + return {props.text}; }); - /** - * The component renders no host element of its own, so `useVisibleTask$` makes it render a - * placeholder ` - + {'shared'} + {'shared'} @@ -1060,56 +1035,56 @@ describe.each([ ); }); - it('should keep every instance of a headless component when there are many of them', async () => { - // every one of them renders the same keyed wrapper around its placeholder, and the key is - // only ever matched against siblings, so the instances must not be mixed up - const Child = component$<{ n: number; text: string }>((props) => { - const initial = useSignal(''); + it('should keep every headless component instance when the list is reordered', async () => { + (globalThis as any).childRenderCounter = 0; + (globalThis as any).childInstanceCounter = 0; + const Child = component$<{ label: string }>((props) => { + (globalThis as any).childRenderCounter++; useTask$(() => { - initial.value = props.text; + (globalThis as any).childInstanceCounter++; }); - return ( - - [{String(props.n)}:{props.text}|{initial.value}] - - ); + return {props.label}; }); - const Headless = component$<{ n: number }>((props) => { - const isLoaded = useSignal(false); - useVisibleTask$(() => { - isLoaded.value = true; - }); - return ; + const Headless = component$<{ name: string; index: number }>((props) => { + useVisibleTask$(() => {}); + return ; }); - const COUNT = 10; - const Cmp = component$(() => ( -
- {Array.from({ length: COUNT }, (_, i) => ( - - ))} - {Array.from({ length: COUNT }, (_, i) => ( - - ))} -
- )); + const Cmp = component$(() => { + const names = useSignal(['a', 'b', 'c']); + return ( +
+ + {names.value.map((name, index) => ( + + ))} +
+ ); + }); const { document } = await render(, { debug }); if (render === ssrRenderToDom) { await trigger(document.body, 'script', 'd:qinit'); } - await getTestPlatform().flush(); - - const texts = Array.from(document.querySelectorAll('span')).map((s) => s.textContent); - // every instance kept the value it was born with, and kept its own number - expect(texts).toHaveLength(COUNT * 2); - expect(new Set(texts).size).toBe(COUNT * 2); - expect(texts.filter((t) => !t!.endsWith('|vfalse]'))).toEqual([]); - expect(texts[0]).toBe('[0:vtrue|vfalse]'); - expect(texts[COUNT]).toBe('[100:vtrue|vfalse]'); + const spansBeforeReorder = Array.from(document.querySelectorAll('span')); + expect(spansBeforeReorder.map((span) => span.textContent)).toEqual(['a0', 'b1', 'c2']); + expect((globalThis as any).childRenderCounter).toBe(3); + expect((globalThis as any).childInstanceCounter).toBe(3); + + await trigger(document.body, 'button', 'click'); + + const spansAfterReorder = Array.from(document.querySelectorAll('span')); + expect(spansAfterReorder.map((span) => span.textContent)).toEqual(['c0', 'b1', 'a2']); + expect(spansAfterReorder[0]).toBe(spansBeforeReorder[2]); + expect(spansAfterReorder[1]).toBe(spansBeforeReorder[1]); + expect(spansAfterReorder[2]).toBe(spansBeforeReorder[0]); + expect((globalThis as any).childRenderCounter).toBe(3); + expect((globalThis as any).childInstanceCounter).toBe(3); // one placeholder per instance, they do not share or steal each other's - expect(document.querySelectorAll('script[q-d\\:qinit]').length).toBe(COUNT * 2); + expect(document.querySelectorAll('script[q-d\\:qinit]').length).toBe(3); + (globalThis as any).childRenderCounter = undefined; + (globalThis as any).childInstanceCounter = undefined; }); it('should not double-register events on component re-render', async () => {