Skip to content

Commit 2234a12

Browse files
asyncLizcopybara-github
authored andcommitted
chore: fix formAssociated disabled attribute not working
There is out-of-sync state when lit's built-in accessor tries to reflect attributes alongside the getter and setter. Instead, the properties just read from attributes and requestUpdate is called when those attributes change. PiperOrigin-RevId: 590710159
1 parent eaefb50 commit 2234a12

File tree

1 file changed

+37
-11
lines changed

1 file changed

+37
-11
lines changed

labs/behaviors/form-associated.ts

Lines changed: 37 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -216,29 +216,55 @@ export function mixinFormAssociated<
216216
return this[internals].labels;
217217
}
218218

219-
// name attribute must be set synchronously
220-
@property({reflect: true})
219+
// Use @property for the `name` and `disabled` properties to add them to the
220+
// `observedAttributes` array and trigger `attributeChangedCallback()`.
221+
//
222+
// We don't use Lit's default getter/setter (`noAccessor: true`) because
223+
// the attributes need to be updated synchronously to work with synchronous
224+
// form APIs, and Lit updates attributes async by default.
225+
@property({noAccessor: true})
221226
get name() {
222227
return this.getAttribute('name') ?? '';
223228
}
224229
set name(name: string) {
225-
const prev = this.name;
226-
// Setting name to null or empty string does not remove the attribute.
230+
// Note: setting name to null or empty does not remove the attribute.
227231
this.setAttribute('name', name);
228-
// Explicit requestUpdate needed for Lit 2.0
229-
this.requestUpdate('name', prev);
232+
// We don't need to call `requestUpdate()` since it's called synchronously
233+
// in `attributeChangedCallback()`.
230234
}
231235

232-
// disabled attribute must be set synchronously
233-
@property({type: Boolean, reflect: true})
236+
@property({type: Boolean, noAccessor: true})
234237
get disabled() {
235238
return this.hasAttribute('disabled');
236239
}
237240
set disabled(disabled: boolean) {
238-
const prev = this.disabled;
239241
this.toggleAttribute('disabled', disabled);
240-
// Explicit requestUpdate needed for Lit 2.0
241-
this.requestUpdate('disabled', prev);
242+
// We don't need to call `requestUpdate()` since it's called synchronously
243+
// in `attributeChangedCallback()`.
244+
}
245+
246+
override attributeChangedCallback(
247+
name: string,
248+
old: string | null,
249+
value: string | null,
250+
) {
251+
// Manually `requestUpdate()` for `name` and `disabled` when their
252+
// attribute or property changes.
253+
// The properties update their attributes, so this callback is invoked
254+
// immediately when the properties are set. We call `requestUpdate()` here
255+
// instead of letting Lit set the properties from the attribute change.
256+
// That would cause the properties to re-set the attribute and invoke this
257+
// callback again in a loop. This leads to stale state when Lit tries to
258+
// determine if a property changed or not.
259+
if (name === 'name' || name === 'disabled') {
260+
// Disabled's value is only false if the attribute is missing and null.
261+
const oldValue = name === 'disabled' ? old !== null : old;
262+
// Trigger a lit update when the attribute changes.
263+
this.requestUpdate(name, oldValue);
264+
return;
265+
}
266+
267+
super.attributeChangedCallback(name, old, value);
242268
}
243269

244270
override requestUpdate(

0 commit comments

Comments
 (0)