Skip to content

Commit

Permalink
fix(core): schedule qrls instead of direct call
Browse files Browse the repository at this point in the history
  • Loading branch information
wmertens committed Jan 20, 2025
1 parent e4b83ac commit 26cd918
Show file tree
Hide file tree
Showing 9 changed files with 90 additions and 14 deletions.
5 changes: 5 additions & 0 deletions .changeset/heavy-radios-dream.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@qwik.dev/core': patch
---

FIX: QRLs are now scheduled instead of directly executed by qwik-loader, so that they are executed in the right order.
2 changes: 1 addition & 1 deletion packages/qwik/handlers.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@
*
* Make sure that these handlers are listed in manifest.ts
*/
export { _task } from '@qwik.dev/core';
export { _run, _task } from '@qwik.dev/core';
3 changes: 3 additions & 0 deletions packages/qwik/src/core/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -805,6 +805,9 @@ export type ResourceReturn<T> = ResourcePending<T> | ResourceResolved<T> | Resou
// @internal (undocumented)
export const _restProps: (props: Record<string, any>, omit: string[], target?: {}) => {};

// @internal
export const _run: (...args: unknown[]) => ValueOrPromise<void>;

// @internal
export function _serialize(data: unknown[]): Promise<string>;

Expand Down
27 changes: 27 additions & 0 deletions packages/qwik/src/core/client/queue-qrl.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { ChoreType } from '../shared/scheduler';
import type { QRLInternal } from '../shared/qrl/qrl-class';
import { getInvokeContext } from '../use/use-core';
import { useLexicalScope } from '../use/use-lexical-scope.public';
import { getDomContainer } from './dom-container';
import { _getQContainerElement } from './dom-container';

/**
* This is called by qwik-loader to schedule a QRL. It has to be synchronous.
*
* @internal
*/
export const queueQRL = (...args: unknown[]) => {
// This will already check container
const [runQrl] = useLexicalScope<[QRLInternal<(...args: unknown[]) => unknown>]>();
const context = getInvokeContext();
const el = context.$element$!;
const containerElement = _getQContainerElement(el) as HTMLElement;
const container = getDomContainer(containerElement);

const scheduler = container.$scheduler$;
if (!scheduler) {
throw new Error('No scheduler found');
}

return scheduler(ChoreType.RUN_QRL, null, runQrl, args);
};
1 change: 1 addition & 0 deletions packages/qwik/src/core/internal.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
export { _noopQrl, _noopQrlDEV, _regSymbol } from './shared/qrl/qrl';
export { _walkJSX } from './ssr/ssr-render-jsx';
export { _SharedContainer } from './shared/shared-container';
export { queueQRL as _run } from './client/queue-qrl';
export { scheduleTask as _task } from './use/use-task';
export { _wrapSignal, _wrapProp } from './signal/signal-utils';
export { _restProps } from './shared/utils/prop';
Expand Down
44 changes: 35 additions & 9 deletions packages/qwik/src/core/shared/scheduler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,18 +126,24 @@ export const enum ChoreType {
/* order of elements (not encoded here) */
MICRO /* **************************** */ = 0b0000_1111,

/** Ensure tha the QRL promise is resolved before processing next chores in the queue */
/** Ensure that the QRL promise is resolved before processing next chores in the queue */
QRL_RESOLVE /* ********************** */ = 0b0000_0001,
RESOURCE /* ************************* */ = 0b0000_0010,
TASK /* ***************************** */ = 0b0000_0011,
NODE_DIFF /* ************************ */ = 0b0000_0100,
NODE_PROP /* ************************ */ = 0b0000_0101,
COMPONENT_SSR /* ******************** */ = 0b0000_0110,
COMPONENT /* ************************ */ = 0b0000_0111,
RECOMPUTE_AND_SCHEDULE_EFFECTS /* *** */ = 0b0000_1000,
RUN_QRL,
RESOURCE,
TASK,
NODE_DIFF,
NODE_PROP,
COMPONENT_SSR,
COMPONENT,
RECOMPUTE_AND_SCHEDULE_EFFECTS,

// Next macro level
JOURNAL_FLUSH /* ******************** */ = 0b0001_0000,
// Next macro level
VISIBLE /* ************************** */ = 0b0010_0000,
// Next macro level
CLEANUP_VISIBLE /* ****************** */ = 0b0011_0000,
// Next macro level
WAIT_FOR_ALL /* ********************* */ = 0b1111_1111,
}

