Skip to content

Commit d82f44a

Browse files
VarixoVarixo
committed
fix: prevent infinity loop by inserting the same projection before itself
1 parent 7ef293a commit d82f44a

File tree

13 files changed

+165
-43
lines changed

13 files changed

+165
-43
lines changed

‎.changeset/five-shoes-deny.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: prevent infinity loop by inserting the same projection before itself

‎packages/qwik/src/core/client/dom-container.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import {
2929
Q_PROPS_SEPARATOR,
3030
USE_ON_LOCAL_SEQ_IDX,
3131
getQFuncs,
32+
QLocaleAttr,
3233
} from '../shared/utils/markers';
3334
import { isPromise } from '../shared/utils/promises';
3435
import { isSlotProp } from '../shared/utils/prop';
@@ -141,7 +142,7 @@ export class DomContainer extends _SharedContainer implements IClientContainer {
141142
() => this.scheduleRender(),
142143
() => vnode_applyJournal(this.$journal$),
143144
{},
144-
element.getAttribute('q:locale')!
145+
element.getAttribute(QLocaleAttr)!
145146
);
146147
this.qContainer = element.getAttribute(QContainerAttr)!;
147148
if (!this.qContainer) {

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

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ import {
7373
vnode_getType,
7474
vnode_insertBefore,
7575
vnode_isElementVNode,
76+
vnode_isProjection,
7677
vnode_isTextVNode,
7778
vnode_isVNode,
7879
vnode_isVirtualVNode,
@@ -94,7 +95,7 @@ import { getNewElementNamespaceData } from './vnode-namespace';
9495
import { WrappedSignal, EffectProperty, isSignal, SubscriptionData } from '../signal/signal';
9596
import type { Signal } from '../signal/signal.public';
9697
import { executeComponent } from '../shared/component-execution';
97-
import { isParentSlotProp, isSlotProp } from '../shared/utils/prop';
98+
import { isSlotProp } from '../shared/utils/prop';
9899
import { escapeHTML } from '../shared/utils/character-escaping';
99100
import { clearAllEffects } from '../signal/signal-cleanup';
100101
import { serializeAttribute } from '../shared/utils/styles';
@@ -507,6 +508,21 @@ export const vnode_diff = (
507508
// All is good.
508509
// console.log(' NOOP', String(vCurrent));
509510
} else {
511+
const parent = vnode_getParent(vProjectedNode);
512+
const isAlreadyProjected =
513+
!!parent && !(vnode_isElementVNode(parent) && vnode_getElementName(parent) === QTemplate);
514+
if (isAlreadyProjected && vParent !== parent) {
515+
/**
516+
* The node is already projected, but structure has been changed. In next steps we will
517+
* insert the vProjectedNode at the end. However we will find existing projection elements
518+
* (from already projected THE SAME projection as vProjectedNode!) during
519+
* vnode_insertBefore. We need to remove vnode from the vnode tree to avoid referencing it
520+
* to self and cause infinite loop. Don't remove it from DOM to avoid additional operations
521+
* and flickering.
522+
*/
523+
vnode_remove(journal, parent, vProjectedNode, false);
524+
}
525+
510526
// move from q:template to the target node
511527
vnode_insertBefore(
512528
journal,
@@ -1329,7 +1345,7 @@ export function cleanup(container: ClientContainer, vNode: VNode) {
13291345
const attrs = vnode_getProps(vCursor);
13301346
for (let i = 0; i < attrs.length; i = i + 2) {
13311347
const key = attrs[i] as string;
1332-
if (!isParentSlotProp(key) && isSlotProp(key)) {
1348+
if (isSlotProp(key)) {
13331349
const value = attrs[i + 1];
13341350
if (value) {
13351351
attrs[i + 1] = null; // prevent infinite loop
@@ -1349,8 +1365,7 @@ export function cleanup(container: ClientContainer, vNode: VNode) {
13491365
}
13501366
}
13511367

1352-
const isProjection =
1353-
type & VNodeFlags.Virtual && vnode_getProp(vCursor as VirtualVNode, QSlot, null) !== null;
1368+
const isProjection = vnode_isProjection(vCursor);
13541369
// Descend into children
13551370
if (!isProjection) {
13561371
// Only if it is not a projection

‎packages/qwik/src/core/client/vnode.ts

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,6 @@ import {
141141
QScopedStyle,
142142
QSlot,
143143
QSlotParent,
144-
QSlotRef,
145144
QStyle,
146145
QStylesAllSelector,
147146
Q_PROPS_SEPARATOR,
@@ -333,6 +332,15 @@ export const vnode_isVirtualVNode = (vNode: VNode): vNode is VirtualVNode => {
333332
return (flag & VNodeFlags.Virtual) === VNodeFlags.Virtual;
334333
};
335334

335+
export const vnode_isProjection = (vNode: VNode): vNode is VirtualVNode => {
336+
assertDefined(vNode, 'Missing vNode');
337+
const flag = (vNode as VNode)[VNodeProps.flags];
338+
return (
339+
(flag & VNodeFlags.Virtual) === VNodeFlags.Virtual &&
340+
vnode_getProp(vNode as VirtualVNode, QSlot, null) !== null
341+
);
342+
};
343+
336344
const ensureTextVNode = (vNode: VNode): TextVNode => {
337345
assertTrue(vnode_isTextVNode(vNode), 'Expecting TextVNode was: ' + vnode_getNodeTypeName(vNode));
338346
return vNode as TextVNode;
@@ -401,7 +409,7 @@ export const vnode_ensureElementInflated = (vnode: VNode) => {
401409
/** Walks the VNode tree and materialize it using `vnode_getFirstChild`. */
402410
export function vnode_walkVNode(
403411
vNode: VNode,
404-
callback?: (vNode: VNode, vParent: VNode | null) => void
412+
callback?: (vNode: VNode, vParent: VNode | null) => boolean | void
405413
): void {
406414
let vCursor: VNode | null = vNode;
407415
// Depth first traversal
@@ -411,7 +419,9 @@ export function vnode_walkVNode(
411419
}
412420
let vParent: VNode | null = null;
413421
do {
414-
callback?.(vCursor, vParent);
422+
if (callback?.(vCursor, vParent)) {
423+
return;
424+
}
415425
const vFirstChild = vnode_getFirstChild(vCursor);
416426
if (vFirstChild) {
417427
vCursor = vFirstChild;
@@ -1794,8 +1804,6 @@ function materializeFromVNodeData(
17941804
isDev && vnode_setAttr(null, vParent, ELEMENT_ID, id);
17951805
} else if (peek() === VNodeDataChar.PROPS) {
17961806
vnode_setAttr(null, vParent, ELEMENT_PROPS, consumeValue());
1797-
} else if (peek() === VNodeDataChar.SLOT_REF) {
1798-
vnode_setAttr(null, vParent, QSlotRef, consumeValue());
17991807
} else if (peek() === VNodeDataChar.KEY) {
18001808
vnode_setAttr(null, vParent, ELEMENT_KEY, consumeValue());
18011809
} else if (peek() === VNodeDataChar.SEQ) {
@@ -1807,6 +1815,8 @@ function materializeFromVNodeData(
18071815
container = getDomContainer(element);
18081816
}
18091817
setEffectBackRefFromVNodeData(vParent, consumeValue(), container);
1818+
} else if (peek() === VNodeDataChar.SLOT_PARENT) {
1819+
vnode_setProp(vParent, QSlotParent, consumeValue());
18101820
} else if (peek() === VNodeDataChar.CONTEXT) {
18111821
vnode_setAttr(null, vParent, QCtxAttr, consumeValue());
18121822
} else if (peek() === VNodeDataChar.OPEN) {

‎packages/qwik/src/core/client/vnode.unit.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -249,10 +249,10 @@ describe('vnode', () => {
249249
});
250250
it('should place attributes on Virtual', () => {
251251
parent.innerHTML = ``;
252-
document.qVNodeData.set(parent, '{?:sref_@:key_}');
252+
document.qVNodeData.set(parent, '{?:sparent_@:key_}');
253253
expect(vParent).toMatchVDOM(
254254
<test>
255-
<Fragment {...({ 'q:sref': ':sref_' } as any)} key=":key_" />
255+
<Fragment {...({ 'q:sparent': ':sparent_' } as any)} key=":key_" />
256256
</test>
257257
);
258258
});

‎packages/qwik/src/core/shared/utils/markers.ts

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,21 +3,12 @@ import { QContainerValue } from '../types';
33
/** State factory of the component. */
44
export const OnRenderProp = 'q:renderFn';
55

6-
/** Component style host prefix */
7-
export const ComponentStylesPrefixHost = '💎';
8-
96
/** Component style content prefix */
107
export const ComponentStylesPrefixContent = '⚡️';
118

12-
/** Prefix used to identify on listeners. */
13-
export const EventPrefix = 'on:';
14-
15-
/** Attribute used to mark that an event listener is attached. */
16-
export const EventAny = 'on:.';
179
/** `<some-element q:slot="...">` */
1810
export const QSlot = 'q:slot';
19-
export const QSlotParent = ':';
20-
export const QSlotRef = 'q:sref';
11+
export const QSlotParent = 'q:sparent';
2112
export const QSlotS = 'q:s';
2213
export const QStyle = 'q:style';
2314
export const QStyleSelector = 'style[q\\:style]';
@@ -26,7 +17,6 @@ export const QStylesAllSelector = QStyleSelector + ',' + QStyleSSelector;
2617
export const QScopedStyle = 'q:sstyle';
2718
export const QCtxAttr = 'q:ctx';
2819
export const QBackRefs = 'q:brefs';
29-
export const QManifestHash = 'q:manifest-hash';
3020
export const QFuncsPrefix = 'qFuncs_';
3121

3222
export const getQFuncs = (
@@ -49,7 +39,6 @@ export const QIgnore = 'q:ignore';
4939
export const QIgnoreEnd = '/' + QIgnore;
5040
export const QContainerAttr = 'q:container';
5141
export const QContainerAttrEnd = '/' + QContainerAttr;
52-
export const QShadowRoot = 'q:shadowroot';
5342

5443
export const QTemplate = 'q:template';
5544

@@ -72,7 +61,6 @@ export const RenderEvent = 'qRender';
7261
export const TaskEvent = 'qTask';
7362

7463
/** `<q:slot name="...">` */
75-
export const QSlotInertName = '\u0000';
7664
export const QDefaultSlot = '';
7765

7866
/**
@@ -88,10 +76,6 @@ export const ELEMENT_KEY = 'q:key';
8876
export const ELEMENT_PROPS = 'q:props';
8977
export const ELEMENT_SEQ = 'q:seq';
9078
export const ELEMENT_SEQ_IDX = 'q:seqIdx';
91-
export const ELEMENT_SELF_ID = -1;
92-
export const ELEMENT_ID_SELECTOR = '[q\\:id]';
93-
export const ELEMENT_ID_PREFIX = '#';
94-
export const INLINE_FN_PREFIX = '@';
9579
export const Q_PREFIX = 'q:';
9680

9781
/** Non serializable markers - always begins with `:` character */

‎packages/qwik/src/core/shared/utils/prop.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,11 @@
11
import { createPropsProxy, type Props, type PropsProxy } from '../jsx/jsx-runtime';
22
import { _CONST_PROPS, _VAR_PROPS } from './constants';
3-
import { NON_SERIALIZABLE_MARKER_PREFIX, QSlotParent } from './markers';
3+
import { NON_SERIALIZABLE_MARKER_PREFIX } from './markers';
44

55
export function isSlotProp(prop: string): boolean {
66
return !prop.startsWith('q:') && !prop.startsWith(NON_SERIALIZABLE_MARKER_PREFIX);
77
}
88

9-
export function isParentSlotProp(prop: string): boolean {
10-
return prop.startsWith(QSlotParent);
11-
}
12-
139
/** @internal */
1410
export const _restProps = (props: PropsProxy, omit: string[], target: Props = {}) => {
1511
let constPropsTarget: Props | null = null;

‎packages/qwik/src/core/shared/vnode-data-types.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,8 @@ export const VNodeDataChar = {
6161
ID_CHAR: /* ********* */ '=',
6262
PROPS: /* ************** */ 62, // `>` - `q:props' - Component Props
6363
PROPS_CHAR: /* ****** */ '>',
64-
SLOT_REF: /* *********** */ 63, // `?` - `q:sref` - Slot reference.
65-
SLOT_REF_CHAR: /* *** */ '?',
64+
SLOT_PARENT: /* ******** */ 63, // `?` - `q:sparent` - Slot parent.
65+
SLOT_PARENT_CHAR: /* */ '?',
6666
KEY: /* **************** */ 64, // `@` - `q:key` - Element key.
6767
KEY_CHAR: /* ******** */ '@',
6868
SEQ: /* **************** */ 91, // `[` - `q:seq' - Seq value from `useSequentialScope()`

‎packages/qwik/src/core/ssr/ssr-render-jsx.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {
2525
QDefaultSlot,
2626
QScopedStyle,
2727
QSlot,
28+
QSlotParent,
2829
qwikInspectorAttr,
2930
} from '../shared/utils/markers';
3031
import { isPromise } from '../shared/utils/promises';
@@ -192,7 +193,7 @@ function processJSXNode(
192193
if (componentFrame) {
193194
const compId = componentFrame.componentNode.id || '';
194195
const projectionAttrs = isDev ? [DEBUG_TYPE, VirtualType.Projection] : [];
195-
projectionAttrs.push(':', compId);
196+
projectionAttrs.push(QSlotParent, compId);
196197
ssr.openProjection(projectionAttrs);
197198
const host = componentFrame.componentNode;
198199
const node = ssr.getLastNode();

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

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1189,6 +1189,115 @@ describe.each([
11891189
);
11901190
});
11911191

1192+
it('should render the same content projection with different structure', async () => {
1193+
const Cmp1 = component$(() => {
1194+
return (
1195+
<>
1196+
<h1>Test</h1>
1197+
<p>Test content</p>
1198+
</>
1199+
);
1200+
});
1201+
1202+
const Cmp2 = component$((props: { toggle: boolean }) => {
1203+
return (
1204+
<>
1205+
{props.toggle && <Slot />}
1206+
{!props.toggle && (
1207+
<>
1208+
<Slot />
1209+
</>
1210+
)}
1211+
</>
1212+
);
1213+
});
1214+
1215+
const Parent = component$(() => {
1216+
const toggle = useSignal(true);
1217+
return (
1218+
<div>
1219+
<button onClick$={() => (toggle.value = !toggle.value)}>toggle</button>
1220+
<Cmp2 toggle={toggle.value}>
1221+
<Cmp1 />
1222+
</Cmp2>
1223+
</div>
1224+
);
1225+
});
1226+
const { vNode, document } = await render(<Parent />, { debug: DEBUG });
1227+
expect(cleanupAttrs(document.body.querySelector('div')?.outerHTML)).toEqual(
1228+
'<div><button>toggle</button><h1>Test</h1><p>Test content</p></div>'
1229+
);
1230+
expect(vNode).toMatchVDOM(
1231+
<Component ssr-required>
1232+
<div>
1233+
<button>toggle</button>
1234+
<Component ssr-required>
1235+
<Fragment ssr-required>
1236+
<Projection ssr-required>
1237+
<Component ssr-required>
1238+
<Fragment ssr-required>
1239+
<h1>Test</h1>
1240+
<p>Test content</p>
1241+
</Fragment>
1242+
</Component>
1243+
</Projection>
1244+
</Fragment>
1245+
</Component>
1246+
</div>
1247+
</Component>
1248+
);
1249+
1250+
await trigger(document.body, 'button', 'click');
1251+
expect(cleanupAttrs(document.body.querySelector('div')?.outerHTML)).toEqual(
1252+
'<div><button>toggle</button><h1>Test</h1><p>Test content</p></div>'
1253+
);
1254+
1255+
expect(vNode).toMatchVDOM(
1256+
<Component ssr-required>
1257+
<div>
1258+
<button>toggle</button>
1259+
<Component ssr-required>
1260+
<Fragment ssr-required>
1261+
<Projection ssr-required>
1262+
<Component ssr-required>
1263+
<Fragment ssr-required>
1264+
<Fragment ssr-required>
1265+
<h1>Test</h1>
1266+
<p>Test content</p>
1267+
</Fragment>
1268+
</Fragment>
1269+
</Component>
1270+
</Projection>
1271+
</Fragment>
1272+
</Component>
1273+
</div>
1274+
</Component>
1275+
);
1276+
await trigger(document.body, 'button', 'click');
1277+
expect(cleanupAttrs(document.body.querySelector('div')?.outerHTML)).toEqual(
1278+
'<div><button>toggle</button><h1>Test</h1><p>Test content</p></div>'
1279+
);
1280+
expect(vNode).toMatchVDOM(
1281+
<Component ssr-required>
1282+
<div>
1283+
<button>toggle</button>
1284+
<Component ssr-required>
1285+
<Fragment ssr-required>
1286+
<Projection ssr-required>
1287+
<Component ssr-required>
1288+
<Fragment ssr-required>
1289+
<h1>Test</h1>
1290+
<p>Test content</p>
1291+
</Fragment>
1292+
</Component>
1293+
</Projection>
1294+
</Fragment>
1295+
</Component>
1296+
</div>
1297+
</Component>
1298+
);
1299+
});
1300+
11921301
describe('ensureProjectionResolved', () => {
11931302
(globalThis as any).log = [] as string[];
11941303
beforeEach(() => {

0 commit comments

Comments
 (0)