Skip to content
Closed
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/nice-teams-grow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@qwik.dev/core': patch
---

fix: ensure DOM is updated during long running tasks
158 changes: 103 additions & 55 deletions packages/qwik/src/core/client/vnode-diff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
import { Fragment, type Props } from '../shared/jsx/jsx-runtime';
import { directGetPropsProxyProp, type PropsProxy } from '../shared/jsx/props-proxy';
import { Slot } from '../shared/jsx/slot.public';
import type { JSXNodeInternal, JSXOutput } from '../shared/jsx/types/jsx-node';
import type { JSXNodeInternal } from '../shared/jsx/types/jsx-node';
import type { JSXChildren } from '../shared/jsx/types/jsx-qwik-attributes';
import { SSRComment, SSRRaw, SkipRender } from '../shared/jsx/utils.public';
import type { QRLInternal } from '../shared/qrl/qrl-class';
Expand Down Expand Up @@ -44,9 +44,9 @@
Q_PREFIX,
dangerouslySetInnerHTML,
} from '../shared/utils/markers';
import { isPromise } from '../shared/utils/promises';
import { isPromise, retryOnPromise } from '../shared/utils/promises';
import { isSlotProp } from '../shared/utils/prop';
import { hasClassAttr } from '../shared/utils/scoped-styles';
import { addComponentStylePrefix, hasClassAttr } from '../shared/utils/scoped-styles';
import { serializeAttribute } from '../shared/utils/styles';
import { isArray, type ValueOrPromise } from '../shared/utils/types';
import { trackSignalAndAssignHost } from '../use/use-core';
Expand Down Expand Up @@ -84,7 +84,7 @@

