From 7deeb87e055d3dc0b82814091ef6f9c772c31eba Mon Sep 17 00:00:00 2001 From: Varixo Date: Sun, 22 Sep 2024 20:07:24 +0200 Subject: [PATCH] fix(v2): only wrapped signal as a subscriber --- .../core/v2/shared/shared-serialization.ts | 10 +++------- packages/qwik/src/core/v2/signal/v2-signal.ts | 20 ++++++++++--------- .../qwik/src/core/v2/signal/v2-subscriber.ts | 6 +++--- 3 files changed, 17 insertions(+), 19 deletions(-) diff --git a/packages/qwik/src/core/v2/shared/shared-serialization.ts b/packages/qwik/src/core/v2/shared/shared-serialization.ts index 94cc744da01..5b36a2c4655 100644 --- 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; 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 index 45949d8b7cb..07873e449d3 100644 --- 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 extends Subscriber implements ISignal { +export class Signal implements ISignal { $untrackedValue$: T; /** Store a list of effects which are dependent on this signal. */ @@ -157,7 +157,6 @@ export class Signal extends Subscriber implements ISignal { $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 extends Subscriber implements ISignal { // 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,7 +482,7 @@ export class ComputedSignal extends Signal { } // TO DISCUSS: shouldn't this type of signal have the $dependencies$ array instead of EVERY type of signal? -export class WrappedSignal extends Signal { +export class WrappedSignal extends Signal implements Subscriber { $args$: any[]; $func$: (...args: any[]) => T; $funcStr$: string | null; @@ -489,6 +490,7 @@ export class WrappedSignal extends Signal { // 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 index 220d99221c8..5bca543126f 100644 --- 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).$effects$; + const effectSubscriptions = (subscriber as WrappedSignal).$effects$; if (!effectSubscriptions) { return false; }