fix: correct event rendering and attaching

Author: Varixo

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

@@ -26,11 +26,10 @@ import { ChoreType } from '../shared/util-chore-type';
 import { escapeHTML } from '../shared/utils/character-escaping';
 import { _OWNER } from '../shared/utils/constants';
 import {
-  getEventNameFromJsxEvent,
+  getEventDataFromHtmlAttribute,
+  getEventNameFromHtmlAttribute,
   getEventNameScopeFromJsxEvent,
   isHtmlAttributeAnEventName,
-  isJsxPropertyAnEventName,
-  jsxEventToHtmlAttribute,
 } from '../shared/utils/event-names';
 import { getFileLocationFromJsx } from '../shared/utils/jsx-filename';
 import {
@@ -604,23 +603,22 @@ 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];
-        if (isJsxPropertyAnEventName(key)) {
-          // 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 = getEventNameFromJsxEvent(key);
-          const scope = getEventNameScopeFromJsxEvent(key);
-          if (eventName) {
-            vNewNode!.setProp(HANDLER_PREFIX + ':' + scope + ':' + eventName, value);
-            registerQwikLoaderEvent(eventName);
-          }
+        if (isHtmlAttributeAnEventName(key)) {
+          const data = getEventDataFromHtmlAttribute(key);
+          if (data) {
+            const scope = data[0];
+            const eventName = data[1];
+
+            if (eventName) {
+              vNewNode!.setProp(HANDLER_PREFIX + ':' + scope + ':' + eventName, value);
+              registerQwikLoaderEvent(eventName);
+            }
 
-          if (scope) {
-            // add an event attr with empty value for qwikloader element selector.
-            // We don't need value here. For ssr this value is a QRL,
-            // but for CSR value should be just empty
-            const htmlEvent = jsxEventToHtmlAttribute(key);
-            if (htmlEvent) {
-              vNewNode!.setAttr(htmlEvent, '', journal);
+            if (scope) {
+              // add an event attr with empty value for qwikloader element selector.
+              // We don't need value here. For ssr this value is a QRL,
+              // but for CSR value should be just empty
+              vNewNode!.setAttr(key, '', journal);
             }
           }
 
@@ -867,7 +865,7 @@ export const vnode_diff = (
     };
 
     const recordJsxEvent = (key: string, value: any) => {
-      const eventName = getEventNameFromJsxEvent(key);
+      const eventName = getEventNameFromHtmlAttribute(key);
       const scope = getEventNameScopeFromJsxEvent(key);
       if (eventName) {
         record(':' + scope + ':' + eventName, value);
@@ -879,10 +877,7 @@ export const vnode_diff = (
         // add an event attr with empty value for qwikloader element selector.
         // We don't need value here. For ssr this value is a QRL,
         // but for CSR value should be just empty
-        const htmlEvent = jsxEventToHtmlAttribute(key);
-        if (htmlEvent) {
-          record(htmlEvent, '');
-        }
+        record(key, '');
       }
     };
 
@@ -910,7 +905,7 @@ export const vnode_diff = (
       } else if (dstKey === undefined) {
         // Destination exhausted: add remaining source keys
         const srcValue = srcAttrs[srcIdx + 1];
-        if (isJsxPropertyAnEventName(srcKey)) {
+        if (isHtmlAttributeAnEventName(srcKey)) {
           patchEventDispatch = true;
           recordJsxEvent(srcKey, srcValue);
         } else {
@@ -932,7 +927,7 @@ export const vnode_diff = (
       } else if (srcKey < dstKey) {
         // Source has a key not in destination: add it
         const srcValue = srcAttrs[srcIdx + 1];
-        if (isJsxPropertyAnEventName(srcKey)) {
+        if (isHtmlAttributeAnEventName(srcKey)) {
           patchEventDispatch = true;
           recordJsxEvent(srcKey, srcValue);
         } else {

packages/qwik/src/core/shared/component-execution.ts

@@ -29,7 +29,7 @@ import { MAX_RETRY_ON_PROMISE_COUNT, isPromise, maybeThen, safeCall } from './ut
 import { isArray, isPrimitive, type ValueOrPromise } from './utils/types';
 import { getSubscriber } from '../reactive-primitives/subscriber';
 import { EffectProperty } from '../reactive-primitives/types';
-import { EventNameJSXScope } from './utils/event-names';
+import { EventNameHtmlScope } from './utils/event-names';
 
 /**
  * Use `executeComponent` to execute a component.
@@ -160,8 +160,8 @@ function addUseOnEvents(
           // if the component is headless, we need to add the event to the placeholder element
           if (
             key === qVisibleEvent ||
-            key.startsWith(EventNameJSXScope.document) ||
-            key.startsWith(EventNameJSXScope.window)
+            key.startsWith(EventNameHtmlScope.document) ||
+            key.startsWith(EventNameHtmlScope.window)
           ) {
             if (!placeholderElement) {
               const [createdElement, newJsx] = injectPlaceholderElement(jsxResult);

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

@@ -209,3 +209,15 @@ export function isPreventDefault(key: string): boolean {
 export const fromCamelToKebabCase = (text: string): string => {
   return text.replace(/([A-Z-])/g, '-$1').toLowerCase();
 };
+
+export const getEventDataFromHtmlAttribute = (htmlKey: string): [string, string] | null => {
+  const data = /^on(|-(window|document)):(.+)$/.exec(htmlKey);
+  return data
+    ? [
+        data[1]
+          // remove `-` from prefix
+          .substring(1),
+        data[3],
+      ]
+    : null;
+};

packages/qwik/src/core/ssr/ssr-render-jsx.ts

@@ -16,7 +16,11 @@ import type { QRL } from '../shared/qrl/qrl.public';
 import { qrlToString, type SerializationContext } from '../shared/serdes/index';
 import { DEBUG_TYPE, VirtualType } from '../shared/types';
 import { isAsyncGenerator } from '../shared/utils/async-generator';
-import { isHtmlAttributeAnEventName, isPreventDefault } from '../shared/utils/event-names';
+import {
+  getEventDataFromHtmlAttribute,
+  isHtmlAttributeAnEventName,
+  isPreventDefault,
+} from '../shared/utils/event-names';
 import { EMPTY_ARRAY } from '../shared/utils/flyweight';
 import { getFileLocationFromJsx } from '../shared/utils/jsx-filename';
 import {
@@ -435,9 +439,9 @@ function addQwikEventToSerializationContext(
   qrl: QRL
 ) {
   // TODO extract window/document too so qwikloader can precisely listen
-  const match = /^on(|-(window|document)):(.+)$/.exec(key);
-  if (match) {
-    const eventName = match[3];
+  const data = getEventDataFromHtmlAttribute(key);
+  if (data) {
+    const eventName = data[1];
     serializationCtx.$eventNames$.add(eventName);
     serializationCtx.$eventQrls$.add(qrl);
   }

packages/qwik/src/core/tests/use-visible-task.spec.tsx

@@ -15,7 +15,7 @@ import {
   useVisibleTask$,
 } from '@qwik.dev/core';
 import { domRender, ssrRenderToDom, trigger } from '@qwik.dev/core/testing';
-import { describe, expect, it, vi } from 'vitest';
+import { describe, expect, it } from 'vitest';
 import { ErrorProvider } from '../../testing/rendering.unit-util';
 import { delay } from '../shared/utils/promises';
 
@@ -295,25 +295,24 @@ describe.each([
   });
 
   it('should merge multiple visible tasks when empty', async () => {
-    const consoleSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
+    (globalThis as any).log = [] as string[];
     const Cmp = component$(() => {
       useVisibleTask$(
         () => {
-          console.warn('task1');
+          (globalThis as any).log.push('task1');
         },
         { strategy: 'document-ready' }
       );
       useVisibleTask$(() => {
-        console.warn('task2');
+        (globalThis as any).log.push('task2');
       });
       return <></>;
     });
     const { document } = await render(<Cmp />, { debug });
     if (render === ssrRenderToDom) {
       await trigger(document.body, 'script', ':document:qinit');
     }
-    consoleSpy.mockRestore();
-    expect(consoleSpy).toHaveBeenCalledTimes(3); // 2 tasks + warning about qvisible;
+    expect((globalThis as any).log).toEqual(['task1', 'task2']);
   });
 
   describe(render.name + ': track', () => {

packages/qwik/src/testing/element-fixture.ts

@@ -125,7 +125,6 @@ export async function trigger(
     await dispatch(element, attrName, event, scope);
   }
   const waitForQueueChore = container?.$scheduler$(ChoreType.WAIT_FOR_QUEUE);
-  await getTestPlatform().flush();
   if (waitForIdle && waitForQueueChore) {
     await waitForQueueChore.$returnValue$;
   }