Merge pull request #7419 from QwikDev/v2-var-attribute-diff

fix: attribute diffing

Author: wmertens

.changeset/friendly-beers-heal.md

@@ -0,0 +1,5 @@
+---
+'@qwik.dev/core': patch
+---
+
+fix: attribute diffing was not working correctly in some edge cases

.gitignore

@@ -52,4 +52,7 @@ test-results
 sandbox
 
 # We need to ignore this because "changesets" will try to replace it
-/CHANGELOG.md
+/CHANGELOG.md
+
+# Vitest coverage
+packages/coverage/*

package.json

@@ -115,6 +115,7 @@
     "@types/which-pm-runs": "1.0.2",
     "@typescript-eslint/eslint-plugin": "7.16.1",
     "@typescript-eslint/parser": "7.16.1",
+    "@vitest/coverage-v8": "3.0.8",
     "all-contributors-cli": "6.26.1",
     "brotli": "1.3.3",
     "concurrently": "8.2.2",
@@ -155,7 +156,7 @@
     "vite-imagetools": "7.0.4",
     "vite-plugin-dts": "4.3.0",
     "vite-tsconfig-paths": "5.0.1",
-    "vitest": "2.1.4",
+    "vitest": "3.0.8",
     "watchlist": "0.3.1",
     "which-pm-runs": "1.1.0",
     "zod": "3.22.4"

packages/insights/package.json

@@ -35,7 +35,7 @@
     "undici": "*",
     "vite": "5.4.10",
     "vite-tsconfig-paths": "5.0.1",
-    "vitest": "2.1.4",
+    "vitest": "3.0.8",
     "zod": "3.22.4"
   },
   "engines": {

packages/qwik/package.json

@@ -18,7 +18,7 @@
     "kleur": "4.1.5",
     "prettier": "3.4.2",
     "ts-morph": "23.0.0",
-    "vitest": "2.1.4"
+    "vitest": "3.0.8"
   },
   "engines": {
     "node": "^18.17.0 || ^20.3.0 || >=21.0.0"

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

@@ -832,9 +832,9 @@ export const vnode_diff = (
         } else if (typeof value === 'function') {
           value(element);
           return;
-        } else if (value == null) {
-          return;
-        } else {
+        }
+        // handling null value is not needed here, because we are filtering null values earlier
+        else {
           throw qError(QError.invalidRefValue, [currentFile]);
         }
       }
@@ -847,7 +847,12 @@ export const vnode_diff = (
         value = trackSignalAndAssignHost(value, vnode, key, container, signalData);
       }
 
-      vnode_setAttr(journal, vnode, key, serializeAttribute(key, value, scopedStyleIdPrefix));
+      vnode_setAttr(
+        journal,
+        vnode,
+        key,
+        value !== null ? serializeAttribute(key, value, scopedStyleIdPrefix) : null
+      );
       if (value === null) {
         // if we set `null` than attribute was removed and we need to shorten the dstLength
         dstLength = dstAttrs.length;
@@ -883,7 +888,6 @@ export const vnode_diff = (
       } else if (srcKey == null) {
         // Source has more keys, so we need to remove them from destination
         if (dstKey && isHtmlAttributeAnEventName(dstKey)) {
-          patchEventDispatch = true;
           dstIdx++;
         } else {
           record(dstKey!, null);
@@ -930,6 +934,7 @@ export const vnode_diff = (
         // we need to increment dstIdx too, because we added destination key and value to the VNode
         // and dstAttrs is a reference to the VNode
         dstIdx++;
+        dstLength = dstAttrs.length;
         dstKey = dstIdx < dstLength ? dstAttrs[dstIdx++] : null;
       } else {
         // Source is missing the key, so we need to remove it from destination.
@@ -1444,7 +1449,7 @@ function markVNodeAsDeleted(vCursor: VNode) {
  * 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 = ':';
+export const HANDLER_PREFIX = ':';
 let count = 0;
 const enum SiblingsArray {
   Name = 0,