Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(core): schedule QRLs instead of direct execution #7269

Open
wants to merge 8 commits into
base: build/v2
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.
5 changes: 5 additions & 0 deletions .changeset/olive-cameras-collect.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@qwik.dev/core': patch
---

CHORE: replace the `_hW` export in segments with a shared export `_task` in core. This opens up using QRLs from core.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ jobs:
pnpm install --frozen-lockfile

- name: 'build: qwik'
run: pnpm build --qwik --set-dist-tag="${{ needs.changes.outputs.disttag }}"
run: pnpm build --qwik --insights --set-dist-tag="${{ needs.changes.outputs.disttag }}"

- name: Print Qwik Dist Build
continue-on-error: true
Expand Down
14 changes: 7 additions & 7 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -172,20 +172,20 @@
"build.clean": "rm -rf packages/qwik/dist/ && rm -rf packages/qwik-router/lib/ && rm -rf packages/docs/dist/ && rm -rf packages/insights/dist/",
"build.cli": "tsx --require ./scripts/runBefore.ts scripts/index.ts --cli --dev",
"build.cli.prod": "tsx --require ./scripts/runBefore.ts scripts/index.ts --cli",
"build.core": "tsx --require ./scripts/runBefore.ts scripts/index.ts --tsc --build --qwikrouter --api --platform-binding",
"build.core": "tsx --require ./scripts/runBefore.ts scripts/index.ts --tsc --qwik --insights --qwikrouter --api --platform-binding",
"build.eslint": "tsx --require ./scripts/runBefore.ts scripts/index.ts --eslint",
"build.full": "tsx --require ./scripts/runBefore.ts scripts/index.ts --tsc --tsc-docs --qwik --supabaseauthhelpers --api --eslint --qwikrouter --qwikworker --qwikreact --cli --platform-binding --wasm",
"build.local": "tsx --require ./scripts/runBefore.ts scripts/index.ts --tsc --tsc-docs --qwik --supabaseauthhelpers --api --eslint --qwikrouter --qwikworker --qwikreact --cli --platform-binding-wasm-copy",
"build.only_javascript": "tsx --require ./scripts/runBefore.ts scripts/index.ts --tsc --build --api",
"build.full": "tsx --require ./scripts/runBefore.ts scripts/index.ts --tsc --tsc-docs --qwik --insights --supabaseauthhelpers --api --eslint --qwikrouter --qwikworker --qwikreact --cli --platform-binding --wasm",
"build.local": "tsx --require ./scripts/runBefore.ts scripts/index.ts --tsc --tsc-docs --qwik --insights --supabaseauthhelpers --api --eslint --qwikrouter --qwikworker --qwikreact --cli --platform-binding-wasm-copy",
"build.only_javascript": "tsx --require ./scripts/runBefore.ts scripts/index.ts --tsc --qwik --api",
"build.packages.docs": "pnpm -C ./packages/docs/ run build",
"build.packages.insights": "pnpm -C ./packages/insights/ run build",
"build.platform": "tsx --require ./scripts/runBefore.ts scripts/index.ts --platform-binding",
"build.platform.copy": "tsx --require ./scripts/runBefore.ts scripts/index.ts --platform-binding-wasm-copy",
"build.qwik-router": "tsx --require ./scripts/runBefore.ts scripts/index.ts --tsc --qwikrouter",
"build.validate": "tsx --require ./scripts/runBefore.ts scripts/index.ts --tsc --build --api --eslint --qwikrouter --platform-binding --wasm --validate",
"build.vite": "tsx --require ./scripts/runBefore.ts scripts/index.ts --tsc --build --api --qwikrouter --eslint --platform-binding-wasm-copy",
"build.validate": "tsx --require ./scripts/runBefore.ts scripts/index.ts --tsc --qwik --api --eslint --qwikrouter --platform-binding --wasm --validate",
"build.vite": "tsx --require ./scripts/runBefore.ts scripts/index.ts --tsc --qwik --insights --api --qwikrouter --eslint --platform-binding-wasm-copy",
"build.wasm": "tsx --require ./scripts/runBefore.ts scripts/index.ts --wasm",
"build.watch": "tsx --require ./scripts/runBefore.ts scripts/index.ts --build --qwikrouter --watch --dev --platform-binding",
"build.watch": "tsx --require ./scripts/runBefore.ts scripts/index.ts --qwik --qwikrouter --watch --dev --platform-binding",
"change": "changeset",
"cli": "pnpm build.cli && node packages/create-qwik/dist/create-qwik.cjs && tsx --require ./scripts/runBefore.ts scripts/validate-cli.ts --copy-local-qwik-dist",
"cli.qwik": "pnpm build.cli && node packages/qwik/dist/qwik-cli.cjs",
Expand Down
9 changes: 9 additions & 0 deletions packages/qwik/handlers.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/**
* This re-exports the QRL handlers so that they can be used as QRLs.
*
* In vite dev mode, this file is referenced directly. This ensures that the correct path to core is
* used so that there's only one instance of Qwik.
*
* Make sure that these handlers are listed in manifest.ts
*/
export { _run, _task } from '@qwik.dev/core';
2 changes: 2 additions & 0 deletions packages/qwik/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
"./cli": {
"require": "./dist/cli.cjs"
},
"./handlers.mjs": "./handlers.mjs",
"./internal": {
"types": "./core-internal.d.ts",
"import": {
Expand Down Expand Up @@ -144,6 +145,7 @@
"bindings",
"build.d.ts",
"cli.d.ts",
"handlers.mjs",
"jsx-dev-runtime.d.ts",
"jsx-runtime.d.ts",
"loader.d.ts",
Expand Down
9 changes: 6 additions & 3 deletions packages/qwik/src/core/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -316,9 +316,6 @@ function h<TYPE extends string | FunctionComponent<PROPS>, PROPS extends {} = {}
export { h as createElement }
export { h }

// @internal
export const _hW: () => void;

// @internal @deprecated (undocumented)
export const _IMMUTABLE: unique symbol;

Expand Down Expand Up @@ -808,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 Expand Up @@ -997,6 +997,9 @@ export interface SyncQRL<TYPE extends Function = any> extends QRL<TYPE> {
resolved: TYPE;
}

// @internal
export const _task: () => void;

// @public (undocumented)
export interface TaskCtx {
// (undocumented)
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 { 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);
};
7 changes: 6 additions & 1 deletion packages/qwik/src/core/client/vnode-diff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
});
Expand Down
3 changes: 2 additions & 1 deletion packages/qwik/src/core/internal.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
export { _noopQrl, _noopQrlDEV, _regSymbol } from './shared/qrl/qrl';
export { _walkJSX } from './ssr/ssr-render-jsx';
export { _SharedContainer } from './shared/shared-container';
export { _hW } from './use/use-task';
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';
export { _IMMUTABLE } from './shared/utils/constants';
Expand Down
22 changes: 10 additions & 12 deletions packages/qwik/src/core/shared/error/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ 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
'', // 2 unused
'Scheduler not found', // 1
'track() received object, without prop to track', // 2
'Only primitive and object literals can be serialized. {{0}}', // 3
'', // 4 unused
'You can render over a existing q:container. Skipping render().', // 5
Expand Down Expand Up @@ -50,12 +50,11 @@ export const codeToText = (code: number, ...parts: any[]): string => {
"Element must have 'q:container' attribute.", // 42
'Unknown vnode type {{0}}.', // 43
'Materialize error: missing element: {{0}} {{1}} {{2}}', // 44
'Cannot coerce a Signal, use `.value` instead', // 46
'useComputedSignal$ QRL {{0}} {{1}} returned a Promise', // 47
'ComputedSignal is read-only', // 48
'WrappedSignal is read-only', // 49
'SsrError: Promises not expected here.', // 50
'Attribute value is unsafe for SSR', // 51
'Cannot coerce a Signal, use `.value` instead', // 45
'useComputedSignal$ QRL {{0}} {{1}} returned a Promise', // 46
'ComputedSignal is read-only', // 47
'WrappedSignal is read-only', // 48
'Attribute value is unsafe for SSR', // 49
];
let text = MAP[code] ?? '';
if (parts.length) {
Expand All @@ -76,8 +75,8 @@ export const codeToText = (code: number, ...parts: any[]): string => {

export const enum QError {
stringifyClassOrStyle = 0,
UNUSED_1 = 1,
UNUSED_2 = 2,
schedulerNotFound = 1,
trackObjectWithoutProp = 2,
verifySerializable = 3,
UNUSED_4 = 4,
cannotRenderOverExistingContainer = 5,
Expand Down Expand Up @@ -124,8 +123,7 @@ export const enum QError {
computedNotSync = 46,
computedReadOnly = 47,
wrappedReadOnly = 48,
promisesNotExpected = 49,
unsafeAttr = 50,
unsafeAttr = 49,
}

export const qError = (code: number, errorMessageArgs: any[] = []): Error => {
Expand Down
10 changes: 3 additions & 7 deletions packages/qwik/src/core/shared/qrl/qrl-class.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ export const isSyncQrl = (value: any): value is SyncQRLInternal => {
export type QRLInternalMethods<TYPE> = {
readonly $chunk$: string | null;
readonly $symbol$: string;
readonly $refSymbol$: string | null;
readonly $hash$: string;

$capture$: string[] | null;
Expand Down Expand Up @@ -74,8 +73,7 @@ export const createQRL = <TYPE>(
symbolRef: null | ValueOrPromise<TYPE>,
symbolFn: null | (() => Promise<Record<string, TYPE>>),
capture: null | Readonly<number[]>,
captureRef: Readonly<unknown[]> | null,
refSymbol: string | null
captureRef: Readonly<unknown[]> | null
): QRLInternal<TYPE> => {
if (qDev && qSerialize) {
if (captureRef) {
Expand Down Expand Up @@ -201,19 +199,17 @@ export const createQRL = <TYPE>(
}
};

const resolvedSymbol = refSymbol ?? symbol;
const hash = getSymbolHash(resolvedSymbol);
const hash = getSymbolHash(symbol);

Object.assign(qrl, {
getSymbol: () => resolvedSymbol,
getSymbol: () => symbol,
getHash: () => hash,
getCaptured: () => captureRef,
resolve,
$resolveLazy$: resolveLazy,
$setContainer$: setContainer,
$chunk$: chunk,
$symbol$: symbol,
$refSymbol$: refSymbol,
$hash$: hash,
getFn: bindFnToContext,

Expand Down
6 changes: 3 additions & 3 deletions packages/qwik/src/core/shared/qrl/qrl.public.ts
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ export const $ = <T>(expression: T): QRL<T> => {
);
}

return createQRL<T>(null, 's' + runtimeSymbolId++, expression, null, null, null, null);
return createQRL<T>(null, 's' + runtimeSymbolId++, expression, null, null, null);
};
/** @private Use To avoid optimizer replacement */
export const dollar = $;
Expand Down Expand Up @@ -302,7 +302,7 @@ export const sync$ = <T extends Function>(fn: T): SyncQRL<T> => {
fn = new Function('return ' + fn.toString())() as any;
}

return createQRL<T>('', SYNC_QRL, fn, null, null, null, null) as any;
return createQRL<T>('', SYNC_QRL, fn, null, null, null) as any;
};

/**
Expand All @@ -323,5 +323,5 @@ export const _qrlSync = function <TYPE extends Function>(
serializedFn = fn.toString();
}
(fn as any).serialized = serializedFn;
return createQRL<TYPE>('', SYNC_QRL, fn, null, null, null, null) as any;
return createQRL<TYPE>('', SYNC_QRL, fn, null, null, null) as any;
};
6 changes: 3 additions & 3 deletions packages/qwik/src/core/shared/qrl/qrl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export const qrl = <T = any>(
}

// Unwrap subscribers
return createQRL<T>(chunk, symbol, null, symbolFn, null, lexicalScopeCapture, null);
return createQRL<T>(chunk, symbol, null, symbolFn, null, lexicalScopeCapture);
};

/** @internal */
Expand All @@ -95,15 +95,15 @@ export const inlinedQrl = <T>(
lexicalScopeCapture: any[] = EMPTY_ARRAY
): QRL<T> => {
// Unwrap subscribers
return createQRL<T>(null, symbolName, symbol, null, null, lexicalScopeCapture, null);
return createQRL<T>(null, symbolName, symbol, null, null, lexicalScopeCapture);
};

/** @internal */
export const _noopQrl = <T>(
symbolName: string,
lexicalScopeCapture: any[] = EMPTY_ARRAY
): QRL<T> => {
return createQRL<T>(null, symbolName, null, null, null, lexicalScopeCapture, null);
return createQRL<T>(null, symbolName, null, null, null, lexicalScopeCapture);
};

/** @internal */
Expand Down
Loading
Loading