Expand Down Expand Up @@ -206,6 +212,12 @@ export const createScheduler = (
type: ChoreType.TASK | ChoreType.VISIBLE | ChoreType.RESOURCE,
task: Task
): ValueOrPromise<void>;
function schedule(
type: ChoreType.RUN_QRL,
ignore: null,
target: QRLInternal<(...args: unknown[]) => unknown>,
args: unknown[]
): ValueOrPromise<void>;
function schedule(
type: ChoreType.COMPONENT,
host: HostElement,
Expand Down Expand Up @@ -238,7 +250,10 @@ export const createScheduler = (
targetOrQrl: ChoreTarget | string | null = null,
payload: any = null
): ValueOrPromise<any> {
const runLater: boolean = type !== ChoreType.WAIT_FOR_ALL && type !== ChoreType.COMPONENT_SSR;
const runLater: boolean =
type !== ChoreType.WAIT_FOR_ALL &&
type !== ChoreType.COMPONENT_SSR &&
type !== ChoreType.RUN_QRL;
const isTask =
type === ChoreType.TASK ||
type === ChoreType.VISIBLE ||
Expand All @@ -247,6 +262,10 @@ export const createScheduler = (
if (isTask) {
(hostOrTask as Task).$flags$ |= TaskFlags.DIRTY;
}
if (type === ChoreType.RUN_QRL) {
// Bind the qrl to the current context
targetOrQrl = (targetOrQrl as QRLInternal<(...args: unknown[]) => unknown>).getFn();
}
let chore: Chore = {
$type$: type,
$idx$: isTask
Expand Down Expand Up @@ -368,6 +387,12 @@ export const createScheduler = (
const result = runResource(chore.$payload$ as ResourceDescriptor<TaskFn>, container, host);
returnValue = isDomContainer(container) ? null : result;
break;
case ChoreType.RUN_QRL:
{
const fn = chore.$target$ as (...args: unknown[]) => unknown;
returnValue = fn(...(chore.$payload$ as unknown[]));
}
break;
case ChoreType.TASK:
returnValue = runTask(chore.$payload$ as Task<TaskFn, TaskFn>, container, host);
break;
Expand Down Expand Up @@ -593,6 +618,7 @@ function debugChoreToString(chore: Chore): string {
(
{
[ChoreType.QRL_RESOLVE]: 'QRL_RESOLVE',
[ChoreType.RUN_QRL]: 'RUN_QRL',
[ChoreType.RESOURCE]: 'RESOURCE',
[ChoreType.TASK]: 'TASK',
[ChoreType.NODE_DIFF]: 'NODE_DIFF',
Expand Down
1 change: 1 addition & 0 deletions packages/qwik/src/core/shared/shared-serialization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ const inflate = (
switch (typeId) {
case TypeIds.Object:
// We use getters for making complex values lazy
// TODO scan the data for computeQRLs and schedule resolve chores
for (let i = 0; i < (data as any[]).length; i += 4) {
const key = deserializeData(
container,
Expand Down
19 changes: 16 additions & 3 deletions packages/qwik/src/core/ssr/ssr-render-jsx.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { isDev } from '@qwik.dev/core/build';
import { isQwikComponent } from '../shared/component.public';
import { isQrl } from '../shared/qrl/qrl-class';
import { createQRL, isQrl, type QRLInternal } from '../shared/qrl/qrl-class';
import type { QRL } from '../shared/qrl/qrl.public';
import { Fragment, directGetPropsProxyProp } from '../shared/jsx/jsx-runtime';
import { Slot } from '../shared/jsx/slot.public';
Expand Down Expand Up @@ -36,6 +36,7 @@ import { qInspector } from '../shared/utils/qdev';
import { serializeAttribute } from '../shared/utils/styles';
import { QError, qError } from '../shared/error/error';
import { getFileLocationFromJsx } from '../shared/utils/jsx-filename';
import { queueQRL } from '../client/queue-qrl';

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

if (Array.isArray(qrls)) {
for (let i = 0; i <= qrls.length; i++) {
const qrl: unknown = qrls[i];
if (isQrl(qrl)) {
appendToValue(qrlToString(serializationCtx, qrl));
appendToValue(getQrlString(qrl));
addQwikEventToSerializationContext(serializationCtx, key, qrl);
} else if (qrl != null) {
// nested arrays etc.
Expand All @@ -500,7 +513,7 @@ function setEvent(
}
}
} else if (isQrl(qrls)) {
value = qrlToString(serializationCtx, qrls);
value = getQrlString(qrls);
addQwikEventToSerializationContext(serializationCtx, key, qrls);
}

Expand Down
2 changes: 1 addition & 1 deletion packages/qwik/src/optimizer/src/manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type { GlobalInjections, SegmentAnalysis, Path, QwikBundle, QwikManifest

// The handlers that are exported by the core package
// See handlers.mjs
const extraSymbols = new Set(['_task']);
const extraSymbols = new Set(['_run', '_task']);

// This is just the initial prioritization of the symbols and entries
// at build time so there's less work during each SSR. However, SSR should
Expand Down

0 comments on commit 26cd918

Please sign in to comment.