fix(v2): only wrapped signal as a subscriber

QwikDev · Sep 22, 2024 · 7deeb87 · 7deeb87
1 parent cf77a1f
commit 7deeb87
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 19 deletions.
diff --git a/packages/qwik/src/core/v2/shared/shared-serialization.ts b/packages/qwik/src/core/v2/shared/shared-serialization.ts
@@ -723,7 +723,7 @@ export const createSerializationContext = (
                 discoveredValues.push(effect[EffectSubscriptionsProp.EFFECT]);
               }
             }
-            if (obj.$effectDependencies$) {
+            if (obj instanceof WrappedSignal && obj.$effectDependencies$) {
               discoveredValues.push(obj.$effectDependencies$);
             }
             // TODO(mhevery): should scan the QRLs???
@@ -908,16 +908,12 @@ function serialize(serializationContext: SerializationContext): void {
           SerializationConstant.ComputedSignal_CHAR +
             qrlToString(serializationContext, value.$computeQrl$) +
             ';' +
-            $addRoot$(value.$effectDependencies$) +
-            ';' +
             $addRoot$(value.$untrackedValue$) +
             serializeEffectSubs($addRoot$, value.$effects$)
         );
       } else {
         writeString(
           SerializationConstant.Signal_CHAR +
-            $addRoot$(value.$effectDependencies$) +
-            ';' +
             $addRoot$(value.$untrackedValue$) +
             serializeEffectSubs($addRoot$, value.$effects$)
         );
@@ -1116,13 +1112,13 @@ function deserializeSignal2(
         container.$getObjectById$(parseInt(fnParts[i]))
       );
     }
+    const dependencies = container.$getObjectById$(parts[idx++]) as Subscriber[] | null;
+    derivedSignal.$effectDependencies$ = dependencies;
   }
   if (readQrl) {
     const computedSignal = signal as ComputedSignal<any>;
     computedSignal.$computeQrl$ = inflateQRL(container, parseQRL(parts[idx++])) as fixMeAny;
   }
-  const dependencies = container.$getObjectById$(parts[idx++]) as Subscriber[] | null;
-  signal.$effectDependencies$ = dependencies;
   let signalValue = container.$getObjectById$(parts[idx++]);
   if (vnode_isVNode(signalValue)) {
     signalValue = vnode_getNode(signalValue);

diff --git a/packages/qwik/src/core/v2/signal/v2-signal.ts b/packages/qwik/src/core/v2/signal/v2-signal.ts
@@ -148,7 +148,7 @@ export const enum EffectProperty {
   VNODE = '.',
 }
 
-export class Signal<T = any> extends Subscriber implements ISignal<T> {
+export class Signal<T = any> implements ISignal<T> {
   $untrackedValue$: T;
 
   /** Store a list of effects which are dependent on this signal. */
@@ -157,7 +157,6 @@ export class Signal<T = any> extends Subscriber implements ISignal<T> {
   $container$: Container2 | null = null;
 
   constructor(container: Container2 | null, value: T) {
-    super();
     this.$container$ = container;
     this.$untrackedValue$ = value;
     DEBUG && log('new', this);
@@ -198,12 +197,14 @@ export class Signal<T = any> extends Subscriber implements ISignal<T> {
         // to unsubscribe from. So we need to store the reference from the effect back
         // to this signal.
         ensureContains(effectSubscriber, this);
-        // We need to add the subscriber to the effect so that we can clean it up later
-        ensureEffectContainsSubscriber(
-          effectSubscriber[EffectSubscriptionsProp.EFFECT],
-          this,
-          this.$container$
-        );
+        if (isSubscriber(this)) {
+          // We need to add the subscriber to the effect so that we can clean it up later
+          ensureEffectContainsSubscriber(
+            effectSubscriber[EffectSubscriptionsProp.EFFECT],
+            this,
+            this.$container$
+          );
+        }
         DEBUG && log('read->sub', pad('\n' + this.toString(), '  '));
       }
     }
@@ -481,14 +482,15 @@ export class ComputedSignal<T> extends Signal<T> {
 }
 
 // TO DISCUSS: shouldn't this type of signal have the $dependencies$ array instead of EVERY type of signal?
-export class WrappedSignal<T> extends Signal<T> {
+export class WrappedSignal<T> extends Signal<T> implements Subscriber {
   $args$: any[];
   $func$: (...args: any[]) => T;
   $funcStr$: string | null;
 
   // We need a separate flag to know when the computation needs running because
   // we need the old value to know if effects need running after computation
   $invalid$: boolean = true;
+  $effectDependencies$: Subscriber[] | null = null;
 
   constructor(
     container: Container2 | null,

diff --git a/packages/qwik/src/core/v2/signal/v2-subscriber.ts b/packages/qwik/src/core/v2/signal/v2-subscriber.ts
@@ -1,14 +1,14 @@
 import { QSubscribers } from '../../util/markers';
 import type { VNode } from '../client/types';
 import { vnode_getProp } from '../client/vnode';
-import { EffectSubscriptionsProp, isSignal, type Signal } from './v2-signal';
+import { EffectSubscriptionsProp, WrappedSignal, isSignal } from './v2-signal';
 
 export abstract class Subscriber {
   $effectDependencies$: Subscriber[] | null = null;
 }
 
 export function isSubscriber(value: unknown): value is Subscriber {
-  return value instanceof Subscriber;
+  return value instanceof Subscriber || value instanceof WrappedSignal;
 }
 
 export function clearVNodeEffectDependencies(value: VNode): void {
@@ -41,7 +41,7 @@ function clearEffects(subscriber: Subscriber, value: Subscriber | VNode): boolea
   if (!isSignal(subscriber)) {
     return false;
   }
-  const effectSubscriptions = (subscriber as Signal<unknown>).$effects$;
+  const effectSubscriptions = (subscriber as WrappedSignal<unknown>).$effects$;
   if (!effectSubscriptions) {
     return false;
   }