Skip to content

Commit 7d809e7

Browse files
VarixoVarixo
committed
fix: scheduling previously blocked chore
1 parent 0f76bec commit 7d809e7

File tree

6 files changed

+257
-23
lines changed

6 files changed

+257
-23
lines changed

‎.changeset/yummy-grapes-make.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: scheduling previously blocked chore

‎packages/qwik/src/core/client/chore-array.ts‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,6 @@ export class ChoreArray extends Array<Chore> {
3636
}
3737

3838
delete(value: Chore) {
39-
// const idx = this.sortedFindIndex(this, value);
40-
// if (idx >= 0) {
41-
// this.splice(idx, 1);
42-
// }
43-
// return idx;
4439
const idx = this.indexOf(value);
4540
if (idx >= 0) {
4641
this.splice(idx, 1);

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

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1668,23 +1668,18 @@ export const vnode_getProps = (vnode: ElementVNode | VirtualVNode): unknown[] =>
16681668
};
16691669

16701670
export const vnode_isDescendantOf = (vnode: VNode, ancestor: VNode): boolean => {
1671-
let parent: VNode | null = vnode_getParentOrProjectionParent(vnode);
1671+
let parent: VNode | null = vnode_getProjectionParentOrParent(vnode);
16721672
while (parent) {
16731673
if (parent === ancestor) {
16741674
return true;
16751675
}
1676-
parent = vnode_getParentOrProjectionParent(parent);
1676+
parent = vnode_getProjectionParentOrParent(parent);
16771677
}
16781678
return false;
16791679
};
16801680

1681-
export const vnode_getParentOrProjectionParent = (vnode: VNode): VNode | null => {
1682-
const parentProjection: VNode | null = vnode.slotParent;
1683-
if (parentProjection) {
1684-
// This is a projection, so we need to check the parent of the projection
1685-
return parentProjection;
1686-
}
1687-
return vnode.parent;
1681+
export const vnode_getProjectionParentOrParent = (vnode: VNode): VNode | null => {
1682+
return vnode.slotParent || vnode.parent;
16881683
};
16891684

16901685
export const vnode_getNode = (vnode: VNode | null): Element | Text | null => {

‎packages/qwik/src/core/shared/scheduler-rules.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import {
2-
vnode_getParentOrProjectionParent,
2+
vnode_getProjectionParentOrParent,
33
vnode_isDescendantOf,
44
vnode_isVNode,
55
} from '../client/vnode';
@@ -138,7 +138,7 @@ function findAncestorBlockingChore(chore: Chore, type: ChoreSetType): Chore | nu
138138
const isNormalQueue = type === ChoreSetType.CHORES;
139139
// Walk up the ancestor tree and check the map
140140
let current: VNode | null = host;
141-
current = vnode_getParentOrProjectionParent(current);
141+
current = vnode_getProjectionParentOrParent(current);
142142
while (current) {
143143
const blockingChores = isNormalQueue ? current.chores : current.blockedChores;
144144
if (blockingChores) {
@@ -148,7 +148,7 @@ function findAncestorBlockingChore(chore: Chore, type: ChoreSetType): Chore | nu
148148
}
149149
}
150150
}
151-
current = vnode_getParentOrProjectionParent(current);
151+
current = vnode_getProjectionParentOrParent(current);
152152
}
153153
return null;
154154
}

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

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -361,10 +361,7 @@ This is often caused by modifying a signal in an already rendered component duri
361361
return chore;
362362
}
363363
if (!isRunningChore(chore)) {
364-
const idx = choreQueue.add(chore);
365-
if (idx < 0 && vnode_isVNode(chore.$host$)) {
366-
(chore.$host$.chores ||= new ChoreArray()).add(chore);
367-
}
364+
addChore(chore, choreQueue);
368365
}
369366
DEBUG && debugTrace('schedule', chore, choreQueue, blockedChores);
370367

@@ -460,7 +457,8 @@ This is often caused by modifying a signal in an already rendered component duri
460457
if (vnode_isVNode(blockedChore.$host$)) {
461458
blockedChore.$host$.blockedChores?.delete(blockedChore);
462459
}
463-
choreQueue.add(blockedChore);
460+
addChore(blockedChore, choreQueue);
461+
DEBUG && debugTrace('schedule.UNBLOCKED', blockedChore, choreQueue, blockedChores);
464462
blockedChoresScheduled = true;
465463
}
466464
}
@@ -792,6 +790,13 @@ export function addBlockedChore(
792790
}
793791
}
794792

