Merge pull request #7193 from QwikDev/v2-fix-serialize-var-prop

fix: serialize var prop
QwikDev · Dec 27, 2024 · d21ff10 · d21ff10
2 parents 2a60951 + 6578707
commit d21ff10
Show file tree

Hide file tree

Showing 5 changed files with 160 additions and 129 deletions.
diff --git a/.changeset/witty-balloons-thank.md b/.changeset/witty-balloons-thank.md
@@ -0,0 +1,5 @@
+---
+'@qwik.dev/core': patch
+---
+
+fix: serialize var prop
diff --git a/packages/qwik/src/core/client/vnode-diff.ts b/packages/qwik/src/core/client/vnode-diff.ts
@@ -723,8 +723,7 @@ export const vnode_diff = (
     const jsxAttrs = [] as ClientAttrs;
     const props = jsx.varProps;
     for (const key in props) {
-      let value = props[key];
-      value = serializeAttribute(key, value, scopedStyleIdPrefix);
+      const value = props[key];
       if (value != null) {
         mapArray_set(jsxAttrs, key, value, 0);
       }
@@ -791,7 +790,7 @@ export const vnode_diff = (
         value = untrack(() => value.value);
       }
 
-      vnode_setAttr(journal, vnode, key, value);
+      vnode_setAttr(journal, vnode, key, serializeAttribute(key, value, scopedStyleIdPrefix));
       if (value === null) {
         // if we set `null` than attribute was removed and we need to shorten the dstLength
         dstLength = dstAttrs.length;
@@ -830,20 +829,20 @@ export const vnode_diff = (
         if (dstKey && isHtmlAttributeAnEventName(dstKey)) {
           patchEventDispatch = true;
           dstIdx++;
-        } else {
-          record(dstKey!, null);
+        } else if (dstKey) {
+          record(dstKey, null);
           dstIdx--;
         }
         dstKey = dstIdx < dstLength ? dstAttrs[dstIdx++] : null;
       } else if (dstKey == null) {
         // Destination has more keys, so we need to insert them from source.
         const isEvent = isJsxPropertyAnEventName(srcKey);
-        if (isEvent) {
+        if (srcKey && isEvent) {
           // Special handling for events
           patchEventDispatch = true;
           recordJsxEvent(srcKey, srcAttrs[srcIdx]);
-        } else {
-          record(srcKey!, srcAttrs[srcIdx]);
+        } else if (srcKey) {
+          record(srcKey, srcAttrs[srcIdx]);
         }
         srcIdx++;
         srcKey = srcIdx < srcLength ? srcAttrs[srcIdx++] : null;
@@ -874,11 +873,11 @@ export const vnode_diff = (
         dstKey = dstIdx < dstLength ? dstAttrs[dstIdx++] : null;
       } else {
         // Source is missing the key, so we need to remove it from destination.
-        if (isHtmlAttributeAnEventName(dstKey)) {
+        if (dstKey && isHtmlAttributeAnEventName(dstKey)) {
           patchEventDispatch = true;
           dstIdx++;
-        } else {
-          record(dstKey!, null);
+        } else if (dstKey) {
+          record(dstKey, null);
           dstIdx--;
         }
         dstKey = dstIdx < dstLength ? dstAttrs[dstIdx++] : null;

diff --git a/packages/qwik/src/core/tests/attributes.spec.tsx b/packages/qwik/src/core/tests/attributes.spec.tsx
@@ -119,50 +119,84 @@ describe.each([
     );
   });
 
-  it('should bind checked attribute', async () => {
-    const BindCmp = component$(() => {
-      const show = useSignal(false);
-      return (
-        <>
-          <label for="toggle">
-            <input type="checkbox" bind:checked={show} />
-            Show conditional
-          </label>
-          <div>{show.value.toString()}</div>
-        </>
+  describe('binding', () => {
+    it('should bind checked attribute', async () => {
+      const BindCmp = component$(() => {
+        const show = useSignal(false);
+        return (
+          <>
+            <label for="toggle">
+              <input type="checkbox" bind:checked={show} />
+              Show conditional
+            </label>
+            <div>{show.value.toString()}</div>
+          </>
+        );
+      });
+
+      const { vNode, document } = await render(<BindCmp />, { debug });
+
+      expect(vNode).toMatchVDOM(
+        <Component ssr-required>
+          <Fragment ssr-required>
+            <label for="toggle">
+              <input type="checkbox" checked={false} />
+              {'Show conditional'}
+            </label>
+            <div>false</div>
+          </Fragment>
+        </Component>
+      );
+
+      // simulate checkbox click
+      const input = document.querySelector('input')!;
+      input.checked = true;
+      await trigger(document.body, 'input', 'input');
+
+      expect(vNode).toMatchVDOM(
+        <Component ssr-required>
+          <Fragment ssr-required>
+            <label for="toggle">
+              <input type="checkbox" checked={true} />
+              {'Show conditional'}
+            </label>
+            <div>true</div>
+          </Fragment>
+        </Component>
       );
     });
 
-    const { vNode, document } = await render(<BindCmp />, { debug });
+    it('should bind textarea value', async () => {
+      const Cmp = component$(() => {
+        const value = useSignal('123');
+        return (
+          <div>
+            <textarea bind:value={value} />
+            <input bind:value={value} />
+          </div>
+        );
+      });
+      const { document } = await render(<Cmp />, { debug });
 
-    expect(vNode).toMatchVDOM(
-      <Component ssr-required>
-        <Fragment ssr-required>
-          <label for="toggle">
-            <input type="checkbox" checked={false} />
-            {'Show conditional'}
-          </label>
-          <div>false</div>
-        </Fragment>
-      </Component>
-    );
+      await expect(document.querySelector('div')).toMatchDOM(
+        <div>
+          <textarea>123</textarea>
+          <input value="123" />
+        </div>
+      );
 
-    // simulate checkbox click
-    const input = document.querySelector('input')!;
-    input.checked = true;
-    await trigger(document.body, 'input', 'input');
+      // simulate input
+      const textarea = document.querySelector('textarea')!;
+      textarea.value = 'abcd';
+      await trigger(document.body, textarea, 'input');
 
-    expect(vNode).toMatchVDOM(
-      <Component ssr-required>
-        <Fragment ssr-required>
-          <label for="toggle">
-            <input type="checkbox" checked={true} />
-            {'Show conditional'}
-          </label>
-          <div>true</div>
-        </Fragment>
-      </Component>
-    );
+      await expect(document.querySelector('div')).toMatchDOM(
+        <div>
+          <textarea>abcd</textarea>
+          <input value="abcd" />
+        </div>
+      );
+    });
   });
 
   it('should render preventdefault attribute', async () => {

diff --git a/packages/qwik/src/core/tests/render-styles.spec.tsx b/packages/qwik/src/core/tests/render-styles.spec.tsx
@@ -1,9 +1,15 @@
 import {
   Fragment as Component,
   component$,
+  createContextId,
   Fragment,
   Fragment as Signal,
+  type Signal as SignalType,
   useStore,
+  useContext,
+  useComputed$,
+  useSignal,
+  useContextProvider,
 } from '@qwik.dev/core';
 import { domRender, ssrRenderToDom, trigger } from '@qwik.dev/core/testing';
 import { describe, expect, it } from 'vitest';
@@ -70,4 +76,71 @@ describe.each([
       </Component>
     );
   });
+
+  it('should render styles from computed and context', async () => {
+    const Ctx = createContextId<{
+      val: SignalType<Record<string, number>>;
+      abc: SignalType<number>;
+    }>('test');
+
+    const Child = component$(() => {
+      const ctx = useContext(Ctx);
+      // use computed to add context value as dependency
+      const color = useComputed$(() => (ctx.abc.value > 0 ? 'red' : 'green'));
+      return (
+        // use spread props to convert div props to var props
+        <div style={{ color: color.value }} {...ctx.val.value}>
+          Abcd
+        </div>
+      );
+    });
+
+    const Parent = component$(() => {
+      const signal = useSignal(0);
+      // use computed to create a new object
+      const comp = useComputed$<Record<string, number>>(() => ({
+        'data-value': signal.value,
+      }));
+      useContextProvider(Ctx, {
+        val: comp,
+        abc: signal,
+      });
+      return (
+        <div>
+          <Child />
+          <button onClick$={() => signal.value++}></button>
+        </div>
+      );
+    });
+
+    const { vNode, container } = await render(<Parent />, { debug });
+
+    expect(vNode).toMatchVDOM(
+      <Component ssr-required>
+        <div>
+          <Component ssr-required>
+            <div style="color:green" data-value={0}>
+              Abcd
+            </div>
+          </Component>
+          <button />
+        </div>
+      </Component>
+    );
+
+    await trigger(container.element, 'button', 'click');
+
+    expect(vNode).toMatchVDOM(
+      <Component ssr-required>
+        <div>
+          <Component ssr-required>
+            <div style="color:red" data-value={1}>
+              Abcd
+            </div>
+          </Component>
+          <button />
+        </div>
+      </Component>
+    );
+  });
 });
diff --git a/packages/qwik/src/core/tests/use-signal.spec.tsx b/packages/qwik/src/core/tests/use-signal.spec.tsx
@@ -553,86 +553,6 @@ describe.each([
     });
   });
 
-  describe('binding', () => {
-    it('should bind checked attribute', async () => {
-      const BindCmp = component$(() => {
-        const show = useSignal(false);
-        return (
-          <>
-            <label for="toggle">
-              <input type="checkbox" bind:checked={show} />
-              Show conditional
-            </label>
-            <div>{show.value.toString()}</div>
-          </>
-        );
-      });
-
-      const { vNode, document } = await render(<BindCmp />, { debug });
-
-      expect(vNode).toMatchVDOM(
-        <Component ssr-required>
-          <Fragment ssr-required>
-            <label for="toggle">
-              <input type="checkbox" checked={false} />
-              {'Show conditional'}
-            </label>
-            <div>false</div>
-          </Fragment>
-        </Component>
-      );
-
-      // simulate checkbox click
-      const input = document.querySelector('input')!;
-      input.checked = true;
-      await trigger(document.body, 'input', 'input');
-
-      expect(vNode).toMatchVDOM(
-        <Component ssr-required>
-          <Fragment ssr-required>
-            <label for="toggle">
-              <input type="checkbox" checked={true} />
-              {'Show conditional'}
-            </label>
-            <div>true</div>
-          </Fragment>
-        </Component>
-      );
-    });
-
-    it('should bind textarea value', async () => {
-      const Cmp = component$(() => {
-        const value = useSignal('123');
-        return (
-          <div>
-            <textarea bind:value={value} />
-            <input bind:value={value} />
-          </div>
-        );
-      });
-      const { document } = await render(<Cmp />, { debug });
-
-      await expect(document.querySelector('div')).toMatchDOM(
-        <div>
-          <textarea>123</textarea>
-          <input value="123" />
-        </div>
-      );
-
-      // simulate input
-      const textarea = document.querySelector('textarea')!;
-      textarea.value = 'abcd';
-      await trigger(document.body, textarea, 'input');
-
-      await expect(document.querySelector('div')).toMatchDOM(
-        <div>
-          <textarea>abcd</textarea>
-          <input value="abcd" />
-        </div>
-      );
-    });
-  });
-
   describe('regression', () => {
     it('#4249 - should render signal text with double condition', async () => {
       const Issue4249 = component$(() => {