fix(v2): process only nodes with props separator (#6608)

* fix(v2): process only nodes with props separator * fix(v2): processing only marked nodes
QwikDev · Jun 25, 2024 · 7840f17 · 7840f17
1 parent a223f6b
commit 7840f17
Show file tree

Hide file tree

Showing 12 changed files with 88 additions and 56 deletions.
diff --git a/packages/qwik/src/core/components/prefetch.unit.tsx b/packages/qwik/src/core/components/prefetch.unit.tsx
@@ -1,6 +1,7 @@
 import { describe, expect, it } from 'vitest';
 import { PrefetchServiceWorker, PrefetchGraph } from './prefetch';
 import { renderToString2 as renderToString } from '../../server/v2-ssr-render2';
+import { cleanupAttrs } from '../../testing/element-fixture';
 
 describe('PrefetchServiceWorker', () => {
   describe('render', () => {
@@ -12,9 +13,7 @@ describe('PrefetchServiceWorker', () => {
       const output = await renderToString(<PrefetchServiceWorker nonce="1234" />, {
         containerTagName: 'div',
       });
-      expect(output.html).to.contain(
-        '<script q:key="prefetch-service-worker" : q:container="html" nonce="1234">'
-      );
+      expect(cleanupAttrs(output.html)).to.contain('<script q:container="html" nonce="1234">');
     });
     it('should render script with a scope', async () => {
       const output = await renderToString(
@@ -103,18 +102,14 @@ describe('PrefetchGraph', () => {
       const output = await renderToString(<PrefetchGraph nonce="1234" />, {
         containerTagName: 'div',
       });
-      expect(output.html).to.contain(
-        '<script q:key="prefetch-graph" : q:container="html" nonce="1234">'
-      );
+      expect(cleanupAttrs(output.html)).to.contain('<script q:container="html" nonce="1234">');
     });
 
     it('should render with a nonce', async () => {
       const output = await renderToString(<PrefetchServiceWorker nonce="1234" />, {
         containerTagName: 'div',
       });
-      expect(output.html).to.contain(
-        '<script q:key="prefetch-service-worker" : q:container="html" nonce="1234">'
-      );
+      expect(cleanupAttrs(output.html)).to.contain('<script q:container="html" nonce="1234">');
     });
     it('should render script with a scope', async () => {
       const output = await renderToString(
@@ -203,9 +198,7 @@ describe('PrefetchGraph', () => {
       const output = await renderToString(<PrefetchGraph nonce="1234" />, {
         containerTagName: 'div',
       });
-      expect(output.html).to.contain(
-        '<script q:key="prefetch-graph" : q:container="html" nonce="1234">'
-      );
+      expect(cleanupAttrs(output.html)).to.contain('<script q:container="html" nonce="1234">');
     });
   });
 });
diff --git a/packages/qwik/src/core/v2/client/process-vnode-data.ts b/packages/qwik/src/core/v2/client/process-vnode-data.ts
@@ -49,6 +49,7 @@ import type { ContainerElement, ElementVNode, QDocument } from './types';
 export function processVNodeData(document: Document) {
   const Q_CONTAINER = 'q:container';
   const Q_CONTAINER_END = '/' + Q_CONTAINER;
+  const Q_PROPS_SEPARATOR = ':';
   const qDocument = document as QDocument;
   const vNodeDataMap =
     qDocument.qVNodeData || (qDocument.qVNodeData = new WeakMap<Element, string>());
@@ -66,6 +67,7 @@ export function processVNodeData(document: Document) {
     );
   };
   const getAttribute = prototype.getAttribute as (this: Node, name: string) => string | null;
+  const hasAttribute = prototype.hasAttribute as (this: Node, name: string) => boolean;
   const getNodeType = getter(prototype, 'nodeType') as (this: Node) => number;
 
   // Process all of the `qwik/vnode` script tags by attaching them to the corresponding containers.
@@ -98,7 +100,12 @@ export function processVNodeData(document: Document) {
     const nodeType = getNodeType.call(node);
     if (nodeType === 1 /* Node.ELEMENT_NODE */) {
       const qContainer = getAttribute.call(node, Q_CONTAINER);
-      return qContainer === null ? NodeType.ELEMENT : NodeType.ELEMENT_CONTAINER;
+      if (qContainer === null) {
+        const isQElement = hasAttribute.call(node, Q_PROPS_SEPARATOR);
+        return isQElement ? NodeType.ELEMENT : NodeType.OTHER;
+      } else {
+        return NodeType.ELEMENT_CONTAINER;
+      }
     } else if (nodeType === 8 /* Node.COMMENT_NODE */) {
       const nodeValue = node.nodeValue || ''; // nodeValue is monomorphic so it does not need fast path
       if (nodeValue.startsWith(Q_CONTAINER)) {

diff --git a/packages/qwik/src/core/v2/client/process-vnode-data.unit.tsx b/packages/qwik/src/core/v2/client/process-vnode-data.unit.tsx
@@ -10,8 +10,8 @@ describe('processVnodeData', () => {
   it('should parse simple case', () => {
     const [container] = process(`
       <html q:container="paused">
-        <head></head>
-        <body>
+        <head :></head>
+        <body :>
           HelloWorld
           ${encodeVNode({ 2: 'FF' })}
         </body>
@@ -30,10 +30,10 @@ describe('processVnodeData', () => {
   it('should ignore inner HTML', () => {
     const [container] = process(`
     <html q:container="paused">
-      <head></head>
-      <body>
+      <head :></head>
+      <body :>
         <div q:container="html"><span></span></div>
-        <b>HelloWorld</b>
+        <b :>HelloWorld</b>
         ${encodeVNode({ 2: '2', 4: 'FF' })}
     </body>
     </html>
@@ -51,18 +51,45 @@ describe('processVnodeData', () => {
       </html>
     );
   });
+
+  it('should ignore elements without `:`', async () => {
+    const [container] = process(`
+      <html q:container="paused">
+        <head :></head>
+        <body :>
+          <div q:container="html"><span></span></div>
+          <div>ignore this</div>
+          <b :>HelloWorld</b>
+          ${encodeVNode({ 2: '3', 4: 'FF' })}
+      </body>
+      </html>
+    `);
+    expect(container.rootVNode).toMatchVDOM(
+      <html {...qContainerPaused}>
+        <head />
+        <body>
+          <div dangerouslySetInnerHTML="<span></span>" {...qContainerHtml} />
+          <div>ignore this</div>
+          <b>
+            {'Hello'}
+            {'World'}
+          </b>
+        </body>
+      </html>
+    );
+  });
   describe('nested containers', () => {
     it('should parse', () => {
       const [container1, container2] = process(`
         <html q:container="paused">
-          <head></head>
-          <body>
+          <head :></head>
+          <body :>
             Before
             <div q:container="paused">
-              Foo<b>Bar!</b>
+              Foo<b :>Bar!</b>
               ${encodeVNode({ 0: 'D1', 1: 'DB' })}
             </div>
-            <b>After!</b>
+            <b :>After!</b>
             ${encodeVNode({ 2: 'G2', 4: 'FB' })}
           </body>
         </html>`);
@@ -91,15 +118,15 @@ describe('processVnodeData', () => {
     });
     it('should ignore comments and comment blocks', () => {
       const [container1] = process(`
-        <html q:container="paused">
-          <head></head>
-          <body>
+        <html q:container="paused" :>
+          <head :></head>
+          <body :>
             <!-- comment -->
             Before
             <!--q:container=some-id-->
               Foo<i>Bar!</i>
             <!--/q:container-->
-            <b>After!</b>
+            <b :>After!</b>
             ${encodeVNode({ 2: 'G1', 3: 'FB' })}
           </body>
         </html>`);
@@ -149,7 +176,7 @@ function emitVNodeSeparators(lastSerializedIdx: number, elementIdx: number): str
   let skipCount = elementIdx - lastSerializedIdx;
   // console.log('emitVNodeSeparators', lastSerializedIdx, elementIdx, skipCount);
   while (skipCount != 0) {
-    if (skipCount >= 4096) {
+    if (skipCount > 4096) {
       result += VNodeDataSeparator.ADVANCE_8192_CH;
       skipCount -= 8192;
     } else {

diff --git a/packages/qwik/src/core/v2/tests/component.spec.tsx b/packages/qwik/src/core/v2/tests/component.spec.tsx
@@ -13,7 +13,7 @@ import {
 } from '@builder.io/qwik';
 import { domRender, ssrRenderToDom } from '@builder.io/qwik/testing';
 import { describe, expect, it } from 'vitest';
-import { trigger } from '../../../testing/element-fixture';
+import { cleanupAttrs, trigger } from '../../../testing/element-fixture';
 import { ErrorProvider } from '../../../testing/rendering.unit-util';
 import type { JSXOutput } from '../../render/jsx/types/jsx-node';
 import { useSignal } from '../../use/use-signal';
@@ -452,7 +452,7 @@ describe.each([
         <a></a>
       </p>
     );
-    expect(document.querySelector('p')?.innerHTML).toEqual(
+    expect(cleanupAttrs(document.querySelector('p')?.innerHTML)).toEqual(
       '<b>Test</b>124xx<span>123</span>XXX<a></a>'
     );
   });
@@ -1563,7 +1563,7 @@ describe.each([
     await delay(10);
     // console.log('vNode', String(vNode));
     // console.log('>>>>', div.outerHTML);
-    expect(div.innerHTML).toEqual('<b>(</b>SomeErrorPOST<b>)</b>');
+    expect(cleanupAttrs(div.innerHTML)).toEqual('<b>(</b>SomeErrorPOST<b>)</b>');
   });
 
   describe('regression', () => {

diff --git a/packages/qwik/src/core/v2/tests/projection.spec.tsx b/packages/qwik/src/core/v2/tests/projection.spec.tsx
@@ -21,6 +21,7 @@ import {
 } from '@builder.io/qwik';
 import { vnode_getNextSibling } from '../client/vnode';
 import { HTML_NS, SVG_NS } from '../../util/markers';
+import { cleanupAttrs } from 'packages/qwik/src/testing/element-fixture';
 
 const debug = false;
 
@@ -1687,7 +1688,7 @@ describe.each([
         </Issue1630>,
         { debug }
       );
-      expect(removeKeyAttrs(document.querySelector('div')?.innerHTML || '')).toContain(
+      expect(cleanupAttrs(document.querySelector('div')?.innerHTML || '')).toContain(
         '</p><b>CHILD</b>DYNAMIC'
       );
       await trigger(document.body, 'button', 'click');
@@ -1702,7 +1703,7 @@ describe.each([
           </div>
         </Component>
       );
-      expect(removeKeyAttrs(document.querySelector('div')?.innerHTML || '')).not.toContain(
+      expect(cleanupAttrs(document.querySelector('div')?.innerHTML || '')).not.toContain(
         '<b>CHILD</b>DYNAMIC'
       );
       await trigger(document.body, 'button', 'click');
@@ -1722,7 +1723,7 @@ describe.each([
           </div>
         </Component>
       );
-      expect(removeKeyAttrs(document.querySelector('div')?.innerHTML || '')).toContain(
+      expect(cleanupAttrs(document.querySelector('div')?.innerHTML || '')).toContain(
         '</p><b>CHILD</b>DYNAMIC'
       );
     });
@@ -2225,7 +2226,3 @@ describe.each([
     });
   });
 });
-
-function removeKeyAttrs(innerHTML: string): any {
-  return innerHTML.replaceAll(/ q:key="[^"]+"/g, '');
-}
diff --git a/packages/qwik/src/core/v2/tests/render-api.spec.tsx b/packages/qwik/src/core/v2/tests/render-api.spec.tsx
@@ -35,6 +35,7 @@ import { renderToStream2, renderToString2 } from '../../../server/v2-ssr-render2
 import { _fnSignal } from '../../internal';
 import { render2 } from '../client/dom-render';
 import { vnode_getFirstChild } from '../client/vnode';
+import { cleanupAttrs } from 'packages/qwik/src/testing/element-fixture';
 
 vi.hoisted(() => {
   vi.stubGlobal('QWIK_LOADER_DEFAULT_MINIFIED', 'min');
@@ -825,15 +826,15 @@ describe('render api', () => {
           containerTagName: 'div',
           debug: true,
         });
-        expect(result.html).toContain('<script id="qwikloader">debug</script>');
+        expect(cleanupAttrs(result.html)).toContain('<script id="qwikloader">debug</script>');
       });
 
       it('should emit qwik loader without debug mode', async () => {
         const result = await renderToStringAndSetPlatform(<Counter />, {
           containerTagName: 'div',
           debug: false,
         });
-        expect(result.html).toContain('<script id="qwikloader">min</script>');
+        expect(cleanupAttrs(result.html)).toContain('<script id="qwikloader">min</script>');
       });
     });
     describe('snapshotResult', () => {

diff --git a/packages/qwik/src/core/v2/tests/ssr-render.spec.tsx b/packages/qwik/src/core/v2/tests/ssr-render.spec.tsx
@@ -176,8 +176,10 @@ describe('v2 ssr render', () => {
         </div>
       );
       // we should not stream the comment nodes of the SSRStreamBlock
-      expect(document.querySelector('#stream-block')?.innerHTML).toEqual(
-        '<div>stream content</div>'
+      expect(document.querySelector('#stream-block')).toMatchDOM(
+        <div id="stream-block">
+          <div>stream content</div>
+        </div>
       );
     });
 

diff --git a/packages/qwik/src/core/v2/tests/use-styles-scoped.spec.tsx b/packages/qwik/src/core/v2/tests/use-styles-scoped.spec.tsx
@@ -8,7 +8,7 @@ import { createDocument } from '@builder.io/qwik-dom';
 import { afterEach, describe, expect, it } from 'vitest';
 import { useStore } from '../..';
 import { renderToString2 } from '../../../server/v2-ssr-render2';
-import { trigger } from '../../../testing/element-fixture';
+import { cleanupAttrs, trigger } from '../../../testing/element-fixture';
 import { domRender, ssrRenderToDom } from '../../../testing/rendering.unit-util';
 import '../../../testing/vdom-diff.unit-util';
 import { component$ } from '../../component/component.public';
@@ -123,7 +123,9 @@ describe.each([
       </>
     );
     const style = container.document.querySelector(QStyleSelector);
-    expect(style?.outerHTML).toEqual(`<style q:style="${styleId}">${scopeStyle}</style>`);
+    expect(cleanupAttrs(style?.outerHTML)).toEqual(
+      `<style q:style="${styleId}">${scopeStyle}</style>`
+    );
   });
 
   it('should save styles when JSX deleted', async () => {
@@ -154,7 +156,9 @@ describe.each([
       </Component>
     );
     const style = container.document.querySelector(QStyleSelector);
-    expect(style?.outerHTML).toEqual(`<style q:style="${styleId}">${scopeStyle}</style>`);
+    expect(cleanupAttrs(style?.outerHTML)).toEqual(
+      `<style q:style="${styleId}">${scopeStyle}</style>`
+    );
   });
 
   it('style node should contain q:style attribute', async () => {
@@ -252,7 +256,7 @@ describe.each([
     );
     const qStyles = container.document.querySelectorAll(QStyleSelector);
     expect(qStyles).toHaveLength(2);
-    expect(Array.from(qStyles).map((style) => style.outerHTML)).toEqual(
+    expect(Array.from(qStyles).map((style) => cleanupAttrs(style.outerHTML))).toEqual(
       expect.arrayContaining([
         `<style q:style="${firstStyleId}">${firstScopeStyle}</style>`,
         `<style q:style="${secondStyleId}">${secondScopeStyle}</style>`,

diff --git a/packages/qwik/src/core/v2/tests/use-styles.spec.tsx b/packages/qwik/src/core/v2/tests/use-styles.spec.tsx
@@ -71,9 +71,8 @@ describe.each([
       </>
     );
     const style = container.document.querySelector(QStyleSelector);
-    expect(style?.outerHTML).toEqual(
-      `<style q:style="${(globalThis as any).rawStyleId}">${STYLE_RED}</style>`
-    );
+    const attrs = { 'q:style': (globalThis as any).rawStyleId };
+    expect(style).toMatchDOM(<style {...attrs}>{STYLE_RED}</style>);
   });
 
   it('should save styles when JSX deleted', async () => {
@@ -101,9 +100,8 @@ describe.each([
       </Component>
     );
     const style = container.document.querySelector(QStyleSelector);
-    expect(style?.outerHTML).toEqual(
-      `<style q:style="${(globalThis as any).rawStyleId}">${STYLE_RED}</style>`
-    );
+    const attrs = { 'q:style': (globalThis as any).rawStyleId };
+    expect(style).toMatchDOM(<style {...attrs}>{STYLE_RED}</style>);
   });
 
   it('style node should contain q:style attribute', async () => {

diff --git a/packages/qwik/src/server/v2-ssr-container.ts b/packages/qwik/src/server/v2-ssr-container.ts
@@ -344,11 +344,10 @@ class SSRContainer extends _SharedContainer implements ISSRContainer {
     if (varAttrs) {
       innerHTML = this.writeAttrs(elementName, varAttrs, false);
     }
+    this.write(' :');
+    // Domino sometimes does not like empty attributes, so we need to add a empty value
+    isDev && this.write('=""');
     if (constAttrs && constAttrs.length) {
-      // we have to skip the `ref` prop, so we don't need `:` if there is only this `ref` prop
-      if (constAttrs[0] !== 'ref') {
-        this.write(' :');
-      }
       innerHTML = this.writeAttrs(elementName, constAttrs, true) || innerHTML;
     }
     this.write('>');

diff --git a/packages/qwik/src/testing/element-fixture.ts b/packages/qwik/src/testing/element-fixture.ts
@@ -213,3 +213,7 @@ export async function advanceToNextTimerAndFlush() {
   vi.advanceTimersToNextTimer();
   await getTestPlatform().flush();
 }
+
+export function cleanupAttrs(innerHTML: string | undefined): any {
+  return innerHTML?.replaceAll(/ q:key="[^"]+"/g, '').replaceAll(/ :=""/g, '');
+}
diff --git a/starters/e2e/e2e.lexical-scope.e2e.ts b/starters/e2e/e2e.lexical-scope.e2e.ts
@@ -13,7 +13,7 @@ test.describe("lexical-scope", () => {
 
   test("should rerender without changes", async ({ page }) => {
     const SNAPSHOT =
-      '<p>1</p><p>"&lt;/script&gt;"</p><p>{"a":{"thing":12},"b":"hola","c":123,"d":false,"e":true,"f":null,"h":[1,"string",false,{"hola":1},["hello"]],"promise":{}}</p><p>undefined</p><p>null</p><p>[1,2,"hola",null,{}]</p><p>true</p><p>false</p><p>()=&gt;console.error()</p><p>mutable message</p><p>{"signal":{"value":0},"signalValue":0,"store":{"count":0,"signal":{"value":0}},"storeCount":0,"storeSignal":{"value":0}}</p><p>from a promise</p><p>message, message2, signal, signalValue, store, storeCount, storeSignal</p>';
+      '<p :="">1</p><p :="">"&lt;/script&gt;"</p><p :="">{"a":{"thing":12},"b":"hola","c":123,"d":false,"e":true,"f":null,"h":[1,"string",false,{"hola":1},["hello"]],"promise":{}}</p><p :="">undefined</p><p :="">null</p><p :="">[1,2,"hola",null,{}]</p><p :="">true</p><p :="">false</p><p :="">()=&gt;console.error()</p><p :="">mutable message</p><p :="">{"signal":{"value":0},"signalValue":0,"store":{"count":0,"signal":{"value":0}},"storeCount":0,"storeSignal":{"value":0}}</p><p :="">from a promise</p><p :="">message, message2, signal, signalValue, store, storeCount, storeSignal</p>';
     const RESULT =
       '[1,"</script>",{"a":{"thing":12},"b":"hola","c":123,"d":false,"e":true,"f":null,"h":[1,"string",false,{"hola":1},["hello"]],"promise":{}},"undefined","null",[1,2,"hola",null,{}],true,false,null,"mutable message",null,{"value":0},0,{"count":0,"signal":{"value":0}},0,{"value":0},"from a promise","http://qwik.builder.com/docs?query=true","2022-07-26T17:40:30.255Z","hola()\\\\/ gi",12,"failed message",["\\b: backspace","\\f: form feed","\\n: line feed","\\r: carriage return","\\t: horizontal tab","\\u000b: vertical tab","\\u0000: null character","\': single quote","\\\\: backslash"],"Infinity","-Infinity","NaN","88","qwik",["1","2"],"200000000000000000","[\\"hola\\",12,{\\"a\\":\\"2022-07-26T17:40:30.255Z\\"}]","[[{},{}],[\\"mapkey\\",\\"http://qwik.builder.com/docs?query=true\\"]]"]';