[lit-html] Simplify lit-html attribute handling (#3751)

lit · Mar 24, 2023 · dfd747c · dfd747c
1 parent 7d49a0f
commit dfd747c
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 51 deletions.
diff --git a/.changeset/blue-glasses-tickle.md b/.changeset/blue-glasses-tickle.md
@@ -0,0 +1,7 @@
+---
+'lit-html': major
+'lit': major
+'lit-element': major
+---
+
+Simplify lit-html attribute handling for standards-compliant browsers that iterate attributes in source order
diff --git a/packages/lit-html/src/lit-html.ts b/packages/lit-html/src/lit-html.ts
@@ -667,7 +667,7 @@ export interface DirectiveParent {
 const getTemplateHtml = (
   strings: TemplateStringsArray,
   type: ResultType
-): [TrustedHTML, Array<string | undefined>] => {
+): [TrustedHTML, Array<string>] => {
   // Insert makers into the template HTML to represent the position of
   // bindings. The following code scans the template strings to determine the
   // syntactic position of the bindings. They can be in text position, where
@@ -678,7 +678,7 @@ const getTemplateHtml = (
   // Stores the case-sensitive bound attribute names in the order of their
   // parts. ElementParts are also reflected in this array as undefined
   // rather than a string, to disambiguate from attribute bindings.
-  const attrNames: Array<string | undefined> = [];
+  const attrNames: Array<string> = [];
   let html = type === SVG_RESULT ? '<svg>' : '';
 
   // When we're inside a raw text tag (not it's text content), the regex
@@ -808,9 +808,7 @@ const getTemplateHtml = (
             s.slice(attrNameEndIndex)) +
           marker +
           end
-        : s +
-          marker +
-          (attrNameEndIndex === -2 ? (attrNames.push(undefined), i) : end);
+        : s + marker + (attrNameEndIndex === -2 ? i : end);
   }
 
   const htmlResult: string | TrustedHTML =
@@ -908,56 +906,35 @@ class Template {
         // increment the bindingIndex, and it'll be off by 1 in the element
         // and off by two after it.
         if ((node as Element).hasAttributes()) {
-          // We defer removing bound attributes because on IE we might not be
-          // iterating attributes in their template order, and would sometimes
-          // remove an attribute that we still need to create a part for.
-          const attrsToRemove = [];
           for (const name of (node as Element).getAttributeNames()) {
-            // `name` is the name of the attribute we're iterating over, but not
-            // _necessarily_ the name of the attribute we will create a part
-            // for. They can be different in browsers that don't iterate on
-            // attributes in source order. In that case the attrNames array
-            // contains the attribute name we'll process next. We only need the
-            // attribute name here to know if we should process a bound attribute
-            // on this element.
-            if (
-              name.endsWith(boundAttributeSuffix) ||
-              name.startsWith(marker)
-            ) {
+            if (name.endsWith(boundAttributeSuffix)) {
               const realName = attrNames[attrNameIndex++];
-              attrsToRemove.push(name);
-              if (realName !== undefined) {
-                // Lowercase for case-sensitive SVG attributes like viewBox
-                const value = (node as Element).getAttribute(
-                  realName.toLowerCase() + boundAttributeSuffix
-                )!;
-                const statics = value.split(marker);
-                const m = /([.?@])?(.*)/.exec(realName)!;
-                parts.push({
-                  type: ATTRIBUTE_PART,
-                  index: nodeIndex,
-                  name: m[2],
-                  strings: statics,
-                  ctor:
-                    m[1] === '.'
-                      ? PropertyPart
-                      : m[1] === '?'
-                      ? BooleanAttributePart
-                      : m[1] === '@'
-                      ? EventPart
-                      : AttributePart,
-                });
-              } else {
-                parts.push({
-                  type: ELEMENT_PART,
-                  index: nodeIndex,
-                });
-              }
+              const value = (node as Element).getAttribute(name)!;
+              const statics = value.split(marker);
+              const m = /([.?@])?(.*)/.exec(realName)!;
+              parts.push({
+                type: ATTRIBUTE_PART,
+                index: nodeIndex,
+                name: m[2],
+                strings: statics,
+                ctor:
+                  m[1] === '.'
+                    ? PropertyPart
+                    : m[1] === '?'
+                    ? BooleanAttributePart
+                    : m[1] === '@'
+                    ? EventPart
+                    : AttributePart,
+              });
+              (node as Element).removeAttribute(name);
+            } else if (name.startsWith(marker)) {
+              parts.push({
+                type: ELEMENT_PART,
+                index: nodeIndex,
+              });
+              (node as Element).removeAttribute(name);
             }
           }
-          for (const name of attrsToRemove) {
-            (node as Element).removeAttribute(name);
-          }
         }
         // TODO (justinfagnani): benchmark the regex against testing for each
         // of the 3 raw text element names.