Skip to content

Commit

Permalink
[lit-html] Add DEV_MODE error if duplicate attribute bindings are enc…
Browse files Browse the repository at this point in the history
…ountered (#4523)

Co-authored-by: Steve Orvell <sorvell@google.com>
  • Loading branch information
AndrewJakubowicz and Steve Orvell committed Jan 29, 2024
1 parent 8d3e305 commit 1a32b61
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 0 deletions.
6 changes: 6 additions & 0 deletions .changeset/forty-schools-flash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'lit-html': patch
'lit': patch
---

Add a DEV_MODE error to catch duplicate attribute bindings that otherwise create silent errors.
22 changes: 22 additions & 0 deletions packages/lit-html/src/lit-html.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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().
Expand Down
18 changes: 18 additions & 0 deletions packages/lit-html/src/test/lit-html_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down

0 comments on commit 1a32b61

Please sign in to comment.