chore: add flags to numeric prop key

Author: Varixo

packages/qwik/src/core/client/vnode-diff.ts

@@ -28,17 +28,14 @@ import {
   QSlotParent,
   QBackRefs,
   QTemplate,
-  Q_PREFIX,
-  dangerouslySetInnerHTML,
+  HANDLER_PREFIX,
 } from '../shared/utils/markers';
 import { isPromise } from '../shared/utils/promises';
 import { type ValueOrPromise } from '../shared/utils/types';
 import {
   convertEventNameFromJsxPropToHtmlAttr,
   getEventNameFromJsxProp,
   getEventNameScopeFromJsxProp,
-  isHtmlAttributeAnEventName,
-  isJsxPropertyAnEventName,
 } from '../shared/utils/event-names';
 import { ChoreType } from '../shared/util-chore-type';
 import { hasClassAttr } from '../shared/utils/scoped-styles';
@@ -92,9 +89,13 @@ import { EffectProperty, isSignal, SubscriptionData } from '../signal/signal';
 import type { Signal } from '../signal/signal.public';
 import { executeComponent } from '../shared/component-execution';
 import {
+  StaticPropId,
   getPropId,
   getPropName,
   getSlotName,
+  isEventProp,
+  isHandlerProp,
+  isQProp,
   isSlotProp,
   type NumericPropKey,
 } from '../shared/utils/prop';
