Skip to content

Commit a052f8f

Browse files
wmertenswmertens
committed
fix(core): schedule qrls instead of direct call
1 parent 770ddb2 commit a052f8f

File tree

13 files changed

+124
-35
lines changed

13 files changed

+124
-35
lines changed

‎.changeset/heavy-radios-dream.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: QRLs are now scheduled instead of directly executed by qwik-loader, so that they are executed in the right order.

‎packages/qwik/handlers.mjs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,4 @@
66
*
77
* Make sure that these handlers are listed in manifest.ts
88
*/
9-
export { _task } from '@qwik.dev/core';
9+
export { _run, _task } from '@qwik.dev/core';

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -805,6 +805,9 @@ export type ResourceReturn<T> = ResourcePending<T> | ResourceResolved<T> | Resou
805805
// @internal (undocumented)
806806
export const _restProps: (props: Record<string, any>, omit: string[], target?: {}) => {};
807807

808+
// @internal
809+
export const _run: (...args: unknown[]) => ValueOrPromise<void>;
810+
808811
// @internal
809812
export function _serialize(data: unknown[]): Promise<string>;
810813

‎packages/qwik/src/core/client/queue-qrl.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import { QError, qError } from '../shared/error/error';
2+
import type { QRLInternal } from '../shared/qrl/qrl-class';
3+
import { ChoreType } from '../shared/scheduler';
4+
import { getInvokeContext } from '../use/use-core';
5+
import { useLexicalScope } from '../use/use-lexical-scope.public';
6+
import { _getQContainerElement, getDomContainer } from './dom-container';
7+
8+
/**
9+
* This is called by qwik-loader to schedule a QRL. It has to be synchronous.
10+
*
11+
* @internal
12+
*/
13+
export const queueQRL = (...args: unknown[]) => {
14+
// This will already check container
15+
const [runQrl] = useLexicalScope<[QRLInternal<(...args: unknown[]) => unknown>]>();
16+
const context = getInvokeContext();
17+
const el = context.$element$!;
18+
const containerElement = _getQContainerElement(el) as HTMLElement;
19+
const container = getDomContainer(containerElement);
20+
21+
const scheduler = container.$scheduler$;
22+
if (!scheduler) {
23+
throw qError(QError.schedulerNotFound);
24+
}
25+
26+
return scheduler(ChoreType.RUN_QRL, null, runQrl, args);
27+
};

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -764,7 +764,12 @@ export const vnode_diff = (
764764
let returnValue = false;
765765
qrls.flat(2).forEach((qrl) => {
766766
if (qrl) {
767-
const value = qrl(event, element) as any;
767+
const value = container.$scheduler$(
768+
ChoreType.RUN_QRL,
769+
null,
770+
qrl as QRLInternal<(...args: unknown[]) => unknown>,
771+
[event, element]
772+
) as unknown;
768773
returnValue = returnValue || value === true;
769774
}
770775
});

‎packages/qwik/src/core/internal.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
export { _noopQrl, _noopQrlDEV, _regSymbol } from './shared/qrl/qrl';
22
export { _walkJSX } from './ssr/ssr-render-jsx';
33
export { _SharedContainer } from './shared/shared-container';
4+
export { queueQRL as _run } from './client/queue-qrl';
45
export { scheduleTask as _task } from './use/use-task';
56
export { _wrapSignal, _wrapProp } from './signal/signal-utils';
67
export { _restProps } from './shared/utils/prop';

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ export const codeToText = (code: number, ...parts: any[]): string => {
66
// Keep one error, one line to make it easier to search for the error message.
77
const MAP = [
88
'Error while serializing class or style attributes', // 0
9-
'', // 1 unused
9+
'Scheduler not found', // 1
1010
'', // 2 unused
1111
'Only primitive and object literals can be serialized. {{0}}', // 3
1212
'', // 4 unused
@@ -76,7 +76,7 @@ export const codeToText = (code: number, ...parts: any[]): string => {
7676

7777
export const enum QError {
7878
stringifyClassOrStyle = 0,
79-
UNUSED_1 = 1,
79+
schedulerNotFound = 1,
8080
UNUSED_2 = 2,
8181
verifySerializable = 3,
8282
UNUSED_4 = 4,

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

Lines changed: 50 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -122,18 +122,24 @@ export const enum ChoreType {
122122
/* order of elements (not encoded here) */
123123
MICRO /* **************************** */ = 0b0000_1111,
124124

125-
/** Ensure tha the QRL promise is resolved before processing next chores in the queue */
125+
/** Ensure that the QRL promise is resolved before processing next chores in the queue */
126126
QRL_RESOLVE /* ********************** */ = 0b0000_0001,
127-
RESOURCE /* ************************* */ = 0b0000_0010,
128-
TASK /* ***************************** */ = 0b0000_0011,
129-
NODE_DIFF /* ************************ */ = 0b0000_0100,
130-
NODE_PROP /* ************************ */ = 0b0000_0101,
131-
COMPONENT_SSR /* ******************** */ = 0b0000_0110,
132-
COMPONENT /* ************************ */ = 0b0000_0111,
133-
RECOMPUTE_AND_SCHEDULE_EFFECTS /* *** */ = 0b0000_1000,
127+
RUN_QRL,
128+
RESOURCE,
129+
TASK,
130+
NODE_DIFF,
131+
NODE_PROP,
132+
COMPONENT_SSR,
133+
COMPONENT,
134+
RECOMPUTE_AND_SCHEDULE_EFFECTS,
135+
136+
// Next macro level
134137
JOURNAL_FLUSH /* ******************** */ = 0b0001_0000,
138+
// Next macro level
135139
VISIBLE /* ************************** */ = 0b0010_0000,
140+
// Next macro level
136141
CLEANUP_VISIBLE /* ****************** */ = 0b0011_0000,
142+
// Next macro level
137143
WAIT_FOR_ALL /* ********************* */ = 0b1111_1111,
138144
}
139145

@@ -202,6 +208,12 @@ export const createScheduler = (
202208
type: ChoreType.TASK | ChoreType.VISIBLE | ChoreType.RESOURCE,
203209
task: Task
204210
): ValueOrPromise<void>;
211+
function schedule(
212+
type: ChoreType.RUN_QRL,
213+
ignore: null,
214+
target: QRLInternal<(...args: unknown[]) => unknown>,
215+
args: unknown[]
216+
): ValueOrPromise<void>;
205217
function schedule(
206218
type: ChoreType.COMPONENT,
207219
host: HostElement,
@@ -234,7 +246,10 @@ export const createScheduler = (
234246
targetOrQrl: ChoreTarget | string | null = null,
235247
payload: any = null
236248
): ValueOrPromise<any> {
237-
const runLater: boolean = type !== ChoreType.WAIT_FOR_ALL && type !== ChoreType.COMPONENT_SSR;
249+
const runLater: boolean =
250+
type !== ChoreType.WAIT_FOR_ALL &&
251+
type !== ChoreType.COMPONENT_SSR &&
252+
type !== ChoreType.RUN_QRL;
238253
const isTask =
239254
type === ChoreType.TASK ||
240255
type === ChoreType.VISIBLE ||
@@ -289,15 +304,19 @@ export const createScheduler = (
289304
return runUptoChore.$promise$;
290305
}
291306
while (choreQueue.length) {
292-
const nextChore = choreQueue.shift()!;
307+
const nextChore = choreQueue[0];
293308
const order = choreComparator(nextChore, runUptoChore, rootVNode);
294-
if (order === null) {
295-
continue;
296-
}
297-
if (order > 0) {
309+
if (order !== null && order > 0) {
298310
// we have processed all of the chores up to and including the given chore.
299311
break;
300312
}
313+
// We can take the chore out of the queue
314+
choreQueue.shift();
315+
if (order === null) {
316+
// There was an error with the chore.
317+
DEBUG && debugTrace('skip chore', nextChore, currentChore, choreQueue);
318+
continue;
319+
}
301320
const isDeletedVNode = vNodeAlreadyDeleted(nextChore);
302321
if (
303322
isDeletedVNode &&
@@ -364,6 +383,12 @@ export const createScheduler = (
364383
const result = runResource(chore.$payload$ as ResourceDescriptor<TaskFn>, container, host);
365384
returnValue = isDomContainer(container) ? null : result;
366385
break;
386+
case ChoreType.RUN_QRL:
387+
{
388+
const fn = (chore.$target$ as QRLInternal<(...args: unknown[]) => unknown>).getFn();
389+
returnValue = fn(...(chore.$payload$ as unknown[]));
390+
}
391+
break;
367392
case ChoreType.TASK:
368393
returnValue = runTask(chore.$payload$ as Task<TaskFn, TaskFn>, container, host);
369394
break;
@@ -423,11 +448,11 @@ export const createScheduler = (
423448
}
424449
}
425450
return maybeThenPassError(returnValue, (value) => {
426-
DEBUG && debugTrace('execute.DONE', null, currentChore, choreQueue);
427451
if (currentChore) {
428452
currentChore.$executed$ = true;
429453
currentChore.$resolve$?.(value);
430454
}
455+
DEBUG && debugTrace('execute.DONE', null, currentChore, choreQueue);
431456
currentChore = null;
432457
return (chore.$returnValue$ = value);
433458
});
@@ -476,7 +501,6 @@ function choreComparator(a: Chore, b: Chore, rootVNode: ElementVNode | null): nu
476501
const aHost = a.$host$;
477502
const bHost = b.$host$;
478503

479-
// QRL_RESOLVE does not have a host.
480504
if (aHost !== bHost && aHost !== null && bHost !== null) {
481505
if (vnode_isVNode(aHost) && vnode_isVNode(bHost)) {
482506
// we are running on the client.
@@ -511,13 +535,13 @@ function choreComparator(a: Chore, b: Chore, rootVNode: ElementVNode | null): nu
511535
return idxDiff;
512536
}
513537

514-
// If the host is the same, we need to compare the target.
538+
// If the host is the same (or missing), and the type is the same, we need to compare the target.
515539
if (
516540
a.$target$ !== b.$target$ &&
517-
((a.$type$ === ChoreType.QRL_RESOLVE && b.$type$ === ChoreType.QRL_RESOLVE) ||
518-
(a.$type$ === ChoreType.NODE_PROP && b.$type$ === ChoreType.NODE_PROP) ||
519-
(a.$type$ === ChoreType.RECOMPUTE_AND_SCHEDULE_EFFECTS &&
520-
b.$type$ === ChoreType.RECOMPUTE_AND_SCHEDULE_EFFECTS))
541+
(a.$type$ === ChoreType.QRL_RESOLVE ||
542+
a.$type$ === ChoreType.RUN_QRL ||
543+
a.$type$ === ChoreType.NODE_PROP ||
544+
a.$type$ === ChoreType.RECOMPUTE_AND_SCHEDULE_EFFECTS)
521545
) {
522546
// 1 means that we are going to process chores as FIFO
523547
return 1;
@@ -571,6 +595,7 @@ function debugChoreToString(chore: Chore): string {
571595
(
572596
{
573597
[ChoreType.QRL_RESOLVE]: 'QRL_RESOLVE',
598+
[ChoreType.RUN_QRL]: 'RUN_QRL',
574599
[ChoreType.RESOURCE]: 'RESOURCE',
575600
[ChoreType.TASK]: 'TASK',
576601
[ChoreType.NODE_DIFF]: 'NODE_DIFF',
@@ -586,7 +611,7 @@ function debugChoreToString(chore: Chore): string {
586611
)[chore.$type$] || 'UNKNOWN: ' + chore.$type$;
587612
const host = String(chore.$host$).replaceAll(/\n.*/gim, '');
588613
const qrlTarget = (chore.$target$ as QRLInternal<any>)?.$symbol$;
589-
return `Chore(${type} ${chore.$type$ === ChoreType.QRL_RESOLVE ? qrlTarget : host} ${chore.$idx$})`;
614+
return `Chore(${type} ${chore.$type$ === ChoreType.QRL_RESOLVE || chore.$type$ === ChoreType.RUN_QRL ? qrlTarget : host} ${chore.$idx$})`;
590615
}
591616

592617
function debugTrace(
@@ -603,7 +628,9 @@ function debugTrace(
603628
);
604629
}
605630
if (currentChore) {
606-
lines.push('running: ' + debugChoreToString(currentChore));
631+
lines.push(
632+
`${currentChore.$executed$ ? ' done' : 'running'}: ` + debugChoreToString(currentChore)
633+
);
607634
}
608635
if (queue) {
609636
queue.forEach((chore, idx) => {

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ const inflate = (
190190
switch (typeId) {
191191
case TypeIds.Object:
192192
// We use getters for making complex values lazy
193+
// TODO scan the data for computeQRLs and schedule resolve chores
193194
for (let i = 0; i < (data as any[]).length; i += 4) {
194195
const key = deserializeData(
195196
container,

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

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { isDev } from '@qwik.dev/core/build';
22
import { isQwikComponent } from '../shared/component.public';
3-
import { isQrl } from '../shared/qrl/qrl-class';
3+
import { createQRL, isQrl, type QRLInternal } from '../shared/qrl/qrl-class';
44
import type { QRL } from '../shared/qrl/qrl.public';
55
import { Fragment, directGetPropsProxyProp } from '../shared/jsx/jsx-runtime';
66
import { Slot } from '../shared/jsx/slot.public';
@@ -36,6 +36,7 @@ import { qInspector } from '../shared/utils/qdev';
3636
import { serializeAttribute } from '../shared/utils/styles';
3737
import { QError, qError } from '../shared/error/error';
3838
import { getFileLocationFromJsx } from '../shared/utils/jsx-filename';
39+
import { queueQRL } from '../client/queue-qrl';
3940

4041
class ParentComponentData {
4142
constructor(
@@ -484,12 +485,24 @@ function setEvent(
484485
const appendToValue = (valueToAppend: string) => {
485486
value = (value == null ? '' : value + '\n') + valueToAppend;
486487
};
488+
const getQrlString = (qrl: QRLInternal<unknown>) => {
489+
/**
490+
* If there are captures we need to schedule so everything is executed in the right order + qrls
491+
* are resolved.
492+
*
493+
* For internal qrls (starting with `_`) we assume that they do the right thing.
494+
*/
495+
if (!qrl.$symbol$.startsWith('_') && (qrl.$captureRef$ || qrl.$capture$)) {
496+
qrl = createQRL(null, '_run', queueQRL, null, null, [qrl]);
497+
}
498+
return qrlToString(serializationCtx, qrl);
499+
};
487500

488501
if (Array.isArray(qrls)) {
489502
for (let i = 0; i <= qrls.length; i++) {
490503
const qrl: unknown = qrls[i];
491504
if (isQrl(qrl)) {
492-
appendToValue(qrlToString(serializationCtx, qrl));
505+
appendToValue(getQrlString(qrl));
493506
addQwikEventToSerializationContext(serializationCtx, key, qrl);
494507
} else if (qrl != null) {
495508
// nested arrays etc.
@@ -500,7 +513,7 @@ function setEvent(
500513
}
501514
}
502515
} else if (isQrl(qrls)) {
503-
value = qrlToString(serializationCtx, qrls);
516+
value = getQrlString(qrls);
504517
addQwikEventToSerializationContext(serializationCtx, key, qrls);
505518
}
506519

0 commit comments

Comments
 (0)