Skip to content

Commit 556d962

Browse files
wmertenswmertens
authored
Merge pull request #7106 from QwikDev/v2-fix-array-of-refs
fix: serialization of an array of element refs
2 parents 39cf127 + f3cad07 commit 556d962

File tree

11 files changed

+125
-17
lines changed

11 files changed

+125
-17
lines changed

‎.changeset/calm-cycles-know.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@qwik.dev/core': patch
3+
---
4+
5+
fix: serialization of an array of refs

‎packages/docs/src/routes/api/qwik/api.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2214,4 +2214,4 @@
22142214
"mdFile": "core.withlocale.md"
22152215
}
22162216
]
2217-
}
2217+
}

‎packages/qwik/src/core/api.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -849,6 +849,10 @@ export abstract class _SharedContainer implements Container {
849849
nodeType: number;
850850
id: string;
851851
};
852+
} | null, DomRefConstructor: {
853+
new (...rest: any[]): {
854+
id: string;
855+
};
852856
} | null, symbolToChunkResolver: SymbolToChunkResolver, writer?: StreamWriter, prepVNodeData?: (vNode: any) => void): SerializationContext;
853857
// (undocumented)
854858
abstract setContext<T>(host: HostElement, context: ContextId<T>, value: T): void;

‎packages/qwik/src/core/shared/qrl/qrl.unit.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ describe('serialization', () => {
100100

101101
test('serialize qrls', () => {
102102
const serializationContext = createSerializationContext(
103+
null,
103104
null,
104105
() => '',
105106
() => '',

‎packages/qwik/src/core/shared/shared-container.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,12 +49,16 @@ export abstract class _SharedContainer implements Container {
4949
NodeConstructor: {
5050
new (...rest: any[]): { nodeType: number; id: string };
5151
} | null,
52+
DomRefConstructor: {
53+
new (...rest: any[]): { id: string };
54+
} | null,
5255
symbolToChunkResolver: SymbolToChunkResolver,
5356
writer?: StreamWriter,
5457
prepVNodeData?: (vNode: any) => void
5558
): SerializationContext {
5659
return createSerializationContext(
5760
NodeConstructor,
61+
DomRefConstructor,
5862
symbolToChunkResolver,
5963
this.getHostProp.bind(this),
6064
this.setHostProp.bind(this),

‎packages/qwik/src/core/shared/shared-serialization.ts

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -576,10 +576,9 @@ type SsrNode = {
576576
vnodeData: VNodeData;
577577
};
578578

579-
/** A ref to a DOM element */
580-
class DomVRef {
581-
constructor(public id: string) {}
582-
}
579+
type DomRef = {
580+
id: string;
581+
};
583582

584583
export interface SerializationContext {
585584
$serialize$: () => void;
@@ -627,6 +626,7 @@ export interface SerializationContext {
627626
$breakCircularDepsAndAwaitPromises$: () => ValueOrPromise<void>;
628627

629628
$isSsrNode$: (obj: unknown) => obj is SsrNode;
629+
$isDomRef$: (obj: unknown) => obj is DomRef;
630630

631631
$writer$: StreamWriter;
632632
$syncFns$: string[];
@@ -652,6 +652,10 @@ export const createSerializationContext = (
652652
NodeConstructor: {
653653
new (...rest: any[]): { nodeType: number; id: string };
654654
} | null,
655+
/** DomRef constructor, for instanceof checks. */
656+
DomRefConstructor: {
657+
new (...rest: any[]): { id: string };
658+
} | null,
655659
symbolToChunkResolver: SymbolToChunkResolver,
656660
getProp: (obj: any, prop: string) => any,
657661
setProp: (obj: any, prop: string, value: any) => void,
@@ -685,12 +689,16 @@ export const createSerializationContext = (
685689
const isSsrNode = (NodeConstructor ? (obj) => obj instanceof NodeConstructor : () => false) as (
686690
obj: unknown
687691
) => obj is SsrNode;
692+
const isDomRef = (
693+
DomRefConstructor ? (obj) => obj instanceof DomRefConstructor : () => false
694+
) as (obj: unknown) => obj is DomRef;
688695

689696
return {
690697
$serialize$(): void {
691698
serialize(this);
692699
},
693700
$isSsrNode$: isSsrNode,
701+
$isDomRef$: isDomRef,
694702
$symbolToChunkResolver$: symbolToChunkResolver,
695703
$wasSeen$,
696704
$roots$: roots,
@@ -831,6 +839,8 @@ export const createSerializationContext = (
831839
discoveredValues.push(obj.$el$, obj.$qrl$, obj.$state$, obj.$effectDependencies$);
832840
} else if (isSsrNode(obj)) {
833841
discoveredValues.push(obj.vnodeData);
842+
} else if (isDomRef(obj)) {
843+
discoveredValues.push(obj.id);
834844
} else if (isJSXNode(obj)) {
835845
discoveredValues.push(obj.type, obj.props, obj.constProps, obj.children);
836846
} else if (Array.isArray(obj)) {
@@ -903,7 +913,7 @@ const promiseResults = new WeakMap<Promise<any>, [boolean, unknown]>();
903913
* - Therefore root indexes need to be doubled to get the actual index.
904914
*/
905915
function serialize(serializationContext: SerializationContext): void {
906-
const { $writer$, $isSsrNode$, $setProp$, $storeProxyMap$ } = serializationContext;
916+
const { $writer$, $isSsrNode$, $isDomRef$, $setProp$, $storeProxyMap$ } = serializationContext;
907917
let depth = -1;
908918
// Skip the type for the roots output
909919
let writeType = false;
@@ -1102,23 +1112,19 @@ function serialize(serializationContext: SerializationContext): void {
11021112
// TODO if !out.length, output 0 and restore as {}
11031113
output(TypeIds.Object, out);
11041114
}
1105-
} else if (value instanceof DomVRef) {
1115+
} else if ($isDomRef$(value)) {
11061116
output(TypeIds.RefVNode, value.id);
11071117
} else if (value instanceof Signal) {
11081118
/**
11091119
* Special case: when a Signal value is an SSRNode, it always needs to be a DOM ref instead.
11101120
* It can never be meant to become a vNode, because vNodes are internal only.
11111121
*/
1112-
let v =
1122+
const v =
11131123
value instanceof ComputedSignal &&
11141124
(value.$invalid$ || fastSkipSerialize(value.$untrackedValue$))
11151125
? NEEDS_COMPUTATION
11161126
: value.$untrackedValue$;
1117-
if ($isSsrNode$(v)) {
1118-
// TODO maybe we don't need to store all vnode data if it's only a ref
1119-
serializationContext.$addRoot$(v);
1120-
v = new DomVRef(v.id);
1121-
}
1127+
11221128
if (value instanceof WrappedSignal) {
11231129
output(TypeIds.WrappedSignal, [
11241130
...serializeWrappingFn(serializationContext, value),
@@ -1333,6 +1339,7 @@ export function qrlToString(
13331339
*/
13341340
export async function _serialize(data: unknown[]): Promise<string> {
13351341
const serializationContext = createSerializationContext(
1342+
null,
13361343
null,
13371344
() => '',
13381345
() => '',

‎packages/qwik/src/core/shared/shared-serialization.unit.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -781,6 +781,7 @@ describe('shared-serialization', () => {
781781

782782
async function serialize(...roots: any[]): Promise<any[]> {
783783
const sCtx = createSerializationContext(
784+
null,
784785
null,
785786
() => '',
786787
() => '',

‎packages/qwik/src/core/shared/types.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,9 @@ export interface Container {
4747
NodeConstructor: {
4848
new (...rest: any[]): { nodeType: number; id: string };
4949
} | null,
50+
DomRefConstructor: {
51+
new (...rest: any[]): { id: string };
52+
} | null,
5053
symbolToChunkResolver: SymbolToChunkResolver,
5154
writer?: StreamWriter
5255
): SerializationContext;

‎packages/qwik/src/core/tests/ref.spec.tsx

Lines changed: 77 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
1-
import { Fragment as Component, component$, useSignal, useVisibleTask$ } from '@qwik.dev/core';
1+
import {
2+
Fragment as Component,
3+
component$,
4+
useSignal,
5+
useStore,
6+
useVisibleTask$,
7+
} from '@qwik.dev/core';
28
import { domRender, ssrRenderToDom, trigger } from '@qwik.dev/core/testing';
39
import { describe, expect, it } from 'vitest';
10+
import { isElement } from '../../testing/html';
411

512
const debug = false; //true;
613
Error.stackTraceLimit = 100;
@@ -53,4 +60,73 @@ describe.each([
5360
expect((global as any).logs[1]).toBeDefined();
5461
(global as any).logs = undefined;
5562
});
63+
64+
it('should serialize array of refs', async () => {
65+
(globalThis as any).element = [] as HTMLElement[];
66+
67+
const Parent = component$(() => {
68+
const childElements = useSignal<HTMLElement[]>([]);
69+
70+
useVisibleTask$(() => {
71+
(globalThis as any).element.push(childElements.value[0]);
72+
});
73+
74+
return <div ref={(element) => childElements.value.push(element)}></div>;
75+
});
76+
77+
const { document } = await render(<Parent />, { debug });
78+
79+
if (ssrRenderToDom === render) {
80+
await trigger(document.body, 'div', 'qvisible');
81+
}
82+
83+
expect(isElement((globalThis as any).element[0])).toBeTruthy();
84+
(globalThis as any).element = undefined;
85+
});
86+
87+
it('should serialize object of refs', async () => {
88+
(globalThis as any).element = [] as HTMLElement[];
89+
90+
const Parent = component$(() => {
91+
const childElements = useSignal<{ obj: HTMLElement[] }>({ obj: [] });
92+
93+
useVisibleTask$(() => {
94+
(globalThis as any).element.push(childElements.value.obj[0]);
95+
});
96+
97+
return <div ref={(element) => childElements.value.obj.push(element)}></div>;
98+
});
99+
100+
const { document } = await render(<Parent />, { debug });
101+
102+
if (ssrRenderToDom === render) {
103+
await trigger(document.body, 'div', 'qvisible');
104+
}
105+
106+
expect(isElement((globalThis as any).element[0])).toBeTruthy();
107+
(globalThis as any).element = undefined;
108+
});
109+
110+
it('should serialize refs inside store', async () => {
111+
(globalThis as any).element = [] as HTMLElement[];
112+
113+
const Parent = component$(() => {
114+
const childElements = useStore<{ obj: HTMLElement[] }>({ obj: [] });
115+
116+
useVisibleTask$(() => {
117+
(globalThis as any).element.push(childElements.obj[0]);
118+
});
119+
120+
return <div ref={(element) => childElements.obj.push(element)}></div>;
121+
});
122+
123+
const { document } = await render(<Parent />, { debug });
124+
125+
if (ssrRenderToDom === render) {
126+
await trigger(document.body, 'div', 'qvisible');
127+
}
128+
129+
expect(isElement((globalThis as any).element[0])).toBeTruthy();
130+
(globalThis as any).element = undefined;
131+
});
56132
});

‎packages/qwik/src/server/ssr-container.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ import {
6969
import { Q_FUNCS_PREFIX } from './ssr-render';
7070
import type { PrefetchResource, RenderOptions, RenderToStreamResult } from './types';
7171
import { createTimer } from './utils';
72-
import { SsrComponentFrame, SsrNode } from './ssr-node';
72+
import { DomRef, SsrComponentFrame, SsrNode } from './ssr-node';
7373
import {
7474
TagNesting,
7575
allowedContent,
@@ -225,6 +225,7 @@ class SSRContainer extends _SharedContainer implements ISSRContainer {
225225
};
226226
this.serializationCtx = this.serializationCtxFactory(
227227
SsrNode,
228+
DomRef,
228229
this.symbolToChunkResolver,
229230
opts.writer,
230231
(vNodeData: VNodeData) => this.addVNodeToSerializationRoots(vNodeData)
@@ -1125,10 +1126,10 @@ class SSRContainer extends _SharedContainer implements ISSRContainer {
11251126
if (key === 'ref') {
11261127
const lastNode = this.getLastNode();
11271128
if (isSignal(value)) {
1128-
value.value = lastNode;
1129+
value.value = new DomRef(lastNode.id);
11291130
continue;
11301131
} else if (typeof value === 'function') {
1131-
value(lastNode);
1132+
value(new DomRef(lastNode.id));
11321133
continue;
11331134
}
11341135
}

0 commit comments

Comments
 (0)