Merge pull request #8117 from QwikDev/v2-attribute-promises

feat: support promises in attributes

Author: wmertens

.changeset/heavy-clouds-walk.md

@@ -0,0 +1,5 @@
+---
+'@qwik.dev/core': minor
+---
+
+feat: support promises in attributes

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

@@ -14,7 +14,7 @@ import { JSXNodeImpl, isJSXNode } from '../shared/jsx/jsx-node';
 import { Fragment, type Props } from '../shared/jsx/jsx-runtime';
 import { directGetPropsProxyProp, type PropsProxy } from '../shared/jsx/props-proxy';
 import { Slot } from '../shared/jsx/slot.public';
-import type { JSXNodeInternal, JSXOutput } from '../shared/jsx/types/jsx-node';
+import type { JSXNodeInternal } from '../shared/jsx/types/jsx-node';
 import type { JSXChildren } from '../shared/jsx/types/jsx-qwik-attributes';
 import { SSRComment, SSRRaw, SkipRender } from '../shared/jsx/utils.public';
 import type { QRLInternal } from '../shared/qrl/qrl-class';
@@ -44,7 +44,7 @@ import {
   Q_PREFIX,
   dangerouslySetInnerHTML,
 } from '../shared/utils/markers';
-import { isPromise } from '../shared/utils/promises';
+import { isPromise, retryOnPromise } from '../shared/utils/promises';
 import { isSlotProp } from '../shared/utils/prop';
 import { hasClassAttr } from '../shared/utils/scoped-styles';
 import { serializeAttribute } from '../shared/utils/styles';
