Using a directive in an attribute causes it to be marked dirty even if the final value is unchanged

[tec27]

Description

The mechanism by which directives are "resolved" in AttributePart results in it erroneously marking a value as dirty as long as there is at least one level of directive "between" a template and the final output. This means that any attribute that is set to a directive is effectively always marked dirty and re-applied on every render.

Steps to Reproduce

1. Write this code

import {html, render} from 'lit-html';
import {ifDefined} from 'lit-html/directives/if-defined';

function maybeWithMaxlength(haveMaxlength) {
  return html`<input id="my-text"
      type="text"
      maxlength="${ifDefined(haveMaxlength ? 20 : undefined)}" />`;
}

render(maybeWithMaxlength(true), document.body);

const input = document.querySelector('#my-text');
const observer = new MutationObserver(mutations => {
  alert('maxlength mutated!');
});
observer.observe(input, { attributes: true, attributeFilter: ['maxlength'] });

render(maybeWithMaxlength(true), document.body);

Live Reproduction Link

https://stackblitz.com/edit/lit-html-thtgvg?file=src/index.js

Expected Results

No alert would be shown (maxlength would not be mutated the second time, as its underlying value hasn't actually been changed)

Actual Results

Alert is shown.

Browsers Affected

• Chrome
• Firefox
• Edge
• Safari 11
• Safari 10
• IE 11

Additional Notes

This looks to me to be a problem with AttributePart#setValue, here: https://github.com/Polymer/lit-html/blob/master/src/lib/parts.ts#L113

When a directive is set as the value, the template that contains the attribute calls AttributePart.setValue with the directive (thus changing this.value from the previous value of 20 to a reference to the directive function). When the directive function is later resolved as part of the commit process, setValue is called again with the result of the directive (still 20), but since this.value is currently the directive and 20 is a primitive that isn't equal to the directive, the AttributePart gets marked as dirty.

[ilhan007]

Recently, I experienced the same issue. I have a custom directive and although the value of an attribute (the result of the directive) does not change, setAttribute is called by lit-html with the same value every time the template re-renders.

The case I hit is with the img tag, if you use a directive (reproduces with the built-in ifDefined as well) with the "src", calling setAttribute on each rendering causes network request for the image (if cache is not present or cache expires).

<img src="${myDirective(src)}"

This is a quick hack, that seems to workaround the issue, but it must be a better way of fixing it. The directive below is the built-in ifDefined directive with few additional lines, that can be seen below. It checks if the value is changed and if not - sets "noChange".

export const ifDefined = directive((value: unknown) => (part: Part) => {
	if (value === undefined && part instanceof AttributePart) {
		if (value !== part.value) {
			const name = committer.name;
			element.removeAttribute(name);
		}
	} else {
+		if (part.committer && part.committer.element && part.committer.element.getAttribute(part.committer.name) === value) {
+			part.setValue(noChange);
+		} else {
			part.setValue(value);
+		}
	}
});

[justinfagnani]

This is a general gotcha with directives, but I'm not sure yet how to address this automatically in AttributePart, so the fix in ifDefined seems like a good intermediate fix. I'll make a PR.