Merge pull request #7550 from QwikDev/v2-signal-wrapper-rerender-fix

fix: signal wrapper should not rerender

Author: wmertens

.changeset/unlucky-olives-knock.md

@@ -0,0 +1,5 @@
+---
+'@qwik.dev/core': patch
+---
+
+fix: signal wrapper should not rerender causing missing child error

packages/docs/src/repl/worker/repl-request-handler.ts

@@ -30,20 +30,6 @@ export const requestHandler = (ev: FetchEvent) => {
             return htmlRsp;
           }
 
-          // app client modules
-          // const replEvent: ReplEventMessage = {
-          //   type: 'event',
-          //   clientId,
-          //   event: {
-          //     kind: 'client-module',
-          //     scope: 'network',
-          //     message: [reqUrl.pathname + reqUrl.search],
-          //     start: performance.now(),
-          //   },
-          // };
-
-          // sendMessageToReplServer(replEvent);
-
           return rsp;
         }
 
@@ -169,5 +155,9 @@ const injectDevHtml = (clientId: string, html?: string) => {
   }, true);
 })();`;
 
-  return `<script :>${s}</script>${html || ''}`;
+  if (!html) {
+    return '';
+  }
+
+  return html.replace(/<\/body>/i, `<script>${s}</script></body>`);
 };

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

@@ -96,8 +96,7 @@ export const enum VNodeFlags {
 }
 
 export const enum VNodeFlagsIndex {
-  mask /* ************* */ = ~0b11_111111,
-  negated_mask /* ****** */ = 0b11_111111,
+  mask /* ************** */ = 0b11_111111,
   shift /* ************* */ = 8,
 }
 

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

@@ -991,7 +991,13 @@ export const vnode_diff = (
   }
 
   function expectVirtual(type: VirtualType, jsxKey: string | null) {
-    if (vCurrent && vnode_isVirtualVNode(vCurrent) && getKey(vCurrent) === jsxKey && !!jsxKey) {
+    const checkKey = type === VirtualType.Fragment;
+    if (
+      vCurrent &&
+      vnode_isVirtualVNode(vCurrent) &&
+      getKey(vCurrent) === jsxKey &&
+      (checkKey ? !!jsxKey : true)
+    ) {
       // All is good.
       return;
     } else if (jsxKey !== null) {

packages/qwik/src/core/client/vnode-diff.unit.tsx

@@ -1,11 +1,12 @@
-import { Fragment, _jsxSorted } from '@qwik.dev/core';
+import { Fragment, _fnSignal, _jsxSorted } from '@qwik.dev/core';
 import { vnode_fromJSX } from '@qwik.dev/core/testing';
 import { describe, expect, it } from 'vitest';
 import { vnode_applyJournal, vnode_getFirstChild, vnode_getNode } from './vnode';
 import { vnode_diff } from './vnode-diff';
 import type { QElement } from '../shared/types';
 import { createSignal } from '../reactive-primitives/signal-api';
 import { QError, qError } from '../shared/error/error';
+import type { VirtualVNode } from './types';
 
 describe('vNode-diff', () => {
   it('should find no difference', () => {
@@ -430,7 +431,75 @@ describe('vNode-diff', () => {
       expect(b2).toBe(selectB2());
     });
   });
-  describe.todo('fragments', () => {});
+  describe('fragments', () => {
+    it('should not rerender signal wrapper fragment', async () => {
+      const { vNode, vParent, container } = vnode_fromJSX(
+        _jsxSorted('div', {}, null, [_jsxSorted(Fragment, {}, null, ['1'], 0, null)], 0, null)
+      );
+      const test = _jsxSorted('div', {}, null, [_fnSignal(() => '2', [], '() => "2"')], 0, null);
+      const expected = _jsxSorted(
+        'div',
+        {},
+        null,
+        [_jsxSorted(Fragment, {}, null, ['2'], 0, null)],
+        0,
+        null
+      );
+
+      const signalFragment = vnode_getFirstChild(vNode!) as VirtualVNode;
+      expect(signalFragment).toBeDefined();
+
+      vnode_diff(container, test, vParent, null);
+      vnode_applyJournal(container.$journal$);
+      expect(vNode).toMatchVDOM(expected);
+      expect(signalFragment).toBe(vnode_getFirstChild(vNode!));
+    });
+
+    it('should not rerender promise wrapper fragment', async () => {
+      const { vNode, vParent, container } = vnode_fromJSX(
+        _jsxSorted('div', {}, null, [_jsxSorted(Fragment, {}, null, ['1'], 0, null)], 0, null)
+      );
+      const test = _jsxSorted('div', {}, null, [Promise.resolve('2')], 0, null);
+      const expected = _jsxSorted(
+        'div',
+        {},
+        null,
+        [_jsxSorted(Fragment, {}, null, ['2'], 0, null)],
+        0,
+        null
+      );
+
+      const promiseFragment = vnode_getFirstChild(vNode!) as VirtualVNode;
+      expect(promiseFragment).toBeDefined();
+
+      await vnode_diff(container, test, vParent, null);
+      vnode_applyJournal(container.$journal$);
+      expect(vNode).toMatchVDOM(expected);
+      expect(promiseFragment).toBe(vnode_getFirstChild(vNode!));
+    });
+
+    it('should rerender fragment if no key', async () => {
+      const { vNode, vParent, container } = vnode_fromJSX(
+        _jsxSorted('div', {}, null, [_jsxSorted(Fragment, {}, null, ['1'], 0, null)], 0, null)
+      );
+      const test = _jsxSorted(
+        'div',
+        {},
+        null,
+        [_jsxSorted(Fragment, {}, null, ['2'], 0, null)],
+        0,
+        null
+      );
+
+      const fragment = vnode_getFirstChild(vNode!) as VirtualVNode;
+      expect(fragment).toBeDefined();
+
+      await vnode_diff(container, test, vParent, null);
+      vnode_applyJournal(container.$journal$);
+      expect(vNode).toMatchVDOM(test);
+      expect(fragment).not.toBe(vnode_getFirstChild(vNode!));
+    });
+  });
   describe('attributes', () => {
     describe('const props', () => {
       it('should set attributes', async () => {

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

@@ -1810,7 +1810,7 @@ function materializeFromVNodeData(
 
   const addVNode = (node: VNode) => {
     node[VNodeProps.flags] =
-      (node[VNodeProps.flags] & VNodeFlagsIndex.negated_mask) | (idx << VNodeFlagsIndex.shift);
+      (node[VNodeProps.flags] & VNodeFlagsIndex.mask) | (idx << VNodeFlagsIndex.shift);
     idx++;
     vLast && (vLast[VNodeProps.nextSibling] = node);
     node[VNodeProps.previousSibling] = vLast;

packages/qwik/src/testing/platform.ts

@@ -11,8 +11,20 @@ function createPlatform() {
   }
 
   let render: Queue<any> | null = null;
+  let resolveNextTickImmediate = false;
 
   const moduleCache = new Map<string, { [symbol: string]: any }>();
+  const flushNextTick = async () => {
+    await Promise.resolve();
+    if (render) {
+      try {
+        render.resolve(await render.fn());
+      } catch (e) {
+        render.reject(e);
+      }
+      render = null;
+    }
+  };
   const testPlatform: TestPlatform = {
     isServer: false,
     importSymbol(containerEl, url, symbolName) {
@@ -39,7 +51,7 @@ function createPlatform() {
         return mod[symbolName];
       });
     },
-    nextTick: (renderMarked) => {
+    nextTick: async (renderMarked) => {
       if (!render) {
         render = {
           fn: renderMarked,
@@ -51,6 +63,10 @@ function createPlatform() {
           render!.resolve = resolve;
           render!.reject = reject;
         });
+        if (resolveNextTickImmediate) {
+          resolveNextTickImmediate = false;
+          await flushNextTick();
+        }
       } else if (renderMarked !== render.fn) {
         // TODO(misko): proper error and test
         throw new Error(
@@ -69,14 +85,9 @@ function createPlatform() {
       });
     },
     flush: async () => {
-      await Promise.resolve();
-      if (render) {
-        try {
-          render.resolve(await render.fn());
-        } catch (e) {
-          render.reject(e);
-        }
-        render = null;
+      await flushNextTick();
+      if (!render) {
+        resolveNextTickImmediate = true;
       }
     },
     chunkForSymbol() {