@@ -84,7 +84,7 @@ import { getAttributeNamespace, getNewElementNamespaceData } from './vnode-names
 
 export const vnode_diff = (
   container: ClientContainer,
-  jsxNode: JSXOutput,
+  jsxNode: JSXChildren,
   vStartNode: VNode,
   scopedStyleIdPrefix: string | null
 ) => {
@@ -98,8 +98,7 @@ export const vnode_diff = (
    */
   const stack: any[] = [];
 
-  const asyncQueue: Array<VNode | ValueOrPromise<JSXOutput> | Promise<JSXOutput | JSXChildren>> =
-    [];
+  const asyncQueue: Array<VNode | ValueOrPromise<JSXChildren> | Promise<JSXChildren>> = [];
 
   ////////////////////////////////
   //// Traverse state variables
@@ -151,7 +150,7 @@ export const vnode_diff = (
   //////////////////////////////////////////////
   //////////////////////////////////////////////
 
-  function diff(jsxNode: JSXOutput, vStartNode: VNode) {
+  function diff(jsxNode: JSXChildren, vStartNode: VNode) {
     assertFalse(vnode_isVNode(jsxNode), 'JSXNode should not be a VNode');
     assertTrue(vnode_isVNode(vStartNode), 'vStartNode should be a VNode');
     vParent = vStartNode as ElementVNode | VirtualVNode;
@@ -182,12 +181,10 @@ export const vnode_diff = (
               EffectSubscriptionProp.CONSUMER
             ];
             if (currentSignal !== unwrappedSignal) {
+              const vHost = (vNewNode || vCurrent)!;
               descend(
-                trackSignalAndAssignHost(
-                  unwrappedSignal,
-                  (vNewNode || vCurrent)!,
-                  EffectProperty.VNODE,
-                  container
+                resolveSignalAndDescend(() =>
+                  trackSignalAndAssignHost(unwrappedSignal, vHost, EffectProperty.VNODE, container)
                 ),
                 true
               );
@@ -246,6 +243,21 @@ export const vnode_diff = (
     }
   }
 
+  function resolveSignalAndDescend(fn: () => ValueOrPromise<any>): ValueOrPromise<any> {
+    try {
+      return fn();
+    } catch (e) {
+      // Signal threw a promise (async computed signal) - handle retry and async queue
+      if (isPromise(e)) {
+        // The thrown promise will resolve when the signal is ready, then retry fn() with retry logic
+        const retryPromise = e.then(() => retryOnPromise(fn));
+        asyncQueue.push(retryPromise, vNewNode || vCurrent);
+        return null;
+      }
+      throw e;
+    }
+  }
+
   function advance() {
     if (!shouldAdvance) {
       shouldAdvance = true;
@@ -530,13 +542,19 @@ export const vnode_diff = (
 
   function drainAsyncQueue(): ValueOrPromise<void> {
     while (asyncQueue.length) {
-      const jsxNode = asyncQueue.shift() as ValueOrPromise<JSXNodeInternal>;
+      const jsxNode = asyncQueue.shift() as ValueOrPromise<JSXChildren>;
       const vHostNode = asyncQueue.shift() as VNode;
+
       if (isPromise(jsxNode)) {
-        return jsxNode.then((jsxNode) => {
-          diff(jsxNode, vHostNode);
-          return drainAsyncQueue();
-        });
+        return jsxNode
+          .then((jsxNode) => {
+            diff(jsxNode, vHostNode);
+            return drainAsyncQueue();
+          })
+          .catch((e) => {
+            container.handleError(e, vHostNode);
+            return drainAsyncQueue();
+          });
       } else {
         diff(jsxNode, vHostNode);
       }
@@ -594,6 +612,21 @@ export const vnode_diff = (
   ): boolean {
     const element = createElementWithNamespace(elementName);
 
+    function setAttribute(key: string, value: any, vHost: ElementVNode) {
+      value = serializeAttribute(key, value, scopedStyleIdPrefix);
+      if (value != null) {
+        if (vHost.flags & VNodeFlags.NS_svg) {
+          // only svg elements can have namespace attributes
+          const namespace = getAttributeNamespace(key);
+          if (namespace) {
+            element.setAttributeNS(namespace, key, String(value));
+            return;
+          }
+        }
+        element.setAttribute(key, String(value));
+      }
+    }
+
     const { constProps } = jsx;
     let needsQDispatchEventPatch = false;
     if (constProps) {
@@ -637,15 +670,19 @@ export const vnode_diff = (
         }
 
         if (isSignal(value)) {
-          value = trackSignalAndAssignHost(
-            value as Signal<unknown>,
-            vNewNode as ElementVNode,
-            key,
-            container,
-            CONST_SUBSCRIPTION_DATA
+          const vHost = vNewNode as ElementVNode;
+          const signal = value as Signal<unknown>;
+          value = retryOnPromise(() =>
+            trackSignalAndAssignHost(signal, vHost, key, container, CONST_SUBSCRIPTION_DATA)
           );
         }
 
+        if (isPromise(value)) {
+          const vHost = vNewNode as ElementVNode;
+          value.then((resolvedValue) => setAttribute(key, resolvedValue, vHost));
+          continue;
+        }
+
         if (key === dangerouslySetInnerHTML) {
           if (value) {
             element.innerHTML = String(value);
@@ -665,18 +702,7 @@ export const vnode_diff = (
           continue;
         }
 
-        value = serializeAttribute(key, value, scopedStyleIdPrefix);
-        if (value != null) {
-          if (vNewNode!.flags & VNodeFlags.NS_svg) {
-            // only svg elements can have namespace attributes
-            const namespace = getAttributeNamespace(key);
-            if (namespace) {
-              element.setAttributeNS(namespace, key, String(value));
-              continue;
-            }
-          }
-          element.setAttribute(key, String(value));
-        }
+        setAttribute(key, value, vNewNode as ElementVNode);
       }
     }
     const key = jsx.key;
@@ -803,6 +829,14 @@ export const vnode_diff = (
     let dstIdx = 0;
     let patchEventDispatch = false;
 
+    const setAttribute = (key: string, value: any, vHost: ElementVNode) => {
+      vHost.setAttr(
+        key,
+        value !== null ? serializeAttribute(key, value, scopedStyleIdPrefix) : null,
+        journal
+      );
+    };
+
     const record = (key: string, value: any) => {
       if (key.startsWith(':')) {
         vnode.setProp(key, value);
@@ -837,12 +871,16 @@ export const vnode_diff = (
           // Only if we want to track the signal again
           clearEffectSubscription(container, currentEffect);
         }
-        value = trackSignalAndAssignHost(
-          unwrappedSignal,
-          vnode,
-          key,
-          container,
-          NON_CONST_SUBSCRIPTION_DATA
+
+        const vHost = vnode as ElementVNode;
+        value = retryOnPromise(() =>
+          trackSignalAndAssignHost(
+            unwrappedSignal,
+            vHost,
+            key,
+            container,
+            NON_CONST_SUBSCRIPTION_DATA
+          )
         );
       } else {
         if (currentEffect) {
@@ -853,11 +891,13 @@ export const vnode_diff = (
         }
       }
 
-      vnode.setAttr(
-        key,
-        value !== null ? serializeAttribute(key, value, scopedStyleIdPrefix) : null,
-        journal
-      );
+      if (isPromise(value)) {
+        const vHost = vnode as ElementVNode;
+        value.then((resolvedValue) => setAttribute(key, resolvedValue, vHost));
+        return;
+      }
+
+      setAttribute(key, value, vnode);
     };
 
     const recordJsxEvent = (key: string, value: any) => {
@@ -1113,11 +1153,8 @@ export const vnode_diff = (
   function expectVirtual(type: VirtualType, jsxKey: string | null) {
     const checkKey = type === VirtualType.Fragment;
     const currentKey = getKey(vCurrent);
-    const isSameNode =
-      vCurrent &&
-      vnode_isVirtualVNode(vCurrent) &&
-      currentKey === jsxKey &&
-      (checkKey ? !!jsxKey : true);
+    const currentIsVirtual = vCurrent && vnode_isVirtualVNode(vCurrent);
+    const isSameNode = currentIsVirtual && currentKey === jsxKey && (checkKey ? !!jsxKey : true);
 
     if (isSameNode) {
       // All is good.
@@ -1136,7 +1173,7 @@ export const vnode_diff = (
       isDev && (vNewNode as VirtualVNode).setProp(DEBUG_TYPE, type);
     };
     // For fragments without a key, always create a new virtual node (ensures rerender semantics)
-    if (checkKey && jsxKey === null) {
+    if (jsxKey === null) {
       createNew();
       return;
     }

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

@@ -1,4 +1,4 @@
-import { Fragment, _fnSignal, _jsxSorted, component$ } from '@qwik.dev/core';
+import { Fragment, _fnSignal, _jsxSorted, component$, type JSXOutput } from '@qwik.dev/core';
 import { vnode_fromJSX } from '@qwik.dev/core/testing';
 import { describe, expect, it } from 'vitest';
 import type { SignalImpl } from '../reactive-primitives/impl/signal-impl';
@@ -534,6 +534,19 @@ describe('vNode-diff', () => {
       expect(vNode).toMatchVDOM(test);
       expect(fragment).not.toBe(vnode_getFirstChild(vNode!));
     });
+
+    it('should render fragment if only text was available', async () => {
+      const { vParent, container } = vnode_fromJSX('1');
+      const test = Promise.resolve('2') as unknown as JSXOutput; //_jsxSorted(Fragment, {}, null, ['1'], 0, null);
+
+      await vnode_diff(container, test, vParent, null);
+      vnode_applyJournal(container.$journal$);
+      expect(vParent).toMatchVDOM(
+        <body>
+          <Fragment>2</Fragment>
+        </body>
+      );
+    });
   });
   describe('attributes', () => {
     describe('const props', () => {

packages/qwik/src/core/ssr/ssr-types.ts

@@ -16,7 +16,7 @@ import type { SsrNodeFlags } from '../shared/types';
 import type { ResourceReturnInternal } from '../use/use-resource';
 
 export type SsrAttrKey = string;
-export type SsrAttrValue = string | Signal<any> | boolean | object | null;
+export type SsrAttrValue = string | Signal<any> | Promise<any> | boolean | object | null;
 export type SsrAttrs = Array<SsrAttrKey | SsrAttrValue>;
 
 /** @internal */

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

@@ -17,11 +17,6 @@ import { ELEMENT_BACKPATCH_DATA } from '../../server/qwik-copy';
 const debug = false; //true;
 Error.stackTraceLimit = 100;
 
-vi.hoisted(() => {
-  vi.stubGlobal('QWIK_BACKPATCH_EXECUTOR_MINIFIED', 'min');
-  vi.stubGlobal('QWIK_BACKPATCH_EXECUTOR_DEBUG', 'debug');
-});
-
 describe('SSR Backpatching', () => {
   it('should handle basic backpatching', async () => {
     const Ctx = createContextId<{ descId: Signal<string> }>('bp-ctx-1');

packages/qwik/src/core/tests/use-async-computed.spec.tsx

@@ -1,4 +1,12 @@
-import { Fragment as Signal, component$, useSignal, useTask$ } from '@qwik.dev/core';
+import {
+  $,
+  Fragment as Signal,
+  _jsxSorted,
+  _wrapProp,
+  component$,
+  useSignal,
+  useTask$,
+} from '@qwik.dev/core';
 import { domRender, ssrRenderToDom, trigger, waitForDrain } from '@qwik.dev/core/testing';
 import { describe, expect, it } from 'vitest';
 import { useAsyncComputed$ } from '../use/use-async-computed';
@@ -129,6 +137,57 @@ describe.each([
     );
   });
 
+  it('should render as attribute', async () => {
+    const Counter = component$(() => {
+      const count = useSignal(1);
+      const doubleCount = useAsyncComputed$(({ track }) => Promise.resolve(track(count) * 2));
+      return <button data-count={doubleCount.value} onClick$={() => count.value++}></button>;
+    });
+    const { vNode, container } = await render(<Counter />, { debug });
+    expect(vNode).toMatchVDOM(
+      <>
+        <button data-count="2"></button>
+      </>
+    );
+    await trigger(container.element, 'button', 'click');
+    expect(vNode).toMatchVDOM(
+      <>
+        <button data-count="4"></button>
+      </>
+    );
+  });
+
+  it('should render var prop as attribute', async () => {
+    const Counter = component$(() => {
+      const count = useSignal(1);
+      const doubleCount = useAsyncComputed$(({ track }) => Promise.resolve(track(count) * 2));
+      return _jsxSorted(
+        'button',
+        {
+          'data-count': _wrapProp(doubleCount, 'value'),
+          onClick$: $(() => count.value++),
+        },
+        null,
+        null,
+        0,
+        null,
+        undefined
+      );
+    });
+    const { vNode, container } = await render(<Counter />, { debug });
+    expect(vNode).toMatchVDOM(
+      <>
+        <button data-count="2"></button>
+      </>
+    );
+    await trigger(container.element, 'button', 'click');
+    expect(vNode).toMatchVDOM(
+      <>
+        <button data-count="4"></button>
+      </>
+    );
+  });
+
   describe('loading', () => {
     it('should show loading state', async () => {
       (globalThis as any).delay = () =>