Add @lit-labs/forms and FormAssociated RFC

[justinfagnani]

RFC for a new @lit-labs/forms package based on the work I prototyped in lit/lit#4773

[KonnorRogers]

overall i appreciate the minimal impact on public APIs and the design of these APIs. It feels well thought out and has had quite a bit of time put into it. Overall I don't see any major blockers beyond attachInternals() needing to be managed by Lit with no escape hatches for if it was already defined.

attachInternals has to be managed by Lit. I don't know if there is a way around this, but many people call attachInternals() on a base class so they can leverage CSS custom states on any element, not just form associated elements. Would there be room in the mixin for providing a custom function for grabbing an existing ElementInternals state and if the function is provided, this helper should not attempt to register via attachInternals()

Initial idea is accepting an optional function argument which will get evaluated when calling getInternals(el). Something like the following:

class MyEl extends FormAssociated(BaseElement, { elementInternals: (element) => getInternals(element) })

// "internals" is a "public" field on the base element. The author using Lit's form associated may not have control over this.
class MyEl extends FormAssociated(BaseElement, { elementInternals: (element) => element.internals })

// Some libraries like Material UI use symbols to "protect" element internals.

export const elementInternalsSymbol = new Symbol("elementInternals")
import { elementInternalsSymbol } from "./internal/element-internals.js"
class MyEl extends FormAssociated(BaseElement, { elementInternals: (element) => element[elementInternalsSymbol] })

[KonnorRogers]

What happens if a user has already called attachInternals() in another base class? Is there any escape hatch to provide it with an existing instance of ElementInternals?

[KonnorRogers]

Does this do so via ElementInternals.ariaDisabled?

I know in my tests screen readers still didn't have support for this ~2months ago, but sprouting aria-disabled on the host feels equally bad :/

I'm assuming we could just document this and hopefully it eventually gets fixed, and if needed a consumer could override, call super.formDisabledCallback() and then add their own aria-disabled attribute if needed.

[justinfagnani]

Does this do so via ElementInternals.ariaDisabled?

Yes

I know in my tests screen readers still didn't have support for this ~2months ago, but sprouting aria-disabled on the host feels equally bad :/

I'm assuming we could just document this and hopefully it eventually gets fixed, and if needed a consumer could override, call super.formDisabledCallback() and then add their own aria-disabled attribute if needed.

I think documenting it is good. You could potentially implement a small polyfill that implements ElementInternals.ariaDisabled by sprouting an attribute, and remove that over time.

[jpzwarte]

I like the simplicity of this API, but this is only usable for native <form> integration. I understand you have to start somewhere, but often you want more rich forms where you can have nested objects and/or non-string values. I wonder if both scenarios could be supported at the same time.

[lennartbuit]

Is this representative of how normal form controls operate? In my (limited) testing, value is usually not reflected, and the current value for the value attribute is used as default, so I expected:

  // Setting this field calls internals.setFormValue()
  @formValue()
  @property({reflect: false, attribute: false})
  accessor value: string = '';

  // When the form resets, the value is set to this field
  @formDefaultValue()
  @property({attribute: 'value'})
  accessor defaultValue: string = '';

[justinfagnani]

The point here is that you can use the decorators to construct the public API you want, including as you point out, one that matches the oddities of <input>. Yes, you do not have to reflect any of these decorated fields.

[tbroyer]

Fwiw, I'd still change the example: it's looks a lot like <input> and doesn't require much change to behave like <input>. Either that or explicitly mention that this behaves differently from <input> so nobody ever copies this blindly.

Suggested change

	// Setting this field calls internals.setFormValue()
	@formValue()
	@property({reflect: true})
	accessor value: string = '';
	// When the form resets, the value is set to this field
	@formDefaultValue()
	accessor defaultValue: string = '';
	// Setting this field calls internals.setFormValue()
	@formValue()
	@property({attribute: false})
	accessor value: string = '';
	// When the form resets, the value is set to this field
	@formDefaultValue()
	@property({attribute: 'value', reflect: true})
	accessor defaultValue: string = '';

[lennartbuit]

So I take that you can mimic <select multiple> by having a FormData value containing multiple values for the same name, right? Is that something to spend a few words on?

[justinfagnani]

Yes, but this sentence is merely reflecting the platform API constraints. This library won't dictate or interfere with how form data is represented, so however you would do this manually is how you would do it here.

[lennartbuit]

Not in the scope of this PR, but as a webcomponent author, a section in some docs about both points I mentioned would be very helpful. Very finicky to get right :).

[tbroyer]

nit: how about just calling super.attachInternals() here to let the native implementation throw?

[tbroyer]

Fwiw, I'd still change the example: it's looks a lot like <input> and doesn't require much change to behave like <input>. Either that or explicitly mention that this behaves differently from <input> so nobody ever copies this blindly.

Suggested change

	// Setting this field calls internals.setFormValue()
	@formValue()
	@property({reflect: true})
	accessor value: string = '';
	// When the form resets, the value is set to this field
	@formDefaultValue()
	accessor defaultValue: string = '';
	// Setting this field calls internals.setFormValue()
	@formValue()
	@property({attribute: false})
	accessor value: string = '';
	// When the form resets, the value is set to this field
	@formDefaultValue()
	@property({attribute: 'value', reflect: true})
	accessor defaultValue: string = '';

[tbroyer]

Typo

Suggested change

	return html`<input .value=${this.value}> @input=${this.#onInput}`;
	return html`<input .value=${this.value} @input=${this.#onInput}>`;

[tbroyer]

Suggested change

	- Decorators: The `@defaultValue()` and `@defaultState()` decorators let the
	- Decorators: The `@formDefaultValue()` and `@formDefaultState()` decorators let the

[tbroyer]

Is lib-labs/form handling whether the form is "touched" in some form? Because native <input> elements will have their value and checked change whenever defaultValue and defaultChecked are changed as long as the input hasn't been touched: https://html.spec.whatwg.org/multipage/input.html#the-input-element:concept-fe-dirty-2

If it's a non-goal then maybe be explicit about it (or is it already and I missed it?)

[kyubisation]

Would it be an option to extract the ElementInternals management into its own mixin? I feel like e.g. the assignment of the role via static property would be useful in other scenarios.

I also liked the suggestion in lit/lit#4773 (comment), which also seems like a clean separation between implementers and consumers.