diff --git a/.changeset/crazy-cities-tap.md b/.changeset/crazy-cities-tap.md new file mode 100644 index 00000000000..ef9c95e02d6 --- /dev/null +++ b/.changeset/crazy-cities-tap.md @@ -0,0 +1,5 @@ +--- +'@qwik.dev/core': patch +--- + +fix: serialize correctly null or undefined value for signals diff --git a/packages/qwik/src/core/reactive-primitives/cleanup.ts b/packages/qwik/src/core/reactive-primitives/cleanup.ts index 4bff47584aa..b8a192e76ea 100644 --- a/packages/qwik/src/core/reactive-primitives/cleanup.ts +++ b/packages/qwik/src/core/reactive-primitives/cleanup.ts @@ -16,7 +16,7 @@ import { _PROPS_HANDLER } from '../shared/utils/constants'; /** Class for back reference to the EffectSubscription */ export abstract class BackRef { - [_EFFECT_BACK_REF]: Map | null = null; + [_EFFECT_BACK_REF]: Map | undefined = undefined; } export function clearAllEffects(container: Container, consumer: Consumer): void { @@ -62,7 +62,7 @@ function clearSignal(container: Container, producer: SignalImpl, effect: EffectS } if (producer instanceof WrappedSignalImpl) { - producer.$hostElement$ = null; + producer.$hostElement$ = undefined; clearAllEffects(container, producer); } } diff --git a/packages/qwik/src/core/reactive-primitives/impl/async-computed-signal-impl.ts b/packages/qwik/src/core/reactive-primitives/impl/async-computed-signal-impl.ts index ece8b4fa22c..46b6e694af3 100644 --- a/packages/qwik/src/core/reactive-primitives/impl/async-computed-signal-impl.ts +++ b/packages/qwik/src/core/reactive-primitives/impl/async-computed-signal-impl.ts @@ -36,15 +36,15 @@ export class AsyncComputedSignalImpl implements BackRef { $untrackedLoading$: boolean = false; - $untrackedError$: Error | null = null; + $untrackedError$: Error | undefined = undefined; - $loadingEffects$: null | Set = null; - $errorEffects$: null | Set = null; + $loadingEffects$: undefined | Set = undefined; + $errorEffects$: undefined | Set = undefined; $destroy$: NoSerialize<() => void> | null; $promiseValue$: T | typeof NEEDS_COMPUTATION = NEEDS_COMPUTATION; private $promise$: Promise | null = null; - [_EFFECT_BACK_REF]: Map | null = null; + [_EFFECT_BACK_REF]: Map | undefined = undefined; constructor( container: Container | null, @@ -71,7 +71,7 @@ export class AsyncComputedSignalImpl this.$untrackedLoading$ = value; this.$container$?.$scheduler$( ChoreType.RECOMPUTE_AND_SCHEDULE_EFFECTS, - null, + undefined, this, this.$loadingEffects$ ); @@ -83,7 +83,7 @@ export class AsyncComputedSignalImpl } /** The error that occurred when the signal was resolved. */ - get error(): Error | null { + get error(): Error | undefined { return setupSignalValueAccess( this, () => (this.$errorEffects$ ||= new Set()), @@ -91,12 +91,12 @@ export class AsyncComputedSignalImpl ); } - set untrackedError(value: Error | null) { + set untrackedError(value: Error | undefined) { if (value !== this.$untrackedError$) { this.$untrackedError$ = value; this.$container$?.$scheduler$( ChoreType.RECOMPUTE_AND_SCHEDULE_EFFECTS, - null, + undefined, this, this.$errorEffects$ ); @@ -136,7 +136,7 @@ export class AsyncComputedSignalImpl if (isPromise(untrackedValue)) { const isFirstComputation = this.$promiseValue$ === NEEDS_COMPUTATION; this.untrackedLoading = true; - this.untrackedError = null; + this.untrackedError = undefined; if (this.$promiseValue$ !== NEEDS_COMPUTATION) { // skip cleanup after resuming @@ -148,7 +148,7 @@ export class AsyncComputedSignalImpl DEBUG && log('Promise resolved', promiseValue); this.$promiseValue$ = promiseValue; this.untrackedLoading = false; - this.untrackedError = null; + this.untrackedError = undefined; if (this.setValue(promiseValue)) { DEBUG && log('Scheduling effects for subscribers', this.$effects$?.size); scheduleEffects(this.$container$, this, this.$effects$); diff --git a/packages/qwik/src/core/reactive-primitives/impl/computed-signal-impl.ts b/packages/qwik/src/core/reactive-primitives/impl/computed-signal-impl.ts index c79eb7e7075..cd29db54ad2 100644 --- a/packages/qwik/src/core/reactive-primitives/impl/computed-signal-impl.ts +++ b/packages/qwik/src/core/reactive-primitives/impl/computed-signal-impl.ts @@ -34,7 +34,7 @@ export class ComputedSignalImpl> */ $computeQrl$: S; $flags$: SignalFlags | SerializationSignalFlags; - [_EFFECT_BACK_REF]: Map | null = null; + [_EFFECT_BACK_REF]: Map | undefined = undefined; constructor( container: Container | null, @@ -55,7 +55,7 @@ export class ComputedSignalImpl> this.$flags$ |= SignalFlags.INVALID; this.$container$?.$scheduler$( ChoreType.RECOMPUTE_AND_SCHEDULE_EFFECTS, - null, + undefined, this, this.$effects$ ); diff --git a/packages/qwik/src/core/reactive-primitives/impl/signal-impl.ts b/packages/qwik/src/core/reactive-primitives/impl/signal-impl.ts index 6845433c9d1..3c59a0cb242 100644 --- a/packages/qwik/src/core/reactive-primitives/impl/signal-impl.ts +++ b/packages/qwik/src/core/reactive-primitives/impl/signal-impl.ts @@ -23,7 +23,7 @@ export class SignalImpl implements Signal { $untrackedValue$: T; /** Store a list of effects which are dependent on this signal. */ - $effects$: null | Set = null; + $effects$: undefined | Set = undefined; $container$: Container | null = null; $wrappedSignal$: WrappedSignalImpl | null = null; @@ -40,7 +40,7 @@ export class SignalImpl implements Signal { force() { this.$container$?.$scheduler$( ChoreType.RECOMPUTE_AND_SCHEDULE_EFFECTS, - null, + undefined, this, this.$effects$ ); @@ -70,7 +70,7 @@ export class SignalImpl implements Signal { this.$untrackedValue$ = value; this.$container$?.$scheduler$( ChoreType.RECOMPUTE_AND_SCHEDULE_EFFECTS, - null, + undefined, this, this.$effects$ ); diff --git a/packages/qwik/src/core/reactive-primitives/impl/store.ts b/packages/qwik/src/core/reactive-primitives/impl/store.ts index 7f9d4dc2ebe..391f3a9d933 100644 --- a/packages/qwik/src/core/reactive-primitives/impl/store.ts +++ b/packages/qwik/src/core/reactive-primitives/impl/store.ts @@ -96,7 +96,7 @@ export const getOrCreateStore = ( }; export class StoreHandler implements ProxyHandler { - $effects$: null | Map> = null; + $effects$: undefined | Map> = undefined; constructor( public $flags$: StoreFlags, @@ -111,7 +111,7 @@ export class StoreHandler implements ProxyHandler { const target = getStoreTarget(this)!; this.$container$?.$scheduler$( ChoreType.RECOMPUTE_AND_SCHEDULE_EFFECTS, - null, + undefined, this, getEffects(target, prop, this.$effects$) ); @@ -201,7 +201,7 @@ export class StoreHandler implements ProxyHandler { // Changing the length property will trigger effects. this.$container$?.$scheduler$( ChoreType.RECOMPUTE_AND_SCHEDULE_EFFECTS, - null, + undefined, this, getEffects(target, prop, this.$effects$) ); @@ -294,7 +294,7 @@ function setNewValueAndTriggerEffects>( if (effects) { currentStore.$container$?.$scheduler$( ChoreType.RECOMPUTE_AND_SCHEDULE_EFFECTS, - null, + undefined, currentStore, effects ); @@ -304,7 +304,7 @@ function setNewValueAndTriggerEffects>( function getEffects>( target: T, prop: string | symbol, - storeEffects: Map> | null + storeEffects: Map> | undefined ) { let effectsToTrigger: Set | undefined; @@ -328,5 +328,5 @@ function getEffects>( effectsToTrigger!.add(effect); } } - return effectsToTrigger || null; + return effectsToTrigger; } diff --git a/packages/qwik/src/core/reactive-primitives/impl/wrapped-signal-impl.ts b/packages/qwik/src/core/reactive-primitives/impl/wrapped-signal-impl.ts index fd744510585..164ffdc39fc 100644 --- a/packages/qwik/src/core/reactive-primitives/impl/wrapped-signal-impl.ts +++ b/packages/qwik/src/core/reactive-primitives/impl/wrapped-signal-impl.ts @@ -22,8 +22,8 @@ export class WrappedSignalImpl extends SignalImpl implements BackRef { $funcStr$: string | null; $flags$: AllSignalFlags; - $hostElement$: HostElement | null = null; - [_EFFECT_BACK_REF]: Map | null = null; + $hostElement$: HostElement | undefined = undefined; + [_EFFECT_BACK_REF]: Map | undefined = undefined; constructor( container: Container | null, diff --git a/packages/qwik/src/core/reactive-primitives/utils.ts b/packages/qwik/src/core/reactive-primitives/utils.ts index 8d57a61d323..1e90d186af7 100644 --- a/packages/qwik/src/core/reactive-primitives/utils.ts +++ b/packages/qwik/src/core/reactive-primitives/utils.ts @@ -88,7 +88,7 @@ export const addQrlToSerializationCtx = ( export const scheduleEffects = ( container: Container | null, signal: SignalImpl | StoreTarget, - effects: Set | null + effects: Set | undefined ) => { const isBrowser = !isServerPlatform(); if (effects) { diff --git a/packages/qwik/src/core/shared/jsx/props-proxy.ts b/packages/qwik/src/core/shared/jsx/props-proxy.ts index 623d2c19311..1b4e9f16948 100644 --- a/packages/qwik/src/core/shared/jsx/props-proxy.ts +++ b/packages/qwik/src/core/shared/jsx/props-proxy.ts @@ -17,7 +17,7 @@ export function createPropsProxy(owner: JSXNodeImpl): Props { return new Proxy({}, new PropsProxyHandler(owner)); } export class PropsProxyHandler implements ProxyHandler { - $effects$: null | Map> = null; + $effects$: undefined | Map> = undefined; $container$: Container | null = null; constructor(public owner: JSXNodeImpl) {} @@ -176,7 +176,7 @@ export const triggerPropsProxyEffect = (propsProxy: PropsProxyHandler, prop: str if (effects) { propsProxy.$container$?.$scheduler$( ChoreType.RECOMPUTE_AND_SCHEDULE_EFFECTS, - null, + undefined, propsProxy, effects ); @@ -184,11 +184,11 @@ export const triggerPropsProxyEffect = (propsProxy: PropsProxyHandler, prop: str }; function getEffects( - effects: Map> | null, + effects: Map> | undefined, prop: string | symbol ) { // TODO: Handle STORE_ALL_PROPS - return effects?.get(prop) || null; + return effects?.get(prop); } /** diff --git a/packages/qwik/src/core/shared/scheduler.ts b/packages/qwik/src/core/shared/scheduler.ts index c2a5fd916d2..eba15d5949d 100644 --- a/packages/qwik/src/core/shared/scheduler.ts +++ b/packages/qwik/src/core/shared/scheduler.ts @@ -221,9 +221,9 @@ export const createScheduler = ( */ function schedule( type: ChoreType.RECOMPUTE_AND_SCHEDULE_EFFECTS, - host: HostElement | null, + host: HostElement | undefined, target: Signal | StoreTarget, - effects: Set | null + effects: Set | undefined ): Chore; function schedule( type: ChoreType.TASK | ChoreType.VISIBLE, diff --git a/packages/qwik/src/core/shared/serdes/inflate.ts b/packages/qwik/src/core/shared/serdes/inflate.ts index a7b7a776d88..b2ecee9c596 100644 --- a/packages/qwik/src/core/shared/serdes/inflate.ts +++ b/packages/qwik/src/core/shared/serdes/inflate.ts @@ -89,7 +89,7 @@ export const inflate = ( task.$flags$ = v[1]; task.$index$ = v[2]; task.$el$ = v[3] as HostElement; - task[_EFFECT_BACK_REF] = v[4] as Map | null; + task[_EFFECT_BACK_REF] = v[4] as Map | undefined; task.$state$ = v[5]; break; case TypeIds.Resource: @@ -139,7 +139,7 @@ export const inflate = ( const d = data as [ number, unknown[], - Map | null, + Map | undefined, AllSignalFlags, HostElement, ...EffectSubscription[], @@ -160,9 +160,9 @@ export const inflate = ( const asyncComputed = target as AsyncComputedSignalImpl; const d = data as [ AsyncComputeQRL, - Array | null, - Array | null, - Array | null, + Array | undefined, + Array | undefined, + Array | undefined, boolean, Error, unknown?, @@ -172,7 +172,7 @@ export const inflate = ( asyncComputed.$loadingEffects$ = new Set(d[2]); asyncComputed.$errorEffects$ = new Set(d[3]); asyncComputed.$untrackedLoading$ = d[4]; - asyncComputed.$untrackedError$ = d[5] || null; + asyncComputed.$untrackedError$ = d[5]; const hasValue = d.length > 6; if (hasValue) { asyncComputed.$untrackedValue$ = d[6]; @@ -185,9 +185,11 @@ export const inflate = ( case TypeIds.SerializerSignal: case TypeIds.ComputedSignal: { const computed = target as ComputedSignalImpl; - const d = data as [QRLInternal<() => {}>, EffectSubscription[] | null, unknown?]; + const d = data as [QRLInternal<() => {}>, EffectSubscription[] | undefined, unknown?]; computed.$computeQrl$ = d[0]; - computed.$effects$ = new Set(d[1]); + if (d[1]) { + computed.$effects$ = new Set(d[1]); + } const hasValue = d.length > 2; if (hasValue) { computed.$untrackedValue$ = d[2]; @@ -277,7 +279,7 @@ export const inflate = ( JSXNodeImpl | typeof _UNINITIALIZED, Props, Props | null, - Map> | null, + Map> | undefined, ]; let owner = d[0]; if (owner === _UNINITIALIZED) { diff --git a/packages/qwik/src/core/shared/serdes/serdes.unit.ts b/packages/qwik/src/core/shared/serdes/serdes.unit.ts index b1562bff873..2dc45c78908 100644 --- a/packages/qwik/src/core/shared/serdes/serdes.unit.ts +++ b/packages/qwik/src/core/shared/serdes/serdes.unit.ts @@ -16,7 +16,11 @@ import { isSignal, } from '../../reactive-primitives/signal.public'; import { SubscriptionData } from '../../reactive-primitives/subscription-data'; -import { StoreFlags } from '../../reactive-primitives/types'; +import { + EffectProperty, + StoreFlags, + type EffectSubscription, +} from '../../reactive-primitives/types'; import { createResourceReturn } from '../../use/use-resource'; import { Task } from '../../use/use-task'; import { QError } from '../error/error'; @@ -350,7 +354,7 @@ describe('shared-serialization', () => { {number} 0 {number} 0 RootRef 1 - Constant null + Constant undefined Object [ {string} "shared" {number} 2 @@ -411,6 +415,62 @@ describe('shared-serialization', () => { ] (25 chars)" `); + + const objsNull = await serialize({ foo: createSignal(null) }); + expect(_dumpState(objsNull)).toMatchInlineSnapshot(` + " + 0 Object [ + {string} "foo" + Signal [ + Constant null + ] + ] + (22 chars)" + `); + + // undefined signal without effects + const undefinedSignal = createSignal(undefined); + const objsUndefined = await serialize({ foo: undefinedSignal }); + expect(_dumpState(objsUndefined)).toMatchInlineSnapshot(` + " + 0 Object [ + {string} "foo" + Signal [ + Constant undefined + ] + ] + (22 chars)" + `); + + // undefined signal with effects + const ctxSignal = createSignal('test'); + const effectSubscription: EffectSubscription = [ + ctxSignal as SignalImpl, + EffectProperty.COMPONENT, + null, + null, + ]; + (undefinedSignal as SignalImpl).$effects$ = new Set([effectSubscription]); + + const objsUndefinedWithEffects = await serialize({ foo: undefinedSignal }); + expect(_dumpState(objsUndefinedWithEffects)).toMatchInlineSnapshot(` + " + 0 Object [ + {string} "foo" + Signal [ + Constant undefined + Array [ + Signal [ + {string} "test" + ] + {string} ":" + Constant null + Constant null + ] + ] + ] + (54 chars)" + `); }); it(title(TypeIds.WrappedSignal), async () => { const foo = createSignal(3); @@ -429,7 +489,7 @@ describe('shared-serialization', () => { Array [ {number} 3 ] - Constant null + Constant undefined {number} 5 ] 1 WrappedSignal [ @@ -440,7 +500,7 @@ describe('shared-serialization', () => { ] {string} "value" ] - Constant null + Constant undefined {number} 7 ] (66 chars)" @@ -474,7 +534,7 @@ describe('shared-serialization', () => { ] 1 ComputedSignal [ RootRef 5 - Constant null + Constant undefined {number} 2 ] 2 ComputedSignal [ @@ -482,7 +542,7 @@ describe('shared-serialization', () => { ] 3 ComputedSignal [ RootRef 7 - Constant null + Constant undefined {number} 2 ] 4 PreloadQRL "9 10 8" @@ -525,7 +585,7 @@ describe('shared-serialization', () => { 3 {string} "custom_createSerializer_qrl" 4 SerializerSignal [ RootRef 1 - Constant null + Constant undefined {number} 4 ] 5 ForwardRefs [ @@ -585,32 +645,32 @@ describe('shared-serialization', () => { " 0 AsyncComputedSignal [ RootRef 4 - Constant null - Constant null - Constant null + Constant undefined + Constant undefined + Constant undefined Constant false ] 1 AsyncComputedSignal [ RootRef 5 - Constant null - Constant null - Constant null + Constant undefined + Constant undefined + Constant undefined Constant false ] 2 AsyncComputedSignal [ RootRef 6 - Constant null - Constant null - Constant null + Constant undefined + Constant undefined + Constant undefined Constant false ] 3 AsyncComputedSignal [ RootRef 7 - Constant null - Constant null - Constant null + Constant undefined + Constant undefined + Constant undefined Constant false - Constant null + Constant undefined {number} 2 ] 4 PreloadQRL "9 10 8" diff --git a/packages/qwik/src/core/shared/serdes/serialize.ts b/packages/qwik/src/core/shared/serdes/serialize.ts index 596d3368574..34fdd0a1e19 100644 --- a/packages/qwik/src/core/shared/serdes/serialize.ts +++ b/packages/qwik/src/core/shared/serdes/serialize.ts @@ -72,17 +72,17 @@ export async function serialize(serializationContext: SerializationContext): Pro /** Helper to output an array */ const outputArray = ( value: unknown[], - keepNulls: boolean, + keepUndefined: boolean, writeFn: (value: unknown, idx: number) => void ) => { $writer$.write('['); let separator = false; let length; - if (keepNulls) { + if (keepUndefined) { length = value.length; } else { length = value.length - 1; - while (length >= 0 && value[length] === null) { + while (length >= 0 && value[length] === undefined) { length--; } length++; @@ -99,7 +99,7 @@ export async function serialize(serializationContext: SerializationContext): Pro }; /** Output a type,value pair. If the value is an array, it calls writeValue on each item. */ - const output = (type: number, value: number | string | any[], keepNulls?: boolean) => { + const output = (type: number, value: number | string | any[], keepUndefined?: boolean) => { $writer$.write(`${type},`); if (typeof value === 'number') { $writer$.write(value.toString()); @@ -114,7 +114,7 @@ export async function serialize(serializationContext: SerializationContext): Pro } $writer$.write(lastIdx === 0 ? s : s.slice(lastIdx)); } else { - outputArray(value, keepNulls!, (valueItem, idx) => { + outputArray(value, !!keepUndefined, (valueItem, idx) => { writeValue(valueItem, idx); }); } @@ -341,7 +341,7 @@ export async function serialize(serializationContext: SerializationContext): Pro } const out = [storeTarget, flags, effects, ...innerStores]; - while (out[out.length - 1] == null) { + while (out[out.length - 1] === undefined) { out.pop(); } output(TypeIds.Store, out); @@ -350,7 +350,13 @@ export async function serialize(serializationContext: SerializationContext): Pro const result = value[SerializerSymbol](value); if (isPromise(result)) { const forwardRef = resolvePromise(result, $addRoot$, (resolved, resolvedValue) => { - return new PromiseResult(TypeIds.SerializerSignal, resolved, resolvedValue, null, null); + return new PromiseResult( + TypeIds.SerializerSignal, + resolved, + resolvedValue, + undefined, + undefined + ); }); output(TypeIds.ForwardRef, forwardRef); } else { @@ -434,12 +440,29 @@ export async function serialize(serializationContext: SerializationContext): Pro ); } + let keepUndefined = false; + if (v !== NEEDS_COMPUTATION) { out.push(v); + + if (!isAsync && v === undefined) { + /** + * If value is undefined, we need to keep it in the output. If we don't do that, later + * during resuming, the value will be set to symbol(invalid) with flag invalid, and + * thats is incorrect. + */ + keepUndefined = true; + } } - output(isAsync ? TypeIds.AsyncComputedSignal : TypeIds.ComputedSignal, out); + output(isAsync ? TypeIds.AsyncComputedSignal : TypeIds.ComputedSignal, out, keepUndefined); } else { - output(TypeIds.Signal, [value.$untrackedValue$, ...(value.$effects$ || [])]); + const v = value.$untrackedValue$; + const keepUndefined = v === undefined; + const out = [v]; + if (value.$effects$) { + out.push(...value.$effects$); + } + output(TypeIds.Signal, out, keepUndefined); } } else if (value instanceof URL) { output(TypeIds.URL, value.href); @@ -512,9 +535,9 @@ export async function serialize(serializationContext: SerializationContext): Pro value.varProps, value.constProps, value.children, - value.toSort || null, + value.toSort || undefined, ]; - while (out[out.length - 1] == null) { + while (out[out.length - 1] === undefined) { out.pop(); } output(TypeIds.JSXNode, out); @@ -527,7 +550,7 @@ export async function serialize(serializationContext: SerializationContext): Pro value[_EFFECT_BACK_REF], value.$state$, ]; - while (out[out.length - 1] == null) { + while (out[out.length - 1] === undefined) { out.pop(); } output(TypeIds.Task, out); @@ -658,8 +681,8 @@ export class PromiseResult { public $effects$: | Map> | Set - | null = null, - public $qrl$: QRLInternal | null = null + | undefined = undefined, + public $qrl$: QRLInternal | undefined = undefined ) {} } function getCustomSerializerPromise(signal: SerializerSignalImpl, value: any) { @@ -741,8 +764,8 @@ function serializeWrappingFn( return [syncFnId, value.$args$] as const; } -function filterEffectBackRefs(effectBackRef: Map | null) { - let effectBackRefToSerialize: Map | null = null; +function filterEffectBackRefs(effectBackRef: Map | undefined) { + let effectBackRefToSerialize: Map | undefined = undefined; if (effectBackRef) { for (const [effectProp, effect] of effectBackRef) { if (effect[EffectSubscriptionProp.BACK_REF]) { diff --git a/packages/qwik/src/core/ssr/ssr-render-jsx.ts b/packages/qwik/src/core/ssr/ssr-render-jsx.ts index 7f68d3a321e..96ea4eab518 100644 --- a/packages/qwik/src/core/ssr/ssr-render-jsx.ts +++ b/packages/qwik/src/core/ssr/ssr-render-jsx.ts @@ -113,7 +113,7 @@ function processJSXNode( } ) { // console.log('processJSXNode', value); - if (value === null || value === undefined) { + if (value == null) { ssr.textNode(''); } else if (typeof value === 'boolean') { ssr.textNode('');