@@ -618,9 +619,9 @@ export const vnode_diff = (
       // We never tell the vNode about them saving us time and memory.
       for (const key in constProps) {
         let value = constProps[key as unknown as NumericPropKey];
-        const numericKey = key as unknown as NumericPropKey;
+        const numericKey = Number(key) as NumericPropKey;
         const nameKey = getPropName(numericKey);
-        if (isJsxPropertyAnEventName(nameKey)) {
+        if (isEventProp(numericKey)) {
           // So for event handlers we must add them to the vNode so that qwikloader can look them up
           // But we need to mark them so that they don't get pulled into the diff.
           const eventName = getEventNameFromJsxProp(nameKey);
@@ -648,7 +649,7 @@ export const vnode_diff = (
           continue;
         }
 
-        if (nameKey === 'ref') {
+        if (numericKey === StaticPropId.REF) {
           if (isSignal(value)) {
             value.value = element;
             continue;
@@ -676,13 +677,13 @@ export const vnode_diff = (
           );
         }
 
-        if (nameKey === dangerouslySetInnerHTML) {
+        if (numericKey === StaticPropId.INNERHTML) {
           element.innerHTML = value as string;
           element.setAttribute(QContainerAttr, QContainerValue.HTML);
           continue;
         }
 
-        if (elementName === 'textarea' && nameKey === 'value') {
+        if (elementName === 'textarea' && numericKey === StaticPropId.VALUE) {
           if (value && typeof value !== 'string') {
             if (isDev) {
               throw qError(QError.wrongTextareaValue, [currentFile, value]);
@@ -764,11 +765,11 @@ export const vnode_diff = (
     for (const key in props) {
       const value = props[key as unknown as NumericPropKey];
       if (value != null) {
-        mapArray_set(jsxAttrs, key as unknown as NumericPropKey, value, 0);
+        mapArray_set(jsxAttrs, Number(key) as NumericPropKey, value, 0);
       }
     }
     if (jsxKey !== null) {
-      mapArray_set(jsxAttrs, getPropId(ELEMENT_KEY), jsxKey, 0);
+      mapArray_set(jsxAttrs, StaticPropId.ELEMENT_KEY as NumericPropKey, jsxKey, 0);
     }
     const vNode = (vNewNode || vCurrent) as ElementVNode;
     needsQDispatchEventPatch =
@@ -820,12 +821,12 @@ export const vnode_diff = (
 
     const record = (key: NumericPropKey, value: any) => {
       const keyName = getPropName(key);
-      if (keyName.startsWith(':')) {
+      if (isHandlerProp(key)) {
         vnode_setProp(vnode, keyName, value);
         return;
       }
 
-      if (keyName === 'ref') {
+      if (key === StaticPropId.REF) {
         const element = vnode_getNode(vnode) as Element;
         if (isSignal(value)) {
           value.value = element;
@@ -882,17 +883,14 @@ export const vnode_diff = (
     };
 
     while (srcKey !== null || dstKey !== null) {
-      if (
-        (dstKey && getPropName(dstKey).startsWith(HANDLER_PREFIX)) ||
-        (dstKey && getPropName(dstKey).startsWith(Q_PREFIX))
-      ) {
+      if ((dstKey && isHandlerProp(dstKey)) || (dstKey && isQProp(dstKey))) {
         // These are a special keys which we use to mark the event handlers as immutable or
         // element key we need to ignore them.
         dstIdx++; // skip the destination value, we don't care about it.
         dstKey = dstIdx < dstLength ? dstAttrs[dstIdx++] : null;
       } else if (srcKey == null) {
         // Source has more keys, so we need to remove them from destination
-        if (dstKey && isHtmlAttributeAnEventName(getPropName(dstKey))) {
+        if (dstKey && isEventProp(dstKey)) {
           patchEventDispatch = true;
           dstIdx++;
         } else {
@@ -902,7 +900,7 @@ export const vnode_diff = (
         dstKey = dstIdx < dstLength ? dstAttrs[dstIdx++] : null;
       } else if (dstKey == null) {
         // Destination has more keys, so we need to insert them from source.
-        const isEvent = isJsxPropertyAnEventName(getPropName(srcKey));
+        const isEvent = isEventProp(srcKey);
         if (isEvent) {
           // Special handling for events
           patchEventDispatch = true;
@@ -926,7 +924,7 @@ export const vnode_diff = (
         dstKey = dstIdx < dstLength ? dstAttrs[dstIdx++] : null;
       } else if (srcKey < dstKey) {
         // Destination is missing the key, so we need to insert it.
-        if (isJsxPropertyAnEventName(getPropName(srcKey))) {
+        if (isEventProp(srcKey)) {
           // Special handling for events
           patchEventDispatch = true;
           recordJsxEvent(srcKey, srcAttrs[srcIdx]);
@@ -943,7 +941,7 @@ export const vnode_diff = (
         dstKey = dstIdx < dstLength ? dstAttrs[dstIdx++] : null;
       } else {
         // Source is missing the key, so we need to remove it from destination.
-        if (isHtmlAttributeAnEventName(getPropName(dstKey))) {
+        if (isEventProp(dstKey)) {
           patchEventDispatch = true;
           dstIdx++;
         } else {
@@ -1219,7 +1217,15 @@ function getKey(vNode: VNode | null): string | null {
   if (vNode == null) {
     return null;
   }
-  return vnode_getProp<string>(vNode, ELEMENT_KEY, null);
+  const type = vNode[VNodeProps.flags];
+  if ((type & VNodeFlags.ELEMENT_OR_VIRTUAL_MASK) !== 0) {
+    const props = vnode_getProps(vNode);
+    // this works, because q:key is always at 0 position or it is not present
+    if (props[0] === StaticPropId.ELEMENT_KEY) {
+      return props[1] as string | null;
+    }
+  }
+  return null;
 }
 
 /**
@@ -1450,11 +1456,6 @@ function markVNodeAsDeleted(vCursor: VNode) {
   vCursor[VNodeProps.flags] |= VNodeFlags.Deleted;
 }
 
-/**
- * This marks the property as immutable. It is needed for the QRLs so that QwikLoader can get a hold
- * of them. This character must be `:` so that the `vnode_getAttr` can ignore them.
- */
-const HANDLER_PREFIX = ':';
 let count = 0;
 const enum SiblingsArray {
   Name = 0,

packages/qwik/src/core/shared/utils/event-names.ts

@@ -125,6 +125,7 @@ export const convertEventNameFromHtmlAttrToJsxProp = (name: string): string | nu
   return null;
 };
 
+const DOMContentLoadedEventLowercase = DOMContentLoadedEvent.toLowerCase();
 export const createEventName = (
   event: KnownEventNames | KnownEventNames[],
   eventType?: 'window' | 'document' | undefined
@@ -133,7 +134,7 @@ export const createEventName = (
   const map = (name: string) => {
     // DOMContentLoaded is a special case, where the event name is case sensitive
     // https://html.spec.whatwg.org/multipage/indices.html#event-domcontentloaded
-    const isDOMContentLoadedEvent = name === DOMContentLoadedEvent;
+    const isDOMContentLoadedEvent = name.toLowerCase() === DOMContentLoadedEventLowercase;
     const eventName = isDOMContentLoadedEvent
       ? DOMContentLoadedEvent
       : name.charAt(0).toUpperCase() + name.substring(1).toLowerCase();

packages/qwik/src/core/shared/utils/markers.ts

@@ -78,6 +78,12 @@ export const ELEMENT_SEQ = 'q:seq';
 export const ELEMENT_SEQ_IDX = 'q:seqIdx';
 export const Q_PREFIX = 'q:';
 
+/**
+ * This marks the property as immutable. It is needed for the QRLs so that QwikLoader can get a hold
+ * of them. This character must be `:` so that the `vnode_getAttr` can ignore them.
+ */
+export const HANDLER_PREFIX = ':';
+
 /** Non serializable markers - always begins with `:` character */
 export const NON_SERIALIZABLE_MARKER_PREFIX = ':';
 export const USE_ON_LOCAL = NON_SERIALIZABLE_MARKER_PREFIX + 'on';
@@ -92,5 +98,6 @@ export const STREAM_BLOCK_END_COMMENT = 'qkssr-po';
 export const Q_PROPS_SEPARATOR = ':';
 
 export const dangerouslySetInnerHTML = 'dangerouslySetInnerHTML';
+export const refAttr = 'ref';
 export const qwikInspectorAttr = 'data-qwik-inspector';
 export const DOMContentLoadedEvent = 'DOMContentLoaded';

packages/qwik/src/core/shared/utils/prop.ts

@@ -10,29 +10,84 @@ import type { JSXNodeInternal } from '../jsx/types/jsx-node';
 import type { KnownEventNames } from '../jsx/types/jsx-qwik-events';
 import type { Container, HostElement } from '../types';
 import { _CONST_PROPS, _VAR_PROPS } from './constants';
-import { createEventName, parseEventNameFromIndex, isJsxPropertyAnEventName } from './event-names';
-import { NON_SERIALIZABLE_MARKER_PREFIX, QDefaultSlot, Q_PREFIX } from './markers';
+import {
+  createEventName,
+  parseEventNameFromIndex,
+  isJsxPropertyAnEventName,
+  isHtmlAttributeAnEventName,
+} from './event-names';
+import {
+  ELEMENT_ID,
+  ELEMENT_KEY,
+  ELEMENT_PROPS,
+  HANDLER_PREFIX,
+  NON_SERIALIZABLE_MARKER_PREFIX,
+  OnRenderProp,
+  QDefaultSlot,
+  Q_PREFIX,
+  dangerouslySetInnerHTML,
+  refAttr,
+} from './markers';
 
 const propNameToId = new Map<string | symbol, NumericPropKey>();
-export const idToPropName: (string | symbol)[] = [null!];
+const idToPropName: (string | symbol)[] = [];
 export type NumericPropKey = number & { __brand__: 'NumericPropKey' };
 
 const colonOnLength = ':on'.length;
 
+export const enum NumericPropKeyFlags {
+  EVENT = 1,
+  Q_PREFIX = 2,
+  HANDLER_PREFIX = 4,
+  SLOT = 8,
+}
+
+export const NumericFlagsShift = 4;
+
 export const getPropId = (name: string | symbol): NumericPropKey => {
   let id = propNameToId.get(name);
-  if (id) {
+  if (id != null) {
     return id;
   }
-  id = idToPropName.length as NumericPropKey;
-  if (typeof name === 'string' && isJsxPropertyAnEventName(name)) {
-    name = normalizeEvent(name);
+  id = (idToPropName.length << NumericFlagsShift) as NumericPropKey;
+  if (typeof name === 'string') {
+    if (isJsxPropertyAnEventName(name)) {
+      name = normalizeEvent(name);
+      (id as number) |= NumericPropKeyFlags.EVENT;
+    } else if (isHtmlAttributeAnEventName(name)) {
+      (id as number) |= NumericPropKeyFlags.EVENT;
+    } else if (name.startsWith(Q_PREFIX)) {
+      (id as number) |= NumericPropKeyFlags.Q_PREFIX;
+    } else if (name.startsWith(HANDLER_PREFIX)) {
+      (id as number) |= NumericPropKeyFlags.HANDLER_PREFIX;
+    }
+
+    if (!name.startsWith(Q_PREFIX) && !name.startsWith(NON_SERIALIZABLE_MARKER_PREFIX)) {
+      (id as number) |= NumericPropKeyFlags.SLOT;
+    }
   }
   idToPropName.push(name);
   propNameToId.set(name, id);
   return id;
 };
 
+export const StaticPropId = {
+  // ELEMENT_KEY should be always first, because of `getKey` in vnode_diff.ts
+  ELEMENT_KEY: getPropId(ELEMENT_KEY),
+  ELEMENT_ID: getPropId(ELEMENT_ID),
+  ELEMENT_PROPS: getPropId(ELEMENT_PROPS),
+  REF: getPropId(refAttr),
+  INNERHTML: getPropId(dangerouslySetInnerHTML),
+  VALUE: getPropId('value'),
+  ON_RENDER: getPropId(OnRenderProp),
+  CLASS: getPropId('class'),
+  CLASS_NAME: getPropId('classname'),
+};
+
+export const getPropName = <T extends string>(id: NumericPropKey): T => {
+  return idToPropName[id >> NumericFlagsShift] as T;
+};
+
 function normalizeEvent(name: string): string {
   const index = name.indexOf(':on');
   const scope = (name.substring(0, index) || undefined) as 'window' | 'document' | undefined;
@@ -41,13 +96,24 @@ function normalizeEvent(name: string): string {
   return name;
 }
 
-export const getPropName = <T extends string>(id: NumericPropKey): T => {
-  return idToPropName[id] as T;
-};
+function getFlags(id: number) {
+  return ((1 << NumericFlagsShift) - 1) & (id >> 0);
+}
+
+export function isEventProp(numericProp: NumericPropKey): boolean {
+  return (getFlags(numericProp) & NumericPropKeyFlags.EVENT) !== 0;
+}
+
+export function isQProp(numericProp: NumericPropKey): boolean {
+  return (getFlags(numericProp) & NumericPropKeyFlags.Q_PREFIX) !== 0;
+}
+
+export function isHandlerProp(numericProp: NumericPropKey): boolean {
+  return (getFlags(numericProp) & NumericPropKeyFlags.HANDLER_PREFIX) !== 0;
+}
 
 export function isSlotProp(numericProp: NumericPropKey): boolean {
-  const prop = idToPropName[numericProp] as string;
-  return !prop.startsWith(Q_PREFIX) && !prop.startsWith(NON_SERIALIZABLE_MARKER_PREFIX);
+  return (getFlags(numericProp) & NumericPropKeyFlags.SLOT) !== 0;
 }
 
 export function getSlotName(
@@ -88,3 +154,8 @@ export const _restProps = (props: PropsProxy, omit: string[], target: Props = {}
 
   return createPropsProxy(varPropsTarget, constPropsTarget);
 };
+
+export const __testing__ = {
+  propNameToId,
+  idToPropName,
+};

packages/qwik/src/core/shared/utils/prop.unit.ts

@@ -0,0 +1,34 @@
+import { beforeEach, describe, expect, it } from 'vitest';
+import * as propFile from './prop';
+
+describe('prop', () => {
+  beforeEach(() => {
+    propFile.__testing__.propNameToId.clear();
+    propFile.__testing__.idToPropName.length = 0;
+  });
+
+  describe('getPropId', () => {
+    it('adding props to map', async () => {
+      expect(propFile.getPropName(propFile.getPropId('firstProp'))).toBe('firstProp');
+      expect(propFile.getPropName(propFile.getPropId('secondProp'))).toBe('secondProp');
+    });
+
+    it('adding event', async () => {
+      expect(propFile.getPropName(propFile.getPropId('onClick$'))).toBe('onClick$');
+      expect(propFile.getPropName(propFile.getPropId('window:onClick$'))).toBe('window:onClick$');
+      expect(propFile.getPropName(propFile.getPropId('document:onClick$'))).toBe(
+        'document:onClick$'
+      );
+      expect(propFile.getPropName(propFile.getPropId('onDblClick$'))).toBe('onDblclick$');
+      expect(propFile.getPropName(propFile.getPropId('window:onDblClick$'))).toBe(
+        'window:onDblclick$'
+      );
+      expect(propFile.getPropName(propFile.getPropId('document:onDblClick$'))).toBe(
+        'document:onDblclick$'
+      );
+      expect(propFile.getPropName(propFile.getPropId('document:onDOMContentLoaded$'))).toBe(
+        'document:onDOMContentLoaded$'
+      );
+    });
+  });
+});