diff --git a/.changeset/loud-dolphins-relate.md b/.changeset/loud-dolphins-relate.md new file mode 100644 index 00000000000..28521f3f36a --- /dev/null +++ b/.changeset/loud-dolphins-relate.md @@ -0,0 +1,5 @@ +--- +'@qwik.dev/core': patch +--- + +fix: finding context parent and sorting projections in the scheduler diff --git a/packages/qwik/src/core/client/dom-container.ts b/packages/qwik/src/core/client/dom-container.ts index c7c68548eb4..5265f90b391 100644 --- a/packages/qwik/src/core/client/dom-container.ts +++ b/packages/qwik/src/core/client/dom-container.ts @@ -251,14 +251,13 @@ export class DomContainer extends _SharedContainer implements IClientContainer { if (vnode_getProp(vNode, OnRenderProp, null) !== null) { return vNode as any as HostElement; } - // If virtual node, than it could be a slot so we need to read its parent. - const parent = vnode_getProp(vNode, QSlotParent, this.$vnodeLocate$); - if (parent) { - vNode = parent; - continue; - } + vNode = + vnode_getParent(vNode) || + // If virtual node, than it could be a slot so we need to read its parent. + vnode_getProp(vNode, QSlotParent, this.$vnodeLocate$); + } else { + vNode = vnode_getParent(vNode); } - vNode = vnode_getParent(vNode); } return null; } diff --git a/packages/qwik/src/core/client/vnode.ts b/packages/qwik/src/core/client/vnode.ts index 9cd37855f77..07d25c8da31 100644 --- a/packages/qwik/src/core/client/vnode.ts +++ b/packages/qwik/src/core/client/vnode.ts @@ -1889,10 +1889,14 @@ export const vnode_getType = (vnode: VNode): 1 | 3 | 11 => { const isElement = (node: any): node is Element => node && typeof node == 'object' && fastNodeType(node) === /** Node.ELEMENT_NODE* */ 1; -/// These global variables are used to avoid creating new arrays for each call to `vnode_getPathToClosestDomNode`. +/// These global variables are used to avoid creating new arrays for each call to `vnode_documentPosition`. const aPath: VNode[] = []; const bPath: VNode[] = []; -export const vnode_documentPosition = (a: VNode, b: VNode): -1 | 0 | 1 => { +export const vnode_documentPosition = ( + a: VNode, + b: VNode, + rootVNode: ElementVNode | null +): -1 | 0 | 1 => { if (a === b) { return 0; } @@ -1900,10 +1904,14 @@ export const vnode_documentPosition = (a: VNode, b: VNode): -1 | 0 | 1 => { let aDepth = -1; let bDepth = -1; while (a) { - a = (aPath[++aDepth] = a)[VNodeProps.parent]!; + const vNode = (aPath[++aDepth] = a); + a = (vNode[VNodeProps.parent] || + (rootVNode && vnode_getProp(a, QSlotParent, (id) => vnode_locate(rootVNode, id))))!; } while (b) { - b = (bPath[++bDepth] = b)[VNodeProps.parent]!; + const vNode = (bPath[++bDepth] = b); + b = (vNode[VNodeProps.parent] || + (rootVNode && vnode_getProp(b, QSlotParent, (id) => vnode_locate(rootVNode, id))))!; } while (aDepth >= 0 && bDepth >= 0) { @@ -1929,6 +1937,11 @@ export const vnode_documentPosition = (a: VNode, b: VNode): -1 | 0 | 1 => { return -1; } } while (cursor); + if (rootVNode && vnode_getProp(b, QSlotParent, (id) => vnode_locate(rootVNode, id))) { + // The "b" node is a projection, so we need to set it after "a" node, + // because the "a" node could be a context provider. + return -1; + } // The node is not in the list of siblings, that means it must be disconnected. return 1; } @@ -1967,12 +1980,9 @@ export const vnode_getProjectionParentComponent = ( vHost && (vnode_isVirtualVNode(vHost) ? vnode_getProp(vHost, OnRenderProp, null) === null : true) ) { - const qSlotParentProp = vnode_getProp(vHost, QSlotParent, null) as string | VNode | null; - const qSlotParent = - qSlotParentProp && - (typeof qSlotParentProp === 'string' - ? vnode_locate(rootVNode, qSlotParentProp) - : qSlotParentProp); + const qSlotParent = vnode_getProp(vHost, QSlotParent, (id) => + vnode_locate(rootVNode, id) + ); const vProjectionParent = vnode_isVirtualVNode(vHost) && qSlotParent; if (vProjectionParent) { // We found a projection, so we need to go up one more level. diff --git a/packages/qwik/src/core/client/vnode.unit.tsx b/packages/qwik/src/core/client/vnode.unit.tsx index f856a05efcc..6098a74d120 100644 --- a/packages/qwik/src/core/client/vnode.unit.tsx +++ b/packages/qwik/src/core/client/vnode.unit.tsx @@ -655,16 +655,16 @@ describe('vnode', () => { parent.innerHTML = ''; const b = vnode_getFirstChild(vParent) as ElementVNode; const i = vnode_getNextSibling(b) as ElementVNode; - expect(vnode_documentPosition(b, i)).toBe(-1); - expect(vnode_documentPosition(i, b)).toBe(1); + expect(vnode_documentPosition(b, i, null)).toBe(-1); + expect(vnode_documentPosition(i, b, null)).toBe(1); }); it('should compare two virtual vNodes', () => { parent.innerHTML = 'AB'; document.qVNodeData.set(parent, '{B}{B}'); const a = vnode_getFirstChild(vParent) as ElementVNode; const b = vnode_getNextSibling(a) as ElementVNode; - expect(vnode_documentPosition(a, b)).toBe(-1); - expect(vnode_documentPosition(b, a)).toBe(1); + expect(vnode_documentPosition(a, b, null)).toBe(-1); + expect(vnode_documentPosition(b, a, null)).toBe(1); }); it('should compare two virtual vNodes', () => { parent.innerHTML = 'AB'; @@ -672,8 +672,8 @@ describe('vnode', () => { const a = vnode_getFirstChild(vParent) as ElementVNode; const a2 = vnode_getFirstChild(a) as ElementVNode; const b = vnode_getNextSibling(a) as ElementVNode; - expect(vnode_documentPosition(a2, b)).toBe(-1); - expect(vnode_documentPosition(b, a2)).toBe(1); + expect(vnode_documentPosition(a2, b, null)).toBe(-1); + expect(vnode_documentPosition(b, a2, null)).toBe(1); }); }); }); diff --git a/packages/qwik/src/core/shared/scheduler.ts b/packages/qwik/src/core/shared/scheduler.ts index aec222b077a..f5f6777a802 100644 --- a/packages/qwik/src/core/shared/scheduler.ts +++ b/packages/qwik/src/core/shared/scheduler.ts @@ -264,7 +264,7 @@ export const createScheduler = ( }; chore.$promise$ = new Promise((resolve) => (chore.$resolve$ = resolve)); DEBUG && debugTrace('schedule', chore, currentChore, choreQueue); - chore = sortedInsert(choreQueue, chore); + chore = sortedInsert(choreQueue, chore, (container as DomContainer).rootVNode || null); if (!journalFlushScheduled && runLater) { // If we are not currently draining, we need to schedule a drain. journalFlushScheduled = true; @@ -274,7 +274,7 @@ export const createScheduler = ( if (runLater) { return chore.$promise$; } else { - return drainUpTo(chore); + return drainUpTo(chore, (container as DomContainer).rootVNode || null); } } @@ -283,7 +283,7 @@ export const createScheduler = ( * * @param runUptoChore */ - function drainUpTo(runUptoChore: Chore): ValueOrPromise { + function drainUpTo(runUptoChore: Chore, rootVNode: ElementVNode | null): ValueOrPromise { // If it already ran, it's not in the queue if (runUptoChore.$executed$) { return runUptoChore.$returnValue$; @@ -294,7 +294,7 @@ export const createScheduler = ( } while (choreQueue.length) { const nextChore = choreQueue.shift()!; - const order = choreComparator(nextChore, runUptoChore, false); + const order = choreComparator(nextChore, runUptoChore, rootVNode, false); if (order === null) { continue; } @@ -313,7 +313,7 @@ export const createScheduler = ( } const returnValue = executeChore(nextChore); if (isPromise(returnValue)) { - const promise = returnValue.then(() => drainUpTo(runUptoChore)); + const promise = returnValue.then(() => drainUpTo(runUptoChore, rootVNode)); return promise; } } @@ -466,9 +466,24 @@ function vNodeAlreadyDeleted(chore: Chore): boolean { * @returns A number indicating the relative order of the chores, or null if invalid. A negative * number means `a` runs before `b`. */ -function choreComparator(a: Chore, b: Chore, shouldThrowOnHostMismatch: true): number; -function choreComparator(a: Chore, b: Chore, shouldThrowOnHostMismatch: false): number | null; -function choreComparator(a: Chore, b: Chore, shouldThrowOnHostMismatch: boolean): number | null { +function choreComparator( + a: Chore, + b: Chore, + rootVNode: ElementVNode | null, + shouldThrowOnHostMismatch: true +): number; +function choreComparator( + a: Chore, + b: Chore, + rootVNode: ElementVNode | null, + shouldThrowOnHostMismatch: false +): number | null; +function choreComparator( + a: Chore, + b: Chore, + rootVNode: ElementVNode | null, + shouldThrowOnHostMismatch: boolean +): number | null { const macroTypeDiff = (a.$type$ & ChoreType.MACRO) - (b.$type$ & ChoreType.MACRO); if (macroTypeDiff !== 0) { return macroTypeDiff; @@ -483,7 +498,7 @@ function choreComparator(a: Chore, b: Chore, shouldThrowOnHostMismatch: boolean) if (aHost !== bHost && aHost !== null && bHost !== null) { if (vnode_isVNode(aHost) && vnode_isVNode(bHost)) { // we are running on the client. - const hostDiff = vnode_documentPosition(aHost, bHost); + const hostDiff = vnode_documentPosition(aHost, bHost, rootVNode); if (hostDiff !== 0) { return hostDiff; } @@ -530,7 +545,11 @@ function choreComparator(a: Chore, b: Chore, shouldThrowOnHostMismatch: boolean) return 0; } -function sortedFindIndex(sortedArray: Chore[], value: Chore): number { +function sortedFindIndex( + sortedArray: Chore[], + value: Chore, + rootVNode: ElementVNode | null +): number { /// We need to ensure that the `queue` is sorted by priority. /// 1. Find a place where to insert into. let bottom = 0; @@ -538,7 +557,7 @@ function sortedFindIndex(sortedArray: Chore[], value: Chore): number { while (bottom < top) { const middle = bottom + ((top - bottom) >> 1); const midChore = sortedArray[middle]; - const comp = choreComparator(value, midChore, true); + const comp = choreComparator(value, midChore, rootVNode, true); if (comp < 0) { top = middle; } else if (comp > 0) { @@ -551,10 +570,10 @@ function sortedFindIndex(sortedArray: Chore[], value: Chore): number { return ~bottom; } -function sortedInsert(sortedArray: Chore[], value: Chore): Chore { +function sortedInsert(sortedArray: Chore[], value: Chore, rootVNode: ElementVNode | null): Chore { /// We need to ensure that the `queue` is sorted by priority. /// 1. Find a place where to insert into. - const idx = sortedFindIndex(sortedArray, value); + const idx = sortedFindIndex(sortedArray, value, rootVNode); if (idx < 0) { /// 2. Insert the chore into the queue. sortedArray.splice(~idx, 0, value); diff --git a/packages/qwik/src/core/tests/use-context.spec.tsx b/packages/qwik/src/core/tests/use-context.spec.tsx index 1c61fed8e3f..04b98925f75 100644 --- a/packages/qwik/src/core/tests/use-context.spec.tsx +++ b/packages/qwik/src/core/tests/use-context.spec.tsx @@ -25,8 +25,10 @@ import { ssrRenderToDom, trigger, } from '@qwik.dev/core/testing'; -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import type { Signal } from '../signal/signal.public'; +import { ErrorProvider } from '../../testing/rendering.unit-util'; +import * as qError from '../shared/error/error'; const debug = false; //true; Error.stackTraceLimit = 100; @@ -99,6 +101,52 @@ describe.each([ ); }); + it('should find context parent from Slot inside Slot', async () => { + const qErrorSpy = vi.spyOn(qError, 'qError'); + const contextId = createContextId('contextId'); + + const ContextProducer = component$(() => { + const context = { + disabled: false, + }; + useContextProvider(contextId, context); + return ; + }); + + const ProducerParent = component$(() => { + return ( + + + + ); + }); + + const ContextConsumer = component$(() => { + useContext(contextId); + return ; + }); + + const Parent = component$(() => { + return ( + + + + ); + }); + + try { + await render( + + + , + { debug } + ); + expect(qErrorSpy).not.toHaveBeenCalled(); + } catch (e) { + expect(qErrorSpy).not.toHaveBeenCalled(); + } + }); + describe('regression', () => { it('#4038', async () => { interface IMyComponent {