fix: prevent reusing projection if is marked as deleted

Author: Varixo

.changeset/tasty-penguins-ring.md

@@ -0,0 +1,5 @@
+---
+'@qwik.dev/core': patch
+---
+
+fix: prevent reusing projection if is marked as deleted

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

@@ -458,6 +458,8 @@ export const vnode_diff = (
       slotName,
       (id) => vnode_locate(container.rootVNode, id)
     );
+    // if projection is marked as deleted then we need to create a new one
+    vCurrent = vCurrent && vCurrent[VNodeProps.flags] & VNodeFlags.Deleted ? null : vCurrent;
     if (vCurrent == null) {
       vNewNode = vnode_newVirtual();
       // you may be tempted to add the projection into the current parent, but
@@ -566,8 +568,8 @@ export const vnode_diff = (
       while (vCurrent) {
         const toRemove = vCurrent;
         advanceToNextSibling();
-        cleanup(container, toRemove);
         if (vParent === vnode_getParent(toRemove)) {
+          cleanup(container, toRemove);
           // If we are diffing projection than the parent is not the parent of the node.
           // If that is the case we don't want to remove the node from the parent.
           vnode_remove(journal, vParent as ElementVNode | VirtualVNode, toRemove, true);
@@ -1290,6 +1292,7 @@ export function cleanup(container: ClientContainer, vNode: VNode) {
   let vCursor: VNode | null = vNode;
   // Depth first traversal
   if (vnode_isTextVNode(vNode)) {
+    markVNodeAsDeleted(vCursor);
     // Text nodes don't have subscriptions or children;
     return;
   }
@@ -1368,6 +1371,8 @@ export function cleanup(container: ClientContainer, vNode: VNode) {
           return;
         }
       }
+    } else if (type & VNodeFlags.Text) {
+      markVNodeAsDeleted(vCursor);
     }
     // Out of children
     if (vCursor === vNode) {

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

@@ -774,53 +774,71 @@ export const vnode_journalToString = (journal: VNodeJournal): string => {
 
   function stringify(...args: any[]) {
     lines.push(
-      '  ' +
-        args
-          .map((arg) => {
-            if (typeof arg === 'string') {
-              return arg;
-            } else if (arg && isHtmlElement(arg)) {
-              const html = arg.outerHTML;
-              const idx = html.indexOf('>');
-              return '\n    ' + (idx > 0 ? html.substring(0, idx + 1) : html);
-            } else if (arg && isText(arg)) {
-              return JSON.stringify(arg.nodeValue);
-            } else {
-              return String(arg);
-            }
-          })
-          .join(' ')
+      args
+        .map((arg) => {
+          if (typeof arg === 'string') {
+            return arg;
+          } else if (arg && isHtmlElement(arg)) {
+            const html = arg.outerHTML;
+            const hasChildNodes = !!arg.firstElementChild;
+            const idx = html.indexOf('>');
+            const lastIdx = html.lastIndexOf('<');
+            return idx > 0 && hasChildNodes
+              ? html.substring(0, idx + 1) + '...' + html.substring(lastIdx)
+              : html;
+          } else if (arg && isText(arg)) {
+            return JSON.stringify(arg.nodeValue);
+          } else {
+            return String(arg);
+          }
+        })
+        .join(' ')
     );
   }
 
   while (idx < length) {
     const op = journal[idx++] as VNodeJournalOpCode;
     switch (op) {
       case VNodeJournalOpCode.SetText:
-        stringify('SetText', journal[idx++], journal[idx++]);
+        stringify('SetText');
+        stringify('  ', journal[idx++]);
+        stringify('   -->', journal[idx++]);
         break;
       case VNodeJournalOpCode.SetAttribute:
-        stringify('SetAttribute', journal[idx++], journal[idx++], journal[idx++]);
+        stringify('SetAttribute');
+        stringify('  ', journal[idx++]);
+        stringify('   key', journal[idx++]);
+        stringify('   val', journal[idx++]);
         break;
       case VNodeJournalOpCode.HoistStyles:
         stringify('HoistStyles');
         break;
-      case VNodeJournalOpCode.Remove:
-        stringify('Remove', journal[idx++]);
+      case VNodeJournalOpCode.Remove: {
+        stringify('Remove');
+        const parent = journal[idx++];
+        stringify('  ', parent);
         let nodeToRemove: any;
         while (idx < length && typeof (nodeToRemove = journal[idx]) !== 'number') {
-          stringify('  ', nodeToRemove);
+          stringify('   -->', nodeToRemove);
           idx++;
         }
         break;
-      case VNodeJournalOpCode.Insert:
-        stringify('Insert', journal[idx++], journal[idx++]);
+      }
+      case VNodeJournalOpCode.Insert: {
+        stringify('Insert');
+        const parent = journal[idx++];
+        const insertBefore = journal[idx++];
+        stringify('  ', parent);
         let newChild: any;
         while (idx < length && typeof (newChild = journal[idx]) !== 'number') {
-          stringify('  ', newChild);
+          stringify('   -->', newChild);
           idx++;
         }
+        if (insertBefore) {
+          stringify('      ', insertBefore);
+        }
         break;
+      }
     }
   }
   lines.push('END JOURNAL');
@@ -969,6 +987,7 @@ export const vnode_insertBefore = (
   //   : insertBefore;
   const domParentVNode = vnode_getDomParentVNode(parent);
   const parentNode = domParentVNode && domParentVNode[ElementVNodeProps.element];
+
   if (parentNode) {
     const domChildren = vnode_getDomChildrenWithCorrectNamespacesToInsert(
       journal,
@@ -1037,6 +1056,7 @@ export const vnode_remove = (
   if (vnode_isTextVNode(vToRemove)) {
     vnode_ensureTextInflated(journal, vToRemove);
   }
+
   const vPrevious = vToRemove[VNodeProps.previousSibling];
   const vNext = vToRemove[VNodeProps.nextSibling];
   if (vPrevious) {

packages/qwik/src/core/tests/projection.spec.tsx

@@ -1127,6 +1127,68 @@ describe.each([
     );
   });
 
+  it('should cleanup removed projection and remove it from parent component', async () => {
+    const SomeCmp = component$((props: { toggle: boolean }) => {
+      return <>{props.toggle && <Slot />}</>;
+    });
+
+    const Cmp = component$(() => {
+      const toggle = useSignal(true);
+
+      return (
+        <>
+          <button onClick$={() => (toggle.value = !toggle.value)}></button>
+          <SomeCmp toggle={toggle.value}>
+            {toggle.value && <h1>Title 1</h1>}
+            {!toggle.value && <h1>Title 2</h1>}
+          </SomeCmp>
+        </>
+      );
+    });
+
+    const { document, vNode } = await render(<Cmp />, { debug: DEBUG });
+    expect(vNode).toMatchVDOM(
+      <Component ssr-required>
+        <Fragment ssr-required>
+          <button></button>
+          <Component ssr-required>
+            <Fragment ssr-required>
+              <Projection ssr-required>
+                <h1>Title 1</h1>
+              </Projection>
+            </Fragment>
+          </Component>
+        </Fragment>
+      </Component>
+    );
+    await trigger(document.body, 'button', 'click');
+    expect(vNode).toMatchVDOM(
+      <Component ssr-required>
+        <Fragment ssr-required>
+          <button></button>
+          <Component ssr-required>
+            <Fragment ssr-required></Fragment>
+          </Component>
+        </Fragment>
+      </Component>
+    );
+    await trigger(document.body, 'button', 'click');
+    expect(vNode).toMatchVDOM(
+      <Component ssr-required>
+        <Fragment ssr-required>
+          <button></button>
+          <Component ssr-required>
+            <Fragment ssr-required>
+              <Projection ssr-required>
+                <h1>Title 1</h1>
+              </Projection>
+            </Fragment>
+          </Component>
+        </Fragment>
+      </Component>
+    );
+  });
+
   describe('ensureProjectionResolved', () => {
     (globalThis as any).log = [] as string[];
     beforeEach(() => {