Merge pull request #8715 from maiieul/fix-duplicate-projected-children

fix(core): reset creation mode when diffing into an existing projection

Author: maiieul

.changeset/lucky-spiders-wonder.md

@@ -0,0 +1,5 @@
+---
+'@qwik.dev/core': patch
+---
+
+fix: duplicate projected element children when a component throws a promise on first render

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

@@ -409,9 +409,7 @@ function diff(
                   diffContext,
                   diffContext.$jsxValue$.children,
                   true,
-                  // special case for projection, we don't want to expect no children
-                  // because the projection's children are not removed
-                  false
+                  /* isProjection */ true
                 );
               } else if (type === SSRComment) {
                 expectNoMore(diffContext);
@@ -538,15 +536,17 @@ function advance(diffContext: DiffContext) {
  *
  *   In the above example all nodes are on same level so we don't `descendVNode` even thought there is
  *   an array produced by the `map` function.
+ * @param isProjection - True when descending into a Projection, whose children survive re-renders:
+ *   they are diffed against (never re-created) and kept when the new JSX has no children.
  */
 function descend(
   diffContext: DiffContext,
   children: JSXChildren,
   descendVNode: boolean,
-  shouldExpectNoChildren: boolean = true
+  isProjection: boolean = false
 ) {
   if (
-    shouldExpectNoChildren &&
+    !isProjection &&
     (children == null || (descendVNode && isArray(children) && children.length === 0))
   ) {
     expectNoChildren(diffContext);
@@ -560,7 +560,9 @@ function descend(
         'Expecting vCurrent to be defined.'
       );
     const creationMode =
-      diffContext.$isCreationMode$ ||
+      // a projection keeps its children across re-renders, so it never inherits
+      // creation mode from a not-yet-rendered host; create only if new or empty
+      (!isProjection && diffContext.$isCreationMode$) ||
       !!diffContext.$vNewNode$ ||
       !vnode_getFirstChild(diffContext.$vCurrent$!);
 

packages/qwik/src/core/tests/render-promise.spec.tsx

@@ -1,4 +1,4 @@
-import { Fragment as Component, Fragment, component$, useSignal } from '@qwik.dev/core';
+import { Fragment as Component, Fragment, Slot, component$, useSignal } from '@qwik.dev/core';
 import { domRender, ssrRenderToDom } from '@qwik.dev/core/testing';
 import { describe, expect, it } from 'vitest';
 
@@ -43,4 +43,73 @@ describe.each([
     const { document } = await render(<Cmp />, { debug });
     expect(document.querySelector('div')).toHaveProperty('textContent', 'child');
   });
+
+  it('should not duplicate projected element children when component throws a promise on first render', async () => {
+    const Wrapper = component$(() => (
+      <form>
+        <Slot />
+      </form>
+    ));
+    const Title = component$(() => <header>title</header>);
+    const Footer = component$(() => <footer>footer</footer>);
+    const Cmp = component$(() => {
+      // The thrown promise writes a tracked signal, scheduling a second diff
+      // pass before <Wrapper> has rendered its <Slot>. The trailing <Footer />
+      // is required: matching it moves the cursor past the stale <div>, so
+      // expectNoMore() would not clean it up.
+      const store = useSignal<string>();
+      if (!store.value) {
+        throw Promise.resolve().then(() => {
+          store.value = 'resolved';
+        });
+      }
+      return (
+        <Wrapper>
+          <Title />
+          <div class="fields">{store.value}</div>
+          <Footer />
+        </Wrapper>
+      );
+    });
+    const { document } = await render(<Cmp />, { debug });
+    expect(document.querySelectorAll('div.fields')).toHaveLength(1);
+    expect(document.querySelectorAll('header')).toHaveLength(1);
+    expect(document.querySelectorAll('footer')).toHaveLength(1);
+  });
+
+  it('should not duplicate named slot content when component throws a promise on first render', async () => {
+    const Wrapper = component$(() => (
+      <form>
+        <section>
+          <Slot name="top" />
+        </section>
+        <Slot />
+      </form>
+    ));
+    const Title = component$(() => <header>title</header>);
+    const Footer = component$(() => <footer>footer</footer>);
+    const Cmp = component$(() => {
+      const store = useSignal<string>();
+      if (!store.value) {
+        throw Promise.resolve().then(() => {
+          store.value = 'resolved';
+        });
+      }
+      return (
+        <Wrapper>
+          <div q:slot="top" class="banner">
+            banner
+          </div>
+          <Title />
+          <div class="fields">{store.value}</div>
+          <Footer />
+        </Wrapper>
+      );
+    });
+    const { document } = await render(<Cmp />, { debug });
+    expect(document.querySelectorAll('div.banner')).toHaveLength(1);
+    expect(document.querySelectorAll('div.fields')).toHaveLength(1);
+    expect(document.querySelectorAll('header')).toHaveLength(1);
+    expect(document.querySelectorAll('footer')).toHaveLength(1);
+  });
 });