fix: using special characters in attribute static strings

webtides · Nov 12, 2024 · c01c628 · c01c628
1 parent 4d309ee
commit c01c628
Show file tree

Hide file tree

Showing 14 changed files with 164 additions and 114 deletions.
diff --git a/examples/attributes/attributes-element.js b/examples/attributes/attributes-element.js
@@ -11,10 +11,12 @@ class AttributesElement extends TemplateElement {
 
     template() {
         const templateResult = html`
-            <div foo="bar" class="before:block before:content-[&apos;Festivus&apos;]">no interpolation</div>
-            <div foo="${'bar'}">single attribute interpolation</div>
-            <div foo="${'bar'}" bar="${'baz'}">multiple attribute interpolations</div>
-            <div foo="${'bar'} baz ${'foo'}">multiple interpolations in single attribute</div>
+            <div foo1="bar" class="before:block before:content-['Festivus']">no interpolation</div>
+            <div foo2="${'bar'}">single attribute interpolation</div>
+            <div foo3="${'bar'}" bar="${'baz'}">multiple attribute interpolations</div>
+            <div foo4="${'bar'} baz ${'foo'} before:content-['Festivus']">
+                multiple interpolations in single attribute
+            </div>
             <div ?true-boolean="${true}" ?false-boolean="${false}">boolean attributes</div>
             <div .data=${JSON.stringify({ foo: 'bar' })} .list="${JSON.stringify(['foo', 'bar'])}">
                 property attributes .data and .list

diff --git a/examples/child-nodes/child-nodes-element.js b/examples/child-nodes/child-nodes-element.js
@@ -0,0 +1,47 @@
+import { TemplateElement, defineElement, html, Directive, defineDirective } from '../../index.js';
+
+const directive = defineDirective(class extends Directive {});
+
+class ChildNodesElement extends TemplateElement {
+    properties() {
+        return {
+            text: 'Hello',
+            count: 3,
+        };
+    }
+
+    template() {
+        const list = [];
+        for (let i = 0; i < this.count; i++) {
+            list.push(i);
+        }
+
+        const p = document.createElement('p');
+        p.textContent = 'DOM Element';
+
+        const templateResult = html`
+            <div>no interpolation</div>
+            <div>${'single text interpolation'}</div>
+            <div>${'multiple'} text ${'interpolations'}</div>
+            <div>${html`template result interpolation with static text`}</div>
+            <div>${html`<div>template result interpolation with html</div>`}</div>
+            <ul>
+                ${list.map((item) => html`<li>${item}</li>`)}
+            </ul>
+            <div class="test">
+                ${[
+                    this.count,
+                    1,
+                    'Text',
+                    html`<span>Foo</span>`,
+                    html`<span>${this.count}</span>`,
+                    () => 'Function',
+                    p,
+                ]}
+            </div>
+        `;
+        console.log('templateResult', templateResult.toString());
+        return templateResult;
+    }
+}
+defineElement('attributes-element', ChildNodesElement);
diff --git a/examples/child-nodes/index.html b/examples/child-nodes/index.html
@@ -0,0 +1,13 @@
+<!doctype html>
+<html lang="en">
+    <head>
+        <meta charset="UTF-8" />
+        <script src="https://cdn.tailwindcss.com"></script>
+        <script type="module" src="child-nodes-element.js"></script>
+    </head>
+    <body>
+        <div class="container mx-auto p-8 border-4 rounded-lg m-8">
+            <attributes-element></attributes-element>
+        </div>
+    </body>
+</html>
diff --git a/src/dom-parts/AttributePart.js b/src/dom-parts/AttributePart.js
@@ -49,23 +49,15 @@ const processEventAttribute = (node, name) => {
  * @return {(function(*): void)|*}
  */
 const processAttribute = (node, name) => {
-    let oldValue,
-        orphan = true;
-    const attributeNode = globalThis.document?.createAttributeNS(null, name);
+    let oldValue;
     return (newValue) => {
         const value = newValue?.valueOf();
         if (oldValue !== value) {
-            if ((oldValue = value) == null) {
-                if (!orphan) {
-                    node.removeAttributeNode(attributeNode);
-                    orphan = true;
-                }
+            oldValue = value;
+            if (value == null) {
+                node.removeAttribute(name);
             } else {
-                attributeNode.value = value;
-                if (orphan) {
-                    node.setAttributeNodeNS(attributeNode);
-                    orphan = false;
-                }
+                node.setAttribute(name, value);
             }
         }
     };

diff --git a/src/dom-parts/TemplateResult.js b/src/dom-parts/TemplateResult.js
@@ -20,6 +20,11 @@ function makeSSRPlaceholder(type, params) {
     return `{{dom-part?${queryString}}}`;
 }
 
+function makePartParams(type, params) {
+    const urlSearchParams = new URLSearchParams({ type, ...params });
+    return urlSearchParams.toString();
+}
+
 /**
  * Given a template, find part positions as both nodes and attributes and
  * return a string with placeholders as either comment nodes or named attributes.
@@ -43,7 +48,8 @@ export const createTemplateString = (templateStrings, ssr = false) => {
                     attribute,
                     ssr ? makeSSRPlaceholder('directive') : '',
                 );
-                return `<!--\x02$-->`;
+                const params = makePartParams('directive');
+                return `<!--\x02?${params}-->`;
             }
 
             // remove quotes from attribute value to normalize the value
@@ -53,8 +59,11 @@ export const createTemplateString = (templateStrings, ssr = false) => {
             const partsCount = (attribute.match(/\x01/g) || []).length;
             const parts = [];
             for (let index = 0; index < partsCount; index++) {
-                // TODO: maybe we could use url parameters here?! -> ?type=attribute&name=${name}&initialValue=${initialValue}
-                parts.push(`<!--\x02:${name}=${value.replaceAll('\x01', '\x03')}-->`);
+                const params = makePartParams('attribute', {
+                    name,
+                    initialValue: value.replaceAll('\x01', '\x03'),
+                });
+                parts.push(`<!--\x02?${params}-->`);
             }
 
             if (parts.length > 0) {
@@ -110,19 +119,19 @@ export const createTemplateString = (templateStrings, ssr = false) => {
         const partsCount = (content.match(/\x01/g) || []).length;
         const parts = [];
         for (let index = 0; index < partsCount; index++) {
-            // TODO: maybe we could use url parameters here?! -> ?type=raw-text-node&initialValue=${initialValue}
-            parts.push(`<!--\x02/raw-text-node=${content.replaceAll('\x01', '\x03')}-->`);
+            const params = makePartParams('raw-text-node', {
+                initialValue: content.replaceAll('\x01', '\x03'),
+            });
+            parts.push(`<!--\x02?${params}-->`);
         }
         const parsedContent = content.replaceAll('\x01', ssr ? makeSSRPlaceholder('raw-text-node') : '');
         return `${parts.join('')}<${tag}${attributes}>${parsedContent}</${tag}>`;
     });
     // replace interpolation placeholders with our indexed markers
     template = template.replace(partPositions, (partPosition) => {
         if (partPosition === '\x01') {
-            if (ssr) {
-                return `<!--dom-part-${partIndex}-->${makeSSRPlaceholder('node')}<!--/dom-part-${partIndex++}-->`;
-            }
-            return `<!--dom-part-${partIndex}--><!--/dom-part-${partIndex++}-->`;
+            const placeholder = ssr ? makeSSRPlaceholder('node') : '';
+            return `<!--dom-part-${partIndex}-->${placeholder}<!--/dom-part-${partIndex++}-->`;
         } else if (partPosition === '\x02') {
             return `dom-part-${partIndex++}`;
         }
@@ -378,29 +387,32 @@ export class TemplateResult {
                 parts.push({ type: 'node', path: getNodePath(node) });
                 continue;
             }
-            if (/^dom-part-\d+:/.test(node.data)) {
-                const [_, ...attribute] = node.data.split(':');
-                const [name, ...initialValue] = attribute.join(':').split('=');
+            if (/^dom-part-\d+\?type=attribute&(.*?)$/.test(node.data)) {
+                // For html`<div id="${'foo'}" class="${'bar'}"></div>` we will get:
+                // <!--dom-part-0?type=attribute&name=id&initialValue=\x03--><!--dom-part-1?type=attribute&name=class&initialValue=\x03-->
+                const [_, paramsString] = node.data.split('?');
+                const searchParams = new URLSearchParams(paramsString);
                 parts.push({
                     type: 'attribute',
                     path: getNodePath(node),
-                    name: name,
-                    initialValue: initialValue.join('='),
+                    name: searchParams.get('name'),
+                    initialValue: searchParams.get('initialValue'),
                 });
                 continue;
             }
-            if (/^dom-part-\d+\/raw-text-node/.test(node.data)) {
+            if (/^dom-part-\d+\?type=raw-text-node&(.*?)$/.test(node.data)) {
                 // For html`<textarea>${'foo'} bar</textarea>` we will get:
-                // <!--dom-part-0/raw-text-node=\x03 bar-->
-                const [_, ...initialValue] = node.data.split('=');
+                // <!--dom-part-0?type=raw-text-node&initialValue=\x03 bar-->
+                const [_, paramsString] = node.data.split('?');
+                const searchParams = new URLSearchParams(paramsString);
                 parts.push({
                     type: 'raw-text-node',
                     path: getNodePath(node),
-                    initialValue: initialValue.join('='),
+                    initialValue: searchParams.get('initialValue'),
                 });
                 continue;
             }
-            if (/^dom-part-\d+\$/.test(node.data)) {
+            if (/^dom-part-\d+\?type=directive$/.test(node.data)) {
                 parts.push({ type: 'directive', path: getNodePath(node) });
             }
         }

diff --git a/src/util/AttributeParser.js b/src/util/AttributeParser.js
@@ -143,11 +143,13 @@ export function decodeAttribute(attribute) {
  * @returns {string}
  */
 export function encodeAttribute(attribute) {
-    return `${attribute}`
-        .replace(/'/g, '&apos;')
-        .replace(/"/g, '&quot;')
-        .replace(/</g, '&lt;')
-        .replace(/>/g, '&gt;')
-        .replace(/\r\n/g, '\n')
-        .replace(/[\r\n]/g, '\n');
+    return (
+        `${attribute}`
+            // .replace(/'/g, '&apos;')
+            .replace(/"/g, '&quot;')
+            .replace(/</g, '&lt;')
+            .replace(/>/g, '&gt;')
+            .replace(/\r\n/g, '\n')
+            .replace(/[\r\n]/g, '\n')
+    );
 }
diff --git a/test/unit/directive-optional-attribute.test.js b/test/unit/directive-optional-attribute.test.js
@@ -2,7 +2,7 @@
 import { assert } from '@open-wc/testing';
 import { optionalAttribute, OptionalAttributeDirective, spreadAttributes } from '../../src/dom-parts/directives.js';
 import { html } from '../../src/dom-parts/html.js';
-import { stripCommentMarkers } from './template-bindings.test.js';
+import { stripCommentMarkers, stripWhitespace } from '../util/testing-helpers.js';
 
 describe('optionalAttribute directive', () => {
     it('adds an attributes when condition is truthy', async () => {

diff --git a/test/unit/directive-spread-attributes.test.js b/test/unit/directive-spread-attributes.test.js
@@ -2,7 +2,7 @@
 import { assert } from '@open-wc/testing';
 import { html } from '../../src/TemplateElement.js';
 import { spreadAttributes, SpreadAttributesDirective } from '../../src/dom-parts/directives.js';
-import { stripCommentMarkers } from './template-bindings.test.js';
+import { stripCommentMarkers, stripWhitespace } from '../util/testing-helpers.js';
 
 describe('spreadAttributes directive', () => {
     it('maps primitive values to string attributes', async () => {

diff --git a/test/unit/directive-unsafe-html.test.js b/test/unit/directive-unsafe-html.test.js
@@ -3,7 +3,7 @@ import { fixture, defineCE, assert } from '@open-wc/testing';
 import { html, render } from '../../src/TemplateElement.js';
 import { unsafeHTML } from '../../src/dom-parts/directives.js';
 import { DOCUMENT_FRAGMENT_NODE, ELEMENT_NODE } from '../../src/util/DOMHelper.js';
-import { stripCommentMarkers } from './template-bindings.test.js';
+import { stripCommentMarkers, stripWhitespace } from '../util/testing-helpers.js';
 
 describe('unsafeHTML directive', () => {
     it('returns a function', async () => {

diff --git a/test/unit/raw-text-node-part.test.js b/test/unit/raw-text-node-part.test.js
@@ -1,14 +1,11 @@
 import { fixture, assert, nextFrame, oneEvent } from '@open-wc/testing';
 import { render } from '../../src/dom-parts/render.js';
 import { html } from '../../src/TemplateElement.js';
-import { stripCommentMarkers } from './template-bindings.test.js';
+import { stripCommentMarkers, stripWhitespace } from '../util/testing-helpers.js';
 import { RawTextNodePart } from '../../src/dom-parts/RawTextNodePart.js';
 import { convertStringToTemplate } from '../../src/util/DOMHelper.js';
 import { createTemplateString } from '../../src/dom-parts/TemplateResult.js';
 
-// TODO: this is copied and changed from template-result.test.js
-export const stripWhitespace = (html) => html.replace(/\s+/g, ' ').replaceAll('> <', '><').trim();
-
 describe(`RawTextNodePart class`, () => {
     it('stores a node to be processed from the comment marker node', async () => {
         const el = document.createElement('div');
@@ -36,7 +33,7 @@ describe(`RawTextNodePart template string parsing`, () => {
         const templateString = createTemplateString(templateResult.strings);
         assert.equal(
             stripWhitespace(templateString),
-            '<!--template-part--><!--dom-part-0/raw-text-node=\x03--><textarea></textarea><!--/template-part-->',
+            '<!--template-part--><!--dom-part-0?type=raw-text-node&initialValue=%03--><textarea></textarea><!--/template-part-->',
         );
     });
 
@@ -45,7 +42,7 @@ describe(`RawTextNodePart template string parsing`, () => {
         const templateString = createTemplateString(templateResult.strings);
         assert.equal(
             stripWhitespace(templateString),
-            '<!--template-part--><!--dom-part-0/raw-text-node=\x03 bar \x03--><!--dom-part-1/raw-text-node=\x03 bar \x03--><textarea> bar </textarea><!--/template-part-->',
+            '<!--template-part--><!--dom-part-0?type=raw-text-node&initialValue=%03+bar+%03--><!--dom-part-1?type=raw-text-node&initialValue=%03+bar+%03--><textarea> bar </textarea><!--/template-part-->',
         );
     });
 });
@@ -78,24 +75,22 @@ describe(`RawTextNodePart template bindings`, () => {
         const templateResult = html`<textarea>${content}</textarea>`;
         render(templateResult, el);
         assert.equal(stripCommentMarkers(el.innerHTML), '<textarea>foo</textarea>');
-        // TODO: make SSR work here as well
-        // assert.equal(
-        //     stripCommentMarkers(el.innerHTML),
-        //     stripCommentMarkers(templateResult.toString()),
-        //     'CSR template does not match SSR template',
-        // );
+        assert.equal(
+            stripCommentMarkers(el.innerHTML),
+            stripCommentMarkers(templateResult.toString()),
+            'CSR template does not match SSR template',
+        );
     });
 
     it('can have multiple interpolations inside a text only element', async () => {
         const el = document.createElement('div');
         const templateResult = html`<textarea>${'foo'} bar ${'baz'}</textarea>`;
         render(templateResult, el);
         assert.equal(stripCommentMarkers(el.innerHTML), '<textarea>foo bar baz</textarea>');
-        // TODO: make SSR work here as well
-        // assert.equal(
-        //     stripCommentMarkers(el.innerHTML),
-        //     stripCommentMarkers(templateResult.toString()),
-        //     'CSR template does not match SSR template',
-        // );
+        assert.equal(
+            stripCommentMarkers(el.innerHTML),
+            stripCommentMarkers(templateResult.toString()),
+            'CSR template does not match SSR template',
+        );
     });
 });
diff --git a/test/unit/ref-registration.test.js b/test/unit/ref-registration.test.js
@@ -1,6 +1,6 @@
 /* eslint-disable no-unused-expressions */
 import { fixture, defineCE, assert, nextFrame } from '@open-wc/testing';
-import { TemplateElement, html } from '../../src/TemplateElement';
+import { TemplateElement, html } from '../../src/TemplateElement.js';
 
 const elementTag = defineCE(
     class extends TemplateElement {