[lit-html] Add DEV_MODE error if duplicate attribute bindings are encountered (#4523)

AndrewJakubowicz · Steve Orvell · web-flow · commit 1a32b61ecf09 · 2024-01-29T11:08:33.000-08:00
Co-authored-by: Steve Orvell &lt;sorvell@google.com&gt;
diff --git a/.changeset/forty-schools-flash.md b/.changeset/forty-schools-flash.md
@@ -0,0 +1,6 @@
+---
+'lit-html': patch
+'lit': patch
+---
+
+Add a DEV_MODE error to catch duplicate attribute bindings that otherwise create silent errors.
diff --git a/packages/lit-html/src/lit-html.ts b/packages/lit-html/src/lit-html.ts
@@ -1036,6 +1036,28 @@ class Template {
       }
       nodeIndex++;
     }
+
+    if (DEV_MODE) {
+      // If there was a duplicate attribute on a tag, then when the tag is
+      // parsed into an element the attribute gets de-duplicated. We can detect
+      // this mismatch if we haven't precisely consumed every attribute name
+      // when preparing the template. This works because `attrNames` is built
+      // from the template string and `attrNameIndex` comes from processing the
+      // resulting DOM.
+      if (attrNames.length !== attrNameIndex) {
+        throw new Error(
+          `Detected duplicate attribute bindings. This occurs if your template ` +
+            `has duplicate attributes on an element tag. For example ` +
+            `"<input ?disabled=\${true} ?disabled=\${false}>" contains a ` +
+            `duplicate "disabled" attribute. The error was detected in ` +
+            `the following template: \n` +
+            '`' +
+            strings.join('${...}') +
+            '`'
+        );
+      }
+    }
+
     // We could set walker.currentNode to another node here to prevent a memory
     // leak, but every time we prepare a template, we immediately render it
     // and re-use the walker in new TemplateInstance._clone().
diff --git a/packages/lit-html/src/test/lit-html_test.ts b/packages/lit-html/src/test/lit-html_test.ts
@@ -2193,6 +2193,24 @@ suite('lit-html', () => {
         );
       });
 
+      skipTestIfCompiled('Duplicate attributes throw', () => {
+        assert.throws(() => {
+          render(
+            html`<input ?disabled=${true} ?disabled=${false} fooAttribute=${'potato'}>`,
+            container
+          );
+        }, `Detected duplicate attribute bindings. This occurs if your template has duplicate attributes on an element tag. For example "<input ?disabled=\${true} ?disabled=\${false}>" contains a duplicate "disabled" attribute. The error was detected in the following template: \n\`<input ?disabled=\${...} ?disabled=\${...} fooAttribute=\${...}>\``);
+      });
+
+      test('Matching attribute bindings across elements should not throw', () => {
+        assert.doesNotThrow(() => {
+          render(
+            html`<input ?disabled=${true}><input ?disabled=${false}>`,
+            container
+          );
+        });
+      });
+
       test('Expressions inside nested templates throw in dev mode', () => {
         // top level
         assert.throws(() => {
