diff --git a/.changeset/heavy-radios-dream.md b/.changeset/heavy-radios-dream.md new file mode 100644 index 00000000000..8db7e56e562 --- /dev/null +++ b/.changeset/heavy-radios-dream.md @@ -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. diff --git a/packages/qwik/handlers.mjs b/packages/qwik/handlers.mjs index de787e31b57..71aea661713 100644 --- a/packages/qwik/handlers.mjs +++ b/packages/qwik/handlers.mjs @@ -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'; diff --git a/packages/qwik/src/core/api.md b/packages/qwik/src/core/api.md index c8f59ea16c9..7bbda239ded 100644 --- a/packages/qwik/src/core/api.md +++ b/packages/qwik/src/core/api.md @@ -805,6 +805,9 @@ export type ResourceReturn = ResourcePending | ResourceResolved | Resou // @internal (undocumented) export const _restProps: (props: Record, omit: string[], target?: {}) => {}; +// @internal +export const _run: (...args: unknown[]) => ValueOrPromise; + // @internal export function _serialize(data: unknown[]): Promise; diff --git a/packages/qwik/src/core/client/queue-qrl.ts b/packages/qwik/src/core/client/queue-qrl.ts new file mode 100644 index 00000000000..bbcc149b45b --- /dev/null +++ b/packages/qwik/src/core/client/queue-qrl.ts @@ -0,0 +1,27 @@ +import { QError, qError } from '../shared/error/error'; +import type { QRLInternal } from '../shared/qrl/qrl-class'; +import { ChoreType } from '../shared/scheduler'; +import { getInvokeContext } from '../use/use-core'; +import { useLexicalScope } from '../use/use-lexical-scope.public'; +import { _getQContainerElement, getDomContainer } 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 qError(QError.schedulerNotFound); + } + + return scheduler(ChoreType.RUN_QRL, null, runQrl, args); +}; diff --git a/packages/qwik/src/core/client/vnode-diff.ts b/packages/qwik/src/core/client/vnode-diff.ts index 7bb62f59edc..cab386f59dd 100644 --- a/packages/qwik/src/core/client/vnode-diff.ts +++ b/packages/qwik/src/core/client/vnode-diff.ts @@ -764,7 +764,12 @@ export const vnode_diff = ( let returnValue = false; qrls.flat(2).forEach((qrl) => { if (qrl) { - const value = qrl(event, element) as any; + const value = container.$scheduler$( + ChoreType.RUN_QRL, + null, + qrl as QRLInternal<(...args: unknown[]) => unknown>, + [event, element] + ) as unknown; returnValue = returnValue || value === true; } }); diff --git a/packages/qwik/src/core/internal.ts b/packages/qwik/src/core/internal.ts index e5947a25507..5aaf7cbb838 100644 --- a/packages/qwik/src/core/internal.ts +++ b/packages/qwik/src/core/internal.ts @@ -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'; diff --git a/packages/qwik/src/core/shared/error/error.ts b/packages/qwik/src/core/shared/error/error.ts index 409a03042f4..116acfc959b 100644 --- a/packages/qwik/src/core/shared/error/error.ts +++ b/packages/qwik/src/core/shared/error/error.ts @@ -6,7 +6,7 @@ export const codeToText = (code: number, ...parts: any[]): string => { // Keep one error, one line to make it easier to search for the error message. const MAP = [ 'Error while serializing class or style attributes', // 0 - '', // 1 unused + 'Scheduler not found', // 1 '', // 2 unused 'Only primitive and object literals can be serialized. {{0}}', // 3 '', // 4 unused @@ -76,7 +76,7 @@ export const codeToText = (code: number, ...parts: any[]): string => { export const enum QError { stringifyClassOrStyle = 0, - UNUSED_1 = 1, + schedulerNotFound = 1, UNUSED_2 = 2, verifySerializable = 3, UNUSED_4 = 4, diff --git a/packages/qwik/src/core/shared/scheduler.ts b/packages/qwik/src/core/shared/scheduler.ts index 8cc3f594fc9..245b1225a71 100644 --- a/packages/qwik/src/core/shared/scheduler.ts +++ b/packages/qwik/src/core/shared/scheduler.ts @@ -122,18 +122,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, } @@ -202,6 +208,12 @@ export const createScheduler = ( type: ChoreType.TASK | ChoreType.VISIBLE | ChoreType.RESOURCE, task: Task ): ValueOrPromise; + function schedule( + type: ChoreType.RUN_QRL, + ignore: null, + target: QRLInternal<(...args: unknown[]) => unknown>, + args: unknown[] + ): ValueOrPromise; function schedule( type: ChoreType.COMPONENT, host: HostElement, @@ -234,7 +246,10 @@ export const createScheduler = ( targetOrQrl: ChoreTarget | string | null = null, payload: any = null ): ValueOrPromise { - 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 || @@ -289,15 +304,19 @@ export const createScheduler = ( return runUptoChore.$promise$; } while (choreQueue.length) { - const nextChore = choreQueue.shift()!; + const nextChore = choreQueue[0]; const order = choreComparator(nextChore, runUptoChore, rootVNode); - if (order === null) { - continue; - } - if (order > 0) { + if (order !== null && order > 0) { // we have processed all of the chores up to and including the given chore. break; } + // We can take the chore out of the queue + choreQueue.shift(); + if (order === null) { + // There was an error with the chore. + DEBUG && debugTrace('skip chore', nextChore, currentChore, choreQueue); + continue; + } const isDeletedVNode = vNodeAlreadyDeleted(nextChore); if ( isDeletedVNode && @@ -364,6 +383,12 @@ export const createScheduler = ( const result = runResource(chore.$payload$ as ResourceDescriptor, container, host); returnValue = isDomContainer(container) ? null : result; break; + case ChoreType.RUN_QRL: + { + const fn = (chore.$target$ as QRLInternal<(...args: unknown[]) => unknown>).getFn(); + returnValue = fn(...(chore.$payload$ as unknown[])); + } + break; case ChoreType.TASK: returnValue = runTask(chore.$payload$ as Task, container, host); break; @@ -423,11 +448,11 @@ export const createScheduler = ( } } return maybeThenPassError(returnValue, (value) => { - DEBUG && debugTrace('execute.DONE', null, currentChore, choreQueue); if (currentChore) { currentChore.$executed$ = true; currentChore.$resolve$?.(value); } + DEBUG && debugTrace('execute.DONE', null, currentChore, choreQueue); currentChore = null; return (chore.$returnValue$ = value); }); @@ -476,7 +501,6 @@ function choreComparator(a: Chore, b: Chore, rootVNode: ElementVNode | null): nu const aHost = a.$host$; const bHost = b.$host$; - // QRL_RESOLVE does not have a host. if (aHost !== bHost && aHost !== null && bHost !== null) { if (vnode_isVNode(aHost) && vnode_isVNode(bHost)) { // we are running on the client. @@ -511,13 +535,13 @@ function choreComparator(a: Chore, b: Chore, rootVNode: ElementVNode | null): nu return idxDiff; } - // If the host is the same, we need to compare the target. + // If the host is the same (or missing), and the type is the same, we need to compare the target. if ( a.$target$ !== b.$target$ && - ((a.$type$ === ChoreType.QRL_RESOLVE && b.$type$ === ChoreType.QRL_RESOLVE) || - (a.$type$ === ChoreType.NODE_PROP && b.$type$ === ChoreType.NODE_PROP) || - (a.$type$ === ChoreType.RECOMPUTE_AND_SCHEDULE_EFFECTS && - b.$type$ === ChoreType.RECOMPUTE_AND_SCHEDULE_EFFECTS)) + (a.$type$ === ChoreType.QRL_RESOLVE || + a.$type$ === ChoreType.RUN_QRL || + a.$type$ === ChoreType.NODE_PROP || + a.$type$ === ChoreType.RECOMPUTE_AND_SCHEDULE_EFFECTS) ) { // 1 means that we are going to process chores as FIFO return 1; @@ -571,6 +595,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', @@ -586,7 +611,7 @@ function debugChoreToString(chore: Chore): string { )[chore.$type$] || 'UNKNOWN: ' + chore.$type$; const host = String(chore.$host$).replaceAll(/\n.*/gim, ''); const qrlTarget = (chore.$target$ as QRLInternal)?.$symbol$; - return `Chore(${type} ${chore.$type$ === ChoreType.QRL_RESOLVE ? qrlTarget : host} ${chore.$idx$})`; + return `Chore(${type} ${chore.$type$ === ChoreType.QRL_RESOLVE || chore.$type$ === ChoreType.RUN_QRL ? qrlTarget : host} ${chore.$idx$})`; } function debugTrace( @@ -603,7 +628,9 @@ function debugTrace( ); } if (currentChore) { - lines.push('running: ' + debugChoreToString(currentChore)); + lines.push( + `${currentChore.$executed$ ? ' done' : 'running'}: ` + debugChoreToString(currentChore) + ); } if (queue) { queue.forEach((chore, idx) => { diff --git a/packages/qwik/src/core/shared/shared-serialization.ts b/packages/qwik/src/core/shared/shared-serialization.ts index 66523cfa3b0..3c8f55b84c7 100644 --- a/packages/qwik/src/core/shared/shared-serialization.ts +++ b/packages/qwik/src/core/shared/shared-serialization.ts @@ -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, diff --git a/packages/qwik/src/core/ssr/ssr-render-jsx.ts b/packages/qwik/src/core/ssr/ssr-render-jsx.ts index 354d9f0afcb..2d2351ae45a 100644 --- a/packages/qwik/src/core/ssr/ssr-render-jsx.ts +++ b/packages/qwik/src/core/ssr/ssr-render-jsx.ts @@ -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'; @@ -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( @@ -484,12 +485,24 @@ function setEvent( const appendToValue = (valueToAppend: string) => { value = (value == null ? '' : value + '\n') + valueToAppend; }; + const getQrlString = (qrl: QRLInternal) => { + /** + * 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. @@ -500,7 +513,7 @@ function setEvent( } } } else if (isQrl(qrls)) { - value = qrlToString(serializationCtx, qrls); + value = getQrlString(qrls); addQwikEventToSerializationContext(serializationCtx, key, qrls); } diff --git a/packages/qwik/src/core/tests/render-api.spec.tsx b/packages/qwik/src/core/tests/render-api.spec.tsx index e787aadae2d..d3c081c2c0b 100644 --- a/packages/qwik/src/core/tests/render-api.spec.tsx +++ b/packages/qwik/src/core/tests/render-api.spec.tsx @@ -57,6 +57,9 @@ const mapping = { click: 'click.js', s_counter: 's_counter.js', s_click: 's_click.js', + // Our internal symbols + _run: 'core', + _task: 'core', }; const defaultManifest: QwikManifest = { @@ -1121,7 +1124,8 @@ describe('render api', () => { stream, streaming, }); - expect(stream.write).toHaveBeenCalledTimes(5); + // This can change when the size of the output changes + expect(stream.write).toHaveBeenCalledTimes(6); }); }); }); diff --git a/packages/qwik/src/optimizer/src/manifest.ts b/packages/qwik/src/optimizer/src/manifest.ts index c8a29264988..89cce7ebb48 100644 --- a/packages/qwik/src/optimizer/src/manifest.ts +++ b/packages/qwik/src/optimizer/src/manifest.ts @@ -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 diff --git a/packages/qwik/src/testing/rendering.unit-util.tsx b/packages/qwik/src/testing/rendering.unit-util.tsx index f59e43d43a2..b85d7edff0c 100644 --- a/packages/qwik/src/testing/rendering.unit-util.tsx +++ b/packages/qwik/src/testing/rendering.unit-util.tsx @@ -271,14 +271,17 @@ function renderStyles(getStyles: () => Record) { }); } -export async function rerenderComponent(element: HTMLElement) { +export async function rerenderComponent(element: HTMLElement, flush?: boolean) { const container = _getDomContainer(element); const vElement = vnode_locate(container.rootVNode, element); const host = getHostVNode(vElement) as HostElement; const qrl = container.getHostProp>>(host, OnRenderProp)!; const props = container.getHostProp(host, ELEMENT_PROPS); - await container.$scheduler$(ChoreType.COMPONENT, host, qrl, props); - await getTestPlatform().flush(); + container.$scheduler$(ChoreType.COMPONENT, host, qrl, props); + if (flush) { + // Note that this can deadlock + await getTestPlatform().flush(); + } } function getHostVNode(vElement: _VNode | null) {