Skip to content

Commit 2ec1a82

Browse files
VarixoVarixo
authored
Merge pull request #7350 from QwikDev/v2-prevent-reusing-deleted-projection
fix: reusing the same projection
2 parents a26598a + d82f44a commit 2ec1a82

File tree

14 files changed

+282
-68
lines changed

14 files changed

+282
-68
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

‎.changeset/tasty-penguins-ring.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 reusing projection if is marked as deleted

‎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: 25 additions & 5 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';
@@ -458,6 +459,8 @@ export const vnode_diff = (
458459
slotName,
459460
(id) => vnode_locate(container.rootVNode, id)
460461
);
462+
// if projection is marked as deleted then we need to create a new one
463+
vCurrent = vCurrent && vCurrent[VNodeProps.flags] & VNodeFlags.Deleted ? null : vCurrent;
461464
if (vCurrent == null) {
462465
vNewNode = vnode_newVirtual();
463466
// you may be tempted to add the projection into the current parent, but
@@ -505,6 +508,21 @@ export const vnode_diff = (
505508
// All is good.
506509
// console.log(' NOOP', String(vCurrent));
507510
} 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+
508526
// move from q:template to the target node
509527
vnode_insertBefore(
510528
journal,
@@ -566,8 +584,8 @@ export const vnode_diff = (
566584
while (vCurrent) {
567585
const toRemove = vCurrent;
568586
advanceToNextSibling();
569-
cleanup(container, toRemove);
570587
if (vParent === vnode_getParent(toRemove)) {
588+
cleanup(container, toRemove);
571589
// If we are diffing projection than the parent is not the parent of the node.
572590
// If that is the case we don't want to remove the node from the parent.
573591
vnode_remove(journal, vParent as ElementVNode | VirtualVNode, toRemove, true);
@@ -1290,6 +1308,7 @@ export function cleanup(container: ClientContainer, vNode: VNode) {
12901308
let vCursor: VNode | null = vNode;
12911309
// Depth first traversal
12921310
if (vnode_isTextVNode(vNode)) {
1311+
markVNodeAsDeleted(vCursor);
12931312
// Text nodes don't have subscriptions or children;
12941313
return;
12951314
}
@@ -1326,7 +1345,7 @@ export function cleanup(container: ClientContainer, vNode: VNode) {
13261345
const attrs = vnode_getProps(vCursor);
13271346
for (let i = 0; i < attrs.length; i = i + 2) {
13281347
const key = attrs[i] as string;
1329-
if (!isParentSlotProp(key) && isSlotProp(key)) {
1348+
if (isSlotProp(key)) {
13301349
const value = attrs[i + 1];
13311350
if (value) {
13321351
attrs[i + 1] = null; // prevent infinite loop
@@ -1346,8 +1365,7 @@ export function cleanup(container: ClientContainer, vNode: VNode) {
13461365
}
13471366
}
13481367

1349-
const isProjection =
1350-
type & VNodeFlags.Virtual && vnode_getProp(vCursor as VirtualVNode, QSlot, null) !== null;
1368+
const isProjection = vnode_isProjection(vCursor);
13511369
// Descend into children
13521370
if (!isProjection) {
13531371
// Only if it is not a projection
@@ -1368,6 +1386,8 @@ export function cleanup(container: ClientContainer, vNode: VNode) {
13681386
return;
13691387
}
13701388
}
1389+
} else if (type & VNodeFlags.Text) {
1390+
markVNodeAsDeleted(vCursor);
13711391
}
13721392
// Out of children
13731393
if (vCursor === vNode) {

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

Lines changed: 59 additions & 29 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;
@@ -774,53 +784,71 @@ export const vnode_journalToString = (journal: VNodeJournal): string => {
774784

775785
function stringify(...args: any[]) {
776786
lines.push(
777-
' ' +
778-
args
779-
.map((arg) => {
780-
if (typeof arg === 'string') {
781-
return arg;
782-
} else if (arg && isHtmlElement(arg)) {
783-
const html = arg.outerHTML;
784-
const idx = html.indexOf('>');
785-
return '\n ' + (idx > 0 ? html.substring(0, idx + 1) : html);
786-
} else if (arg && isText(arg)) {
787-
return JSON.stringify(arg.nodeValue);
788-
} else {
789-
return String(arg);
790-
}
791-
})
792-
.join(' ')
787+
args
788+
.map((arg) => {
789+
if (typeof arg === 'string') {
790+
return arg;
791+
} else if (arg && isHtmlElement(arg)) {
792+
const html = arg.outerHTML;
793+
const hasChildNodes = !!arg.firstElementChild;
794+
const idx = html.indexOf('>');
795+
const lastIdx = html.lastIndexOf('<');
796+
return idx > 0 && hasChildNodes
797+
? html.substring(0, idx + 1) + '...' + html.substring(lastIdx)
798+
: html;
799+
} else if (arg && isText(arg)) {
800+
return JSON.stringify(arg.nodeValue);
801+
} else {
802+
return String(arg);
803+
}
804+
})
805+
.join(' ')
793806
);
794807
}
795808

796809
while (idx < length) {
797810
const op = journal[idx++] as VNodeJournalOpCode;
798811
switch (op) {
799812
case VNodeJournalOpCode.SetText:
800-
stringify('SetText', journal[idx++], journal[idx++]);
813+
stringify('SetText');
814+
stringify(' ', journal[idx++]);
815+
stringify(' -->', journal[idx++]);
801816
break;
802817
case VNodeJournalOpCode.SetAttribute:
803-
stringify('SetAttribute', journal[idx++], journal[idx++], journal[idx++]);
818+
stringify('SetAttribute');
819+
stringify(' ', journal[idx++]);
820+
stringify(' key', journal[idx++]);
821+
stringify(' val', journal[idx++]);
804822
break;
805823
case VNodeJournalOpCode.HoistStyles:
806824
stringify('HoistStyles');
807825
break;
808-
case VNodeJournalOpCode.Remove:
809-
stringify('Remove', journal[idx++]);
826+
case VNodeJournalOpCode.Remove: {
827+
stringify('Remove');
828+
const parent = journal[idx++];
829+
stringify(' ', parent);
810830
let nodeToRemove: any;
811831
while (idx < length && typeof (nodeToRemove = journal[idx]) !== 'number') {
812-
stringify(' ', nodeToRemove);
832+
stringify(' -->', nodeToRemove);
813833
idx++;
814834
}
815835
break;
816-
case VNodeJournalOpCode.Insert:
817-
stringify('Insert', journal[idx++], journal[idx++]);
836+
}
837+
case VNodeJournalOpCode.Insert: {
838+
stringify('Insert');
839+
const parent = journal[idx++];
840+
const insertBefore = journal[idx++];
841+
stringify(' ', parent);
818842
let newChild: any;
819843
while (idx < length && typeof (newChild = journal[idx]) !== 'number') {
820-
stringify(' ', newChild);
844+
stringify(' -->', newChild);
821845
idx++;
822846
}
847+
if (insertBefore) {
848+
stringify(' ', insertBefore);
849+
}
823850
break;
851+
}
824852
}
825853
}
826854
lines.push('END JOURNAL');
@@ -969,6 +997,7 @@ export const vnode_insertBefore = (
969997
// : insertBefore;
970998
const domParentVNode = vnode_getDomParentVNode(parent);
971999
const parentNode = domParentVNode && domParentVNode[ElementVNodeProps.element];
1000+
9721001
if (parentNode) {
9731002
const domChildren = vnode_getDomChildrenWithCorrectNamespacesToInsert(
9741003
journal,
@@ -1037,6 +1066,7 @@ export const vnode_remove = (
10371066
if (vnode_isTextVNode(vToRemove)) {
10381067
vnode_ensureTextInflated(journal, vToRemove);
10391068
}
1069+
10401070
const vPrevious = vToRemove[VNodeProps.previousSibling];
10411071
const vNext = vToRemove[VNodeProps.nextSibling];
10421072
if (vPrevious) {
@@ -1774,8 +1804,6 @@ function materializeFromVNodeData(
17741804
isDev && vnode_setAttr(null, vParent, ELEMENT_ID, id);
17751805
} else if (peek() === VNodeDataChar.PROPS) {
17761806
vnode_setAttr(null, vParent, ELEMENT_PROPS, consumeValue());
1777-
} else if (peek() === VNodeDataChar.SLOT_REF) {
1778-
vnode_setAttr(null, vParent, QSlotRef, consumeValue());
17791807
} else if (peek() === VNodeDataChar.KEY) {
17801808
vnode_setAttr(null, vParent, ELEMENT_KEY, consumeValue());
17811809
} else if (peek() === VNodeDataChar.SEQ) {
@@ -1787,6 +1815,8 @@ function materializeFromVNodeData(
17871815
container = getDomContainer(element);
17881816
}
17891817
setEffectBackRefFromVNodeData(vParent, consumeValue(), container);
1818+
} else if (peek() === VNodeDataChar.SLOT_PARENT) {
1819+
vnode_setProp(vParent, QSlotParent, consumeValue());
17901820
} else if (peek() === VNodeDataChar.CONTEXT) {
17911821
vnode_setAttr(null, vParent, QCtxAttr, consumeValue());
17921822
} 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()`

0 commit comments

Comments
 (0)