Skip to content

feat: used optimized method for setting changed attribute value #8171

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Nov 21, 2025
+57 −21
Merged

feat: used optimized method for setting changed attribute value #8171

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
78 changes: 57 additions & 21 deletions packages/qwik/src/core/client/vnode-diff.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ import {
QDefaultSlot,
QSlot,
QTemplate,
Q_PREFIX,
dangerouslySetInnerHTML,
} from '../shared/utils/markers';
import { isPromise, retryOnPromise } from '../shared/utils/promises';
Expand All @@ -66,6 +65,7 @@ import type { DomContainer } from './dom-container';
import { VNodeFlags, type ClientAttrs, type ClientContainer } from './types';
import { mapApp_findIndx, mapArray_set } from './util-mapArray';
import {
VNodeJournalOpCode,
vnode_ensureElementInflated,
vnode_getDomParentVNode,
vnode_getElementName,
Expand All @@ -90,7 +90,7 @@ import {
vnode_walkVNode,
type VNodeJournal,
} from './vnode';
import type { ElementVNode, TextVNode, VNode, VirtualVNode } from './vnode-impl';
import { ElementVNode, TextVNode, VNode, VirtualVNode } from './vnode-impl';
import { getAttributeNamespace, getNewElementNamespaceData } from './vnode-namespace';
import { cleanupDestroyable } from '../use/utils/destroyable';
import { SignalImpl } from '../reactive-primitives/impl/signal-impl';
Expand Down Expand Up @@ -858,15 +858,44 @@ export const vnode_diff = (
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
);
/**
* Optimized setAttribute that bypasses redundant checks when we already know:
*
* - The index in dstAttrs (no need for binary search)
* - The vnode is ElementVNode (no instanceof check)
* - The value has changed (no comparison needed)
*/
const setAttributeDirect = (
vnode: ElementVNode,
key: string,
value: any,
dstIdx: number,
isNewKey: boolean
) => {
const serializedValue =
value != null ? serializeAttribute(key, value, scopedStyleIdPrefix) : null;

if (isNewKey) {
// Adding new key - splice into sorted position
if (serializedValue != null) {
(dstAttrs as any).splice(dstIdx, 0, key, serializedValue);
journal.push(VNodeJournalOpCode.SetAttribute, vnode.element, key, serializedValue);
}
} else {
// Updating or removing existing key at dstIdx
if (serializedValue != null) {
// Update existing value
(dstAttrs as any)[dstIdx + 1] = serializedValue;
journal.push(VNodeJournalOpCode.SetAttribute, vnode.element, key, serializedValue);
} else {
// Remove key (value is null)
dstAttrs.splice(dstIdx, 2);
journal.push(VNodeJournalOpCode.SetAttribute, vnode.element, key, null);
}
}
};

const record = (key: string, value: any) => {
const record = (key: string, value: any, dstIdx: number, isNewKey: boolean) => {
if (key.startsWith(':')) {
vnode.setProp(key, value);
return;
Expand Down Expand Up @@ -921,15 +950,21 @@ export const vnode_diff = (
}

if (isPromise(value)) {
// For async values, we can't use the known index since it will be stale by the time
// the promise resolves. Do a binary search to find the current index.
const vHost = vnode as ElementVNode;
const attributePromise = value.then((resolvedValue) =>
setAttribute(key, resolvedValue, vHost)
);
const attributePromise = value.then((resolvedValue) => {
const idx = mapApp_findIndx(dstAttrs, key, 0);
const isNewKey = idx < 0;
const currentDstIdx = isNewKey ? idx ^ -1 : idx;
setAttributeDirect(vHost, key, resolvedValue, currentDstIdx, isNewKey);
});
asyncAttributePromises.push(attributePromise);
return;
}

setAttribute(key, value, vnode);
// Always use optimized direct path - we know the index from the merge algorithm
setAttributeDirect(vnode, key, value, dstIdx, isNewKey);
};

const recordJsxEvent = (key: string, value: any) => {
Expand All @@ -938,7 +973,8 @@ export const vnode_diff = (
const [scope, eventName] = data;
const scopedEvent = getScopedEventName(scope, eventName);
const loaderScopedEvent = getLoaderScopedEventName(scope, scopedEvent);
record(':' + scopedEvent, value);
// Pass dummy index values since ':' prefixed keys take early return via setProp
record(':' + scopedEvent, value, 0, false);
// register an event for qwik loader (window/document prefixed with '-')
registerQwikLoaderEvent(loaderScopedEvent);
patchEventDispatch = true;
Expand All @@ -951,8 +987,8 @@ export const vnode_diff = (
const srcKey = srcIdx < srcAttrs.length ? (srcAttrs[srcIdx] as string) : undefined;
const dstKey = dstIdx < dstAttrs.length ? (dstAttrs[dstIdx] as string) : undefined;

// Skip special keys in destination (HANDLER_PREFIX, Q_PREFIX)
if (dstKey?.startsWith(HANDLER_PREFIX) || dstKey?.startsWith(Q_PREFIX)) {
// Skip special keys in destination HANDLER_PREFIX
if (dstKey?.startsWith(HANDLER_PREFIX)) {
dstIdx += 2; // skip key and value
continue;
}
Expand All @@ -963,7 +999,7 @@ export const vnode_diff = (
// HTML event attributes are immutable and not removed from DOM
dstIdx += 2; // skip key and value
} else {
record(dstKey!, null);
record(dstKey!, null, dstIdx, false);
// After removal, dstAttrs shrinks by 2, so don't advance dstIdx
}
} else if (dstKey === undefined) {
Expand All @@ -972,7 +1008,7 @@ export const vnode_diff = (
if (isHtmlAttributeAnEventName(srcKey)) {
recordJsxEvent(srcKey, srcValue);
} else {
record(srcKey, srcValue);
record(srcKey, srcValue, dstIdx, true);
}
srcIdx += 2; // skip key and value
// After addition, dstAttrs grows by 2 at sorted position, advance dstIdx
Expand All @@ -986,7 +1022,7 @@ export const vnode_diff = (
if (isEventHandler) {
recordJsxEvent(srcKey, srcValue);
} else {
record(srcKey, srcValue);
record(srcKey, srcValue, dstIdx, false);
}
} else if (isEventHandler && !vnode.element.qDispatchEvent) {
// Special case: add event handlers after resume
Expand All @@ -1001,7 +1037,7 @@ export const vnode_diff = (
if (isHtmlAttributeAnEventName(srcKey)) {
recordJsxEvent(srcKey, srcValue);
} else {
record(srcKey, srcValue);
record(srcKey, srcValue, dstIdx, true);
}
srcIdx += 2; // skip key and value
// After addition, dstAttrs grows at sorted position (before dstIdx), advance dstIdx
Expand All @@ -1012,7 +1048,7 @@ export const vnode_diff = (
// HTML event attributes are immutable and not removed from DOM
dstIdx += 2; // skip key and value
} else {
record(dstKey, null);
record(dstKey, null, dstIdx, false);
// After removal, dstAttrs shrinks at dstIdx, so don't advance dstIdx
}
}
Expand Down