793+
export function addChore(chore: Chore, choreArray: ChoreArray) {
794+
const idx = choreArray.add(chore);
795+
if (idx < 0 && vnode_isVNode(chore.$host$)) {
796+
(chore.$host$.chores ||= new ChoreArray()).add(chore);
797+
}
798+
}
799+
795800
function choreTypeToName(type: ChoreType): string {
796801
return (
797802
(

‎packages/qwik/src/core/shared/scheduler.unit.tsx‎

Lines changed: 235 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {
1212
import { Task, TaskFlags } from '../use/use-task';
1313
import type { Props } from './jsx/jsx-runtime';
1414
import type { QRLInternal } from './qrl/qrl-class';
15-
import { createScheduler, type Chore } from './scheduler';
15+
import { addChore, createScheduler, type Chore } from './scheduler';
1616
import { ChoreType } from './util-chore-type';
1717
import type { HostElement } from './types';
1818
import { ELEMENT_SEQ, QContainerAttr } from './utils/markers';
@@ -410,6 +410,240 @@ describe('scheduler', () => {
410410
expect(nextTickSpy).toHaveBeenCalledTimes(2);
411411
});
412412
});
413+
414+
describe('addChore', () => {
415+
let choreArray: ChoreArray;
416+
let vHost: VirtualVNode;
417+
418+
beforeEach(() => {
419+
choreArray = new ChoreArray();
420+
vHost = vnode_newVirtual();
421+
vHost.setProp('q:id', 'testHost');
422+
});
423+
424+
it('should add a new chore to the choreArray', () => {
425+
const task = mockTask(vHost, { qrl: $(() => testLog.push('task1')) });
426+
const chore: Chore = {
427+
$type$: ChoreType.TASK,
428+
$idx$: task.$index$,
429+
$host$: vHost as HostElement,
430+
$target$: null,
431+
$payload$: task,
432+
$state$: 0,
433+
$blockedChores$: null,
434+
$startTime$: undefined,
435+
$endTime$: undefined,
436+
$resolve$: undefined,
437+
$reject$: undefined,
438+
$returnValue$: null!,
439+
};
440+
441+
addChore(chore, choreArray);
442+
443+
expect(choreArray.length).toBe(1);
444+
expect(choreArray[0]).toBe(chore);
445+
});
446+
447+
it('should add chore to vnode host.chores when idx is negative', () => {
448+
const task = mockTask(vHost, { qrl: $(() => testLog.push('task1')) });
449+
const chore: Chore = {
450+
$type$: ChoreType.TASK,
451+
$idx$: task.$index$,
452+
$host$: vHost as HostElement,
453+
$target$: null,
454+
$payload$: task,
455+
$state$: 0,
456+
$blockedChores$: null,
457+
$startTime$: undefined,
458+
$endTime$: undefined,
459+
$resolve$: undefined,
460+
$reject$: undefined,
461+
$returnValue$: null!,
462+
};
463+
464+
addChore(chore, choreArray);
465+
466+
expect(vHost.chores).toBeDefined();
467+
expect(vHost.chores!.length).toBe(1);
468+
expect(vHost.chores![0]).toBe(chore);
469+
});
470+
471+
it('should not add to vnode host.chores when chore already exists (idx >= 0)', () => {
472+
const task = mockTask(vHost, { qrl: $(() => testLog.push('task1')) });
473+
const chore1: Chore = {
474+
$type$: ChoreType.TASK,
475+
$idx$: task.$index$,
476+
$host$: vHost as HostElement,
477+
$target$: null,
478+
$payload$: task,
479+
$state$: 0,
480+
$blockedChores$: null,
481+
$startTime$: undefined,
482+
$endTime$: undefined,
483+
$resolve$: undefined,
484+
$reject$: undefined,
485+
$returnValue$: null!,
486+
};
487+
488+
const chore2: Chore = {
489+
$type$: ChoreType.TASK,
490+
$idx$: task.$index$,
491+
$host$: vHost as HostElement,
492+
$target$: null,
493+
$payload$: task,
494+
$state$: 0,
495+
$blockedChores$: null,
496+
$startTime$: undefined,
497+
$endTime$: undefined,
498+
$resolve$: undefined,
499+
$reject$: undefined,
500+
$returnValue$: null!,
501+
};
502+
503+
addChore(chore1, choreArray);
504+
addChore(chore2, choreArray);
505+
506+
// choreArray should only have 1 chore (the existing one with updated payload)
507+
expect(choreArray.length).toBe(1);
508+
// vHost.chores should only have 1 chore (from first add)
509+
expect(vHost.chores!.length).toBe(1);
510+
});
511+
512+
it('should not add to host.chores when host is not a VNode', () => {
513+
const nonVNodeHost = {} as HostElement;
514+
const task = mockTask(vHost, { qrl: $(() => testLog.push('task1')) });
515+
const chore: Chore = {
516+
$type$: ChoreType.TASK,
517+
$idx$: task.$index$,
518+
$host$: nonVNodeHost,
519+
$target$: null,
520+
$payload$: task,
521+
$state$: 0,
522+
$blockedChores$: null,
523+
$startTime$: undefined,
524+
$endTime$: undefined,
525+
$resolve$: undefined,
526+
$reject$: undefined,
527+
$returnValue$: null!,
528+
};
529+
530+
addChore(chore, choreArray);
531+
532+
expect(choreArray.length).toBe(1);
533+
expect((nonVNodeHost as any).chores).toBeUndefined();
534+
});
535+
536+
it('should maintain correct sort order when adding multiple chores', () => {
537+
const task1 = mockTask(vAHost, { qrl: $(() => testLog.push('a1')), index: 0 });
538+
const task2 = mockTask(vBHost1, { qrl: $(() => testLog.push('b1')), index: 0 });
539+
const task3 = mockTask(vAHost, { qrl: $(() => testLog.push('a2')), index: 1 });
540+
541+
const chore1: Chore = {
542+
$type$: ChoreType.TASK,
543+
$idx$: task1.$index$,
544+
$host$: vAHost as HostElement,
545+
$target$: null,
546+
$payload$: task1,
547+
$state$: 0,
548+
$blockedChores$: null,
549+
$startTime$: undefined,
550+
$endTime$: undefined,
551+
$resolve$: undefined,
552+
$reject$: undefined,
553+
$returnValue$: null!,
554+
};
555+
556+
const chore2: Chore = {
557+
$type$: ChoreType.TASK,
558+
$idx$: task2.$index$,
559+
$host$: vBHost1 as HostElement,
560+
$target$: null,
561+
$payload$: task2,
562+
$state$: 0,
563+
$blockedChores$: null,
564+
$startTime$: undefined,
565+
$endTime$: undefined,
566+
$resolve$: undefined,
567+
$reject$: undefined,
568+
$returnValue$: null!,
569+
};
570+
571+
const chore3: Chore = {
572+
$type$: ChoreType.TASK,
573+
$idx$: task3.$index$,
574+
$host$: vAHost as HostElement,
575+
$target$: null,
576+
$payload$: task3,
577+
$state$: 0,
578+
$blockedChores$: null,
579+
$startTime$: undefined,
580+
$endTime$: undefined,
581+
$resolve$: undefined,
582+
$reject$: undefined,
583+
$returnValue$: null!,
584+
};
585+
586+
// Add in non-sorted order
587+
addChore(chore2, choreArray);
588+
addChore(chore3, choreArray);
589+
addChore(chore1, choreArray);
590+
591+
expect(choreArray.length).toBe(3);
592+
// Should be sorted: vAHost tasks before vBHost1 tasks, and by index within same host
593+
expect(choreArray[0]).toBe(chore1); // vAHost, index 0
594+
expect(choreArray[1]).toBe(chore3); // vAHost, index 1
595+
expect(choreArray[2]).toBe(chore2); // vBHost1, index 0
596+
});
597+
598+
it('should add chores of different types in correct macro order', () => {
599+
const task1 = mockTask(vHost, { qrl: $(() => testLog.push('task')), index: 0 });
600+
const visibleTask = mockTask(vHost, {
601+
qrl: $(() => testLog.push('visible')),
602+
index: 0,
603+
visible: true,
604+
});
605+
606+
const taskChore: Chore = {
607+
$type$: ChoreType.TASK,
608+
$idx$: task1.$index$,
609+
$host$: vHost as HostElement,
610+
$target$: null,
611+
$payload$: task1,
612+
$state$: 0,
613+
$blockedChores$: null,
614+
$startTime$: undefined,
615+
$endTime$: undefined,
616+
$resolve$: undefined,
617+
$reject$: undefined,
618+
$returnValue$: null!,
619+
};
620+
621+
const visibleChore: Chore = {
622+
$type$: ChoreType.VISIBLE,
623+
$idx$: visibleTask.$index$,
624+
$host$: vHost as HostElement,
625+
$target$: null,
626+
$payload$: visibleTask,
627+
$state$: 0,
628+
$blockedChores$: null,
629+
$startTime$: undefined,
630+
$endTime$: undefined,
631+
$resolve$: undefined,
632+
$reject$: undefined,
633+
$returnValue$: null!,
634+
};
635+
636+
// Add visible task first
637+
addChore(visibleChore, choreArray);
638+
// Add regular task second
639+
addChore(taskChore, choreArray);
640+
641+
expect(choreArray.length).toBe(2);
642+
// TASK should come before VISIBLE (different macro order)
643+
expect(choreArray[0]).toBe(taskChore);
644+
expect(choreArray[1]).toBe(visibleChore);
645+
});
646+
});
413647
});
414648

415649
function mockTask(host: VNode, opts: { index?: number; qrl?: QRL; visible?: boolean }): Task {

0 commit comments

Comments
 (0)