feat: reactive var props

.changeset/dull-insects-slide.md

@@ -0,0 +1,5 @@
+---
+'@qwik.dev/core': minor
+---
+
+feat: make props more reactive for var props

packages/qwik/src/core/client/chore-array.ts

@@ -1,6 +1,7 @@
 import { AsyncComputedSignalImpl } from '../reactive-primitives/impl/async-computed-signal-impl';
 import { StoreHandler } from '../reactive-primitives/impl/store';
 import { assertFalse } from '../shared/error/assert';
+import { PropsProxyHandler } from '../shared/jsx/props-proxy';
 import { isQrl } from '../shared/qrl/qrl-utils';
 import type { Chore } from '../shared/scheduler';
 import {
@@ -125,6 +126,7 @@ export function choreComparator(a: Chore, b: Chore): number {
     a.$type$ === ChoreType.RECOMPUTE_AND_SCHEDULE_EFFECTS &&
     b.$type$ === ChoreType.RECOMPUTE_AND_SCHEDULE_EFFECTS &&
     ((a.$target$ instanceof StoreHandler && b.$target$ instanceof StoreHandler) ||
+      (a.$target$ instanceof PropsProxyHandler && b.$target$ instanceof PropsProxyHandler) ||
       (a.$target$ instanceof AsyncComputedSignalImpl &&
         b.$target$ instanceof AsyncComputedSignalImpl)) &&
     a.$payload$ !== b.$payload$

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

@@ -16,7 +16,12 @@ import { assertDefined, assertFalse, assertTrue } from '../shared/error/assert';
 import { QError, qError } from '../shared/error/error';
 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 {
+  directGetPropsProxyProp,
+  triggerPropsProxyEffect,
+  type PropsProxy,
+  type PropsProxyHandler,
+} from '../shared/jsx/props-proxy';
 import { Slot } from '../shared/jsx/slot.public';
 import type { JSXNodeInternal } from '../shared/jsx/types/jsx-node';
 import type { JSXChildren } from '../shared/jsx/types/jsx-qwik-attributes';
@@ -28,7 +33,7 @@ import type { HostElement, QElement, QwikLoaderEventScope, qWindow } from '../sh
 import { DEBUG_TYPE, QContainerValue, VirtualType } from '../shared/types';
 import { ChoreType } from '../shared/util-chore-type';
 import { escapeHTML } from '../shared/utils/character-escaping';
-import { _OWNER } from '../shared/utils/constants';
+import { _OWNER, _PROPS_HANDLER } from '../shared/utils/constants';
 import {
   fromCamelToKebabCase,
   getEventDataFromHtmlAttribute,
@@ -186,10 +191,10 @@ export const vnode_diff = (
             expectVirtual(VirtualType.WrappedSignal, null);
             const unwrappedSignal =
               jsxValue instanceof WrappedSignalImpl ? jsxValue.$unwrapIfSignal$() : jsxValue;
-            const currentSignal = vCurrent?.[_EFFECT_BACK_REF]?.get(EffectProperty.VNODE)?.[
-              EffectSubscriptionProp.CONSUMER
-            ];
-            if (currentSignal !== unwrappedSignal) {
+            const hasUnwrappedSignal = vCurrent?.[_EFFECT_BACK_REF]
+              ?.get(EffectProperty.VNODE)
+              ?.[EffectSubscriptionProp.BACK_REF]?.has(unwrappedSignal);
+            if (!hasUnwrappedSignal) {
               const vHost = (vNewNode || vCurrent)!;
               descend(
                 resolveSignalAndDescend(() =>
@@ -1215,7 +1220,7 @@ export const vnode_diff = (
     let host = (vNewNode || vCurrent) as VirtualVNode | null;
     const jsxNode = jsxValue as JSXNodeInternal;
     if (componentMeta) {
-      const jsxProps = jsxNode.props;
+      const jsxProps = jsxNode.props as PropsProxy;
       // QComponent
       let shouldRender = false;
       const [componentQRL] = componentMeta;
@@ -1246,41 +1251,15 @@ export const vnode_diff = (
       }
 
       if (host) {
-        let vNodeProps = (host as VirtualVNode).getProp<any>(
+        const vNodeProps = (host as VirtualVNode).getProp<PropsProxy | null>(
           ELEMENT_PROPS,
           container.$getObjectById$
         );
-        let propsAreDifferent = false;
         if (!shouldRender) {
-          propsAreDifferent =
-            propsDiffer(
-              (jsxProps as PropsProxy)[_CONST_PROPS],
-              (vNodeProps as PropsProxy)?.[_CONST_PROPS]
-            ) ||
-            propsDiffer(
-              (jsxProps as PropsProxy)[_VAR_PROPS],
-              (vNodeProps as PropsProxy)?.[_VAR_PROPS]
-            );
-          shouldRender = shouldRender || propsAreDifferent;
+          shouldRender ||= handleProps(host, jsxProps, vNodeProps, container);
         }
 
         if (shouldRender) {
-          if (propsAreDifferent) {
-            if (vNodeProps) {
-              // Reuse the same props instance, qrls can use the current props instance
-              // as a capture ref, so we can't change it.
-              // We need to do this directly, because normally we would subscribe to the signals
-              // if any signal is there.
-              vNodeProps[_CONST_PROPS] = (jsxProps as PropsProxy)[_CONST_PROPS];
-              vNodeProps[_VAR_PROPS] = (jsxProps as PropsProxy)[_VAR_PROPS];
-              vNodeProps[_OWNER] = (jsxProps as PropsProxy)[_OWNER];
-            } else if (jsxProps) {
-              // If there is no props instance, create a new one.
-              // We can do this because we are not using the props instance for anything else.
-              (host as VirtualVNode).setProp(ELEMENT_PROPS, jsxProps);
-              vNodeProps = jsxProps;
-            }
-          }
           // Assign the new QRL instance to the host.
           // Unfortunately it is created every time, something to fix in the optimizer.
           (host as VirtualVNode).setProp(OnRenderProp, componentQRL);
@@ -1456,15 +1435,68 @@ function getComponentHash(vNode: VNode | null, getObject: (id: string) => any):
  */
 function Projection() {}
 
-function propsDiffer(
+function handleProps(
+  host: VirtualVNode,
+  jsxProps: PropsProxy,
+  vNodeProps: PropsProxy | null,
+  container: ClientContainer
+): boolean {
+  let shouldRender = false;
+  let propsAreDifferent = false;
+  if (vNodeProps) {
+    const effects = vNodeProps[_PROPS_HANDLER].$effects$;
+    const constPropsDifferent = handleChangedProps(
+      jsxProps[_CONST_PROPS],
+      vNodeProps[_CONST_PROPS],
+      vNodeProps[_PROPS_HANDLER],
+      container,
+      false
+    );
+    propsAreDifferent = constPropsDifferent;
+    shouldRender ||= constPropsDifferent;
+    if (effects && effects.size > 0) {
+      const varPropsDifferent = handleChangedProps(
+        jsxProps[_VAR_PROPS],
+        vNodeProps[_VAR_PROPS],
+        vNodeProps[_PROPS_HANDLER],
+        container
+      );
+
+      propsAreDifferent ||= varPropsDifferent;
+      // don't mark as should render, effects will take care of it
+      // shouldRender ||= varPropsDifferent;
+    }
+  }
+
+  if (propsAreDifferent) {
+    if (vNodeProps) {
+      // Reuse the same props instance, qrls can use the current props instance
+      // as a capture ref, so we can't change it.
+      vNodeProps[_OWNER] = (jsxProps as PropsProxy)[_OWNER];
+    } else if (jsxProps) {
+      // If there is no props instance, create a new one.
+      // We can do this because we are not using the props instance for anything else.
+      (host as VirtualVNode).setProp(ELEMENT_PROPS, jsxProps);
+      vNodeProps = jsxProps;
+    }
+  }
+  return shouldRender;
+}
+
+function handleChangedProps(
   src: Record<string, any> | null | undefined,
-  dst: Record<string, any> | null | undefined
+  dst: Record<string, any> | null | undefined,
+  propsHandler: PropsProxyHandler,
+  container: ClientContainer,
+  triggerEffects: boolean = true
 ): boolean {
   const srcEmpty = isPropsEmpty(src);
   const dstEmpty = isPropsEmpty(dst);
+
   if (srcEmpty && dstEmpty) {
     return false;
   }
+
   if (srcEmpty || dstEmpty) {
     return true;
   }
@@ -1474,7 +1506,6 @@ function propsDiffer(
 
   let srcLen = srcKeys.length;
   let dstLen = dstKeys.length;
-
   if ('children' in src!) {
     srcLen--;
   }
@@ -1492,16 +1523,23 @@ function propsDiffer(
     return true;
   }
 
+  let changed = false;
+  propsHandler.$container$ = container;
   for (const key of srcKeys) {
     if (key === 'children' || key === QBackRefs) {
       continue;
     }
     if (!Object.prototype.hasOwnProperty.call(dst, key) || src![key] !== dst![key]) {
-      return true;
+      changed = true;
+      if (triggerEffects) {
+        triggerPropsProxyEffect(propsHandler, key);
+      } else {
+        return true;
+      }
     }
   }
 
-  return false;
+  return changed;
 }
 
 function isPropsEmpty(props: Record<string, any> | null | undefined): boolean {

packages/qwik/src/core/reactive-primitives/cleanup.ts

@@ -11,6 +11,8 @@ import {
   type EffectSubscription,
 } from './types';
 import { AsyncComputedSignalImpl } from './impl/async-computed-signal-impl';
+import { isPropsProxy, type PropsProxyHandler } from '../shared/jsx/props-proxy';
+import { _PROPS_HANDLER } from '../shared/utils/constants';
 
 /** Class for back reference to the EffectSubscription */
 export abstract class BackRef {
@@ -28,6 +30,7 @@ export function clearAllEffects(container: Container, consumer: Consumer): void
   for (const [, effect] of effects) {
     clearEffectSubscription(container, effect);
   }
+  effects.clear();
 }
 
 export function clearEffectSubscription(container: Container, effect: EffectSubscription) {
@@ -40,12 +43,16 @@ export function clearEffectSubscription(container: Container, effect: EffectSubs
       clearSignal(container, producer, effect);
     } else if (producer instanceof AsyncComputedSignalImpl) {
       clearAsyncComputedSignal(producer, effect);
+    } else if (isPropsProxy(producer)) {
+      const propsHandler = producer[_PROPS_HANDLER];
+      clearStoreOrProps(propsHandler, effect);
     } else if (container.$storeProxyMap$.has(producer)) {
       const target = container.$storeProxyMap$.get(producer)!;
       const storeHandler = getStoreHandler(target)!;
-      clearStore(storeHandler, effect);
+      clearStoreOrProps(storeHandler, effect);
     }
   }
+  backRefs.clear();
 }
 
 function clearSignal(container: Container, producer: SignalImpl, effect: EffectSubscription) {
@@ -74,14 +81,14 @@ function clearAsyncComputedSignal(
   }
 }
 
-function clearStore(producer: StoreHandler, effect: EffectSubscription) {
+function clearStoreOrProps(producer: StoreHandler | PropsProxyHandler, effect: EffectSubscription) {
   const effects = producer?.$effects$;
   if (effects) {
-    for (const [propKey, propEffects] of effects.entries()) {
+    for (const [prop, propEffects] of effects.entries()) {
       if (propEffects.has(effect)) {
         propEffects.delete(effect);
         if (propEffects.size === 0) {
-          effects.delete(propKey);
+          effects.delete(prop);
         }
       }
     }

packages/qwik/src/core/reactive-primitives/impl/store.ts

@@ -17,6 +17,7 @@ import {
   type StoreTarget,
 } from '../types';
 import { ChoreType } from '../../shared/util-chore-type';
+import type { PropsProxy, PropsProxyHandler } from '../../shared/jsx/props-proxy';
 
 const DEBUG = false;
 
@@ -257,9 +258,9 @@ export class StoreHandler implements ProxyHandler<StoreTarget> {
 }
 
 export function addStoreEffect(
-  target: StoreTarget,
+  target: StoreTarget | PropsProxy,
   prop: string | symbol,
-  store: StoreHandler,
+  store: StoreHandler | PropsProxyHandler,
   effectSubscription: EffectSubscription
 ) {
   const effectsMap = (store.$effects$ ||= new Map());
@@ -276,6 +277,7 @@ export function addStoreEffect(
   // to unsubscribe from. So we need to store the reference from the effect back
   // to this signal.
   ensureContainsBackRef(effectSubscription, target);
+  // TODO is this needed with the preloader?
   addQrlToSerializationCtx(effectSubscription, store.$container$);
 
   DEBUG && log('sub', pad('\n' + store.$effects$?.entries.toString(), '  '));

packages/qwik/src/core/reactive-primitives/internal-api.ts

@@ -1,6 +1,6 @@
 import { assertEqual } from '../shared/error/assert';
 import { isPropsProxy } from '../shared/jsx/props-proxy';
-import { _CONST_PROPS, _IMMUTABLE } from '../shared/utils/constants';
+import { _CONST_PROPS, _IMMUTABLE, _VAR_PROPS } from '../shared/utils/constants';
 import { isObject } from '../shared/utils/types';
 import { AsyncComputedSignalImpl } from './impl/async-computed-signal-impl';
 import type { SignalImpl } from './impl/signal-impl';
@@ -71,25 +71,37 @@ export const _wrapProp = <T extends object, P extends keyof T>(
   }
   if (isPropsProxy(obj)) {
     const constProps = obj[_CONST_PROPS];
+    const varProps = obj[_VAR_PROPS];
     if (constProps && prop in constProps) {
       // Const props don't need wrapping
       return constProps[prop as keyof typeof constProps] as WrappedProp<T, P>;
+    } else if (prop in varProps) {
+      const value = varProps[prop as keyof typeof varProps];
+      return wrapIfNotSignal(value as T[P], args);
     }
   } else {
     const target = getStoreTarget(obj);
     if (target) {
       const value = target[prop as P];
-      const wrappedValue = isSignal(value)
-        ? // If the value is already a signal, we don't need to wrap it again
-          value
-        : getWrapped(args);
-      return wrappedValue as WrappedProp<T, P>;
+      return wrapIfNotSignal(value, args);
     }
   }
   // the object is not reactive, so we can just return the value
   return obj[prop as P] as WrappedProp<T, P>;
 };
 
+const wrapIfNotSignal = <T extends object, P extends keyof T>(
+  value: T[P],
+  args: [T, P?]
+): WrappedProp<T, P> => {
+  return (
+    isSignal(value)
+      ? // If the value is already a signal, we don't need to wrap it again
+        value
+      : getWrapped(args)
+  ) as WrappedProp<T, P>;
+};
+
 /** @internal @deprecated v1 compat */
 export const _wrapSignal = <T extends object, P extends keyof T>(obj: T, prop: P) => {
   const r = _wrapProp(obj, prop);