export const vnode_diff = (
container: ClientContainer,
jsxNode: JSXOutput,
jsxNode: JSXChildren,
vStartNode: VNode,
scopedStyleIdPrefix: string | null
) => {
Expand All @@ -98,8 +98,7 @@
*/
const stack: any[] = [];

const asyncQueue: Array<VNode | ValueOrPromise<JSXOutput> | Promise<JSXOutput | JSXChildren>> =
[];
const asyncQueue: Array<VNode | ValueOrPromise<JSXChildren> | Promise<JSXChildren>> = [];

////////////////////////////////
//// Traverse state variables
Expand Down Expand Up @@ -151,7 +150,7 @@
//////////////////////////////////////////////
//////////////////////////////////////////////

function diff(jsxNode: JSXOutput, vStartNode: VNode) {
function diff(jsxNode: JSXChildren, vStartNode: VNode) {
assertFalse(vnode_isVNode(jsxNode), 'JSXNode should not be a VNode');
assertTrue(vnode_isVNode(vStartNode), 'vStartNode should be a VNode');
vParent = vStartNode as ElementVNode | VirtualVNode;
Expand Down Expand Up @@ -182,19 +181,17 @@
EffectSubscriptionProp.CONSUMER
];
if (currentSignal !== unwrappedSignal) {
const vHost = (vNewNode || vCurrent)!;
descend(
trackSignalAndAssignHost(
unwrappedSignal,
(vNewNode || vCurrent)!,
EffectProperty.VNODE,
container
resolveSignalAndDescend(() =>
trackSignalAndAssignHost(unwrappedSignal, vHost, EffectProperty.VNODE, container)
),
true
);
}
} else if (isPromise(jsxValue)) {
expectVirtual(VirtualType.Awaited, null);
asyncQueue.push(jsxValue, vNewNode || vCurrent);
asyncQueue.push(jsxValue, vNewNode || vCurrent, null);
} else if (isJSXNode(jsxValue)) {
const type = jsxValue.type;
if (typeof type === 'string') {
Expand Down Expand Up @@ -246,6 +243,21 @@
}
}

function resolveSignalAndDescend(fn: () => ValueOrPromise<any>): ValueOrPromise<any> {
try {
return fn();
} catch (e) {
// Signal threw a promise (async computed signal) - handle retry and async queue
if (isPromise(e)) {
// The thrown promise will resolve when the signal is ready, then retry fn() with retry logic
const retryPromise = e.then(() => retryOnPromise(fn));
asyncQueue.push(retryPromise, vNewNode || vCurrent, null);
return null;
}
throw e;
}
}

function advance() {
if (!shouldAdvance) {
shouldAdvance = true;
Expand Down Expand Up @@ -530,15 +542,30 @@

function drainAsyncQueue(): ValueOrPromise<void> {
while (asyncQueue.length) {
const jsxNode = asyncQueue.shift() as ValueOrPromise<JSXNodeInternal>;
let jsxNode = asyncQueue.shift() as ValueOrPromise<JSXChildren>;

Check failure on line 545 in packages/qwik/src/core/client/vnode-diff.ts

View workflow job for this annotation

GitHub Actions / Lint Package

'jsxNode' is never reassigned. Use 'const' instead
const vHostNode = asyncQueue.shift() as VNode;
if (isPromise(jsxNode)) {
return jsxNode.then((jsxNode) => {
const styleScopedId = asyncQueue.shift() as string | null;

const diffNode = (jsxNode: JSXChildren, vHostNode: VNode, styleScopedId: string | null) => {
if (styleScopedId) {
vnode_diff(container, jsxNode, vHostNode, addComponentStylePrefix(styleScopedId));
} else {
diff(jsxNode, vHostNode);
return drainAsyncQueue();
});
}
};

if (isPromise(jsxNode)) {
return jsxNode
.then((jsxNode) => {
diffNode(jsxNode, vHostNode, styleScopedId);
return drainAsyncQueue();
})
.catch((e) => {
container.handleError(e, vHostNode);
return drainAsyncQueue();
});
} else {
diff(jsxNode, vHostNode);
diffNode(jsxNode, vHostNode, styleScopedId);
}
}
}
Expand Down Expand Up @@ -594,6 +621,21 @@
): boolean {
const element = createElementWithNamespace(elementName);

function setAttribute(key: string, value: any, vHost: ElementVNode) {
value = serializeAttribute(key, value, scopedStyleIdPrefix);
if (value != null) {
if (vHost.flags & VNodeFlags.NS_svg) {
// only svg elements can have namespace attributes
const namespace = getAttributeNamespace(key);
if (namespace) {
element.setAttributeNS(namespace, key, String(value));
return;
}
}
element.setAttribute(key, String(value));
}
}

const { constProps } = jsx;
let needsQDispatchEventPatch = false;
if (constProps) {
Expand Down Expand Up @@ -637,15 +679,19 @@
}

if (isSignal(value)) {
value = trackSignalAndAssignHost(
value as Signal<unknown>,
vNewNode as ElementVNode,
key,
container,
CONST_SUBSCRIPTION_DATA
const vHost = vNewNode as ElementVNode;
const signal = value as Signal<unknown>;
value = retryOnPromise(() =>
trackSignalAndAssignHost(signal, vHost, key, container, CONST_SUBSCRIPTION_DATA)
);
}

if (isPromise(value)) {
const vHost = vNewNode as ElementVNode;
value.then((resolvedValue) => setAttribute(key, resolvedValue, vHost));
continue;
}

if (key === dangerouslySetInnerHTML) {
if (value) {
element.innerHTML = String(value);
Expand All @@ -665,18 +711,7 @@
continue;
}

value = serializeAttribute(key, value, scopedStyleIdPrefix);
if (value != null) {
if (vNewNode!.flags & VNodeFlags.NS_svg) {
// only svg elements can have namespace attributes
const namespace = getAttributeNamespace(key);
if (namespace) {
element.setAttributeNS(namespace, key, String(value));
continue;
}
}
element.setAttribute(key, String(value));
}
setAttribute(key, value, vNewNode as ElementVNode);
}
}
const key = jsx.key;
Expand Down Expand Up @@ -803,6 +838,14 @@
let dstIdx = 0;
let patchEventDispatch = false;

const setAttribute = (key: string, value: any, vHost: ElementVNode) => {
vHost.setAttr(
key,
value !== null ? serializeAttribute(key, value, scopedStyleIdPrefix) : null,
journal
);
};

const record = (key: string, value: any) => {
if (key.startsWith(':')) {
vnode.setProp(key, value);
Expand Down Expand Up @@ -837,12 +880,16 @@
// Only if we want to track the signal again
clearEffectSubscription(container, currentEffect);
}
value = trackSignalAndAssignHost(
unwrappedSignal,
vnode,
key,
container,
NON_CONST_SUBSCRIPTION_DATA

const vHost = vnode as ElementVNode;
value = retryOnPromise(() =>
trackSignalAndAssignHost(
unwrappedSignal,
vHost,
key,
container,
NON_CONST_SUBSCRIPTION_DATA
)
);
} else {
if (currentEffect) {
Expand All @@ -853,11 +900,13 @@
}
}

vnode.setAttr(
key,
value !== null ? serializeAttribute(key, value, scopedStyleIdPrefix) : null,
journal
);
if (isPromise(value)) {
const vHost = vnode as ElementVNode;
value.then((resolvedValue) => setAttribute(key, resolvedValue, vHost));
return;
}

setAttribute(key, value, vnode);
};

const recordJsxEvent = (key: string, value: any) => {
Expand Down Expand Up @@ -1113,11 +1162,8 @@
function expectVirtual(type: VirtualType, jsxKey: string | null) {
const checkKey = type === VirtualType.Fragment;
const currentKey = getKey(vCurrent);
const isSameNode =
vCurrent &&
vnode_isVirtualVNode(vCurrent) &&
currentKey === jsxKey &&
(checkKey ? !!jsxKey : true);
const currentIsVirtual = vCurrent && vnode_isVirtualVNode(vCurrent);
const isSameNode = currentIsVirtual && currentKey === jsxKey && (checkKey ? !!jsxKey : true);

if (isSameNode) {
// All is good.
Expand All @@ -1136,7 +1182,7 @@
isDev && (vNewNode as VirtualVNode).setProp(DEBUG_TYPE, type);
};
// For fragments without a key, always create a new virtual node (ensures rerender semantics)
if (checkKey && jsxKey === null) {
if (jsxKey === null) {
createNew();
return;
}
Expand Down Expand Up @@ -1169,6 +1215,8 @@
const lookupKeysAreEqual = lookupKey === vNodeLookupKey;
const hashesAreEqual = componentHash === vNodeComponentHash;

const jsxChildren = jsxNode.children;

Check failure on line 1218 in packages/qwik/src/core/client/vnode-diff.ts

View workflow job for this annotation

GitHub Actions / Build Qwik

'jsxChildren' is declared but its value is never read.

if (!lookupKeysAreEqual) {
const createNew = () => {
insertNewComponent(host, componentQRL, jsxProps);
Expand Down Expand Up @@ -1280,7 +1328,7 @@
jsxNode.props
);

asyncQueue.push(jsxOutput, host);
asyncQueue.push(jsxOutput, host, null);
}
}
}
Expand Down
15 changes: 14 additions & 1 deletion packages/qwik/src/core/client/vnode-diff.unit.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Fragment, _fnSignal, _jsxSorted, component$ } from '@qwik.dev/core';
import { Fragment, _fnSignal, _jsxSorted, component$, type JSXOutput } from '@qwik.dev/core';
import { vnode_fromJSX } from '@qwik.dev/core/testing';
import { describe, expect, it } from 'vitest';
import type { SignalImpl } from '../reactive-primitives/impl/signal-impl';
Expand Down Expand Up @@ -534,6 +534,19 @@ describe('vNode-diff', () => {
expect(vNode).toMatchVDOM(test);
expect(fragment).not.toBe(vnode_getFirstChild(vNode!));
});

it('should render fragment if only text was available', async () => {
const { vParent, container } = vnode_fromJSX('1');
const test = Promise.resolve('2') as unknown as JSXOutput; //_jsxSorted(Fragment, {}, null, ['1'], 0, null);

await vnode_diff(container, test, vParent, null);
vnode_applyJournal(container.$journal$);
expect(vParent).toMatchVDOM(
<body>
<Fragment>2</Fragment>
</body>
);
});
});
describe('attributes', () => {
describe('const props', () => {
Expand Down
Loading
Loading