Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider functional mixin pattern for form-associated custom elements #812

Closed
JanMiksovsky opened this issue May 7, 2019 · 5 comments
Closed

Comments

@JanMiksovsky
Copy link

Rather than incorporating support for forms into all custom elements, I was wondering whether it’s worth exploring a functional mixin pattern for defining form support. This would keep form features from affecting components that don’t need those features.

In this approach, form participation would be defined through a mixin function that can be applied a base class (either HTMLElement, or a class deriving from it) to return a new form-capable class. JavaScript pseudo-code for such a mixin would look like:

function FormAssociatedMixin(Base) {
  return FormAssociated extends Base {
    // … Form-related methods/properties defined here, become part of resulting class …
    static get formAssociated() { return true; }
  };
}

Authors creating custom elements with forms in mind would then apply this FormAssociatedMixin to the base class they extend. The sample custom element from the Form Participation Explainer might look like:

class MyControl extends FormAssociatedMixin(HTMLElement) {
  formAssociatedCallback() {  }
  formDisabledCallback() {  }
  formStateRestoreCallback(state, mode) {  }
}

customElements.define('my-control', MyControl);

If someone wants form support, they extend FormAssociatedMixin(HTMLElement); if they don’t intend their component to be used with forms, they extend HTMLElement as they do now.

Among other things, this could keep ElementInternals clean for components that don’t support forms. The form-related properties and methods currently planned for ElementInternals could become private properties and methods:

function FormAssociatedMixin(Base) {
  return FormAssociated extends Base {
    static get formAssociated() { return true; }
    #setFormValue(value, state) {  }
    #checkValidity() {  }
    #reportValidity() {  }};
}

And then invoked directly on this, rather than on ElementInternals:

class MyControl extends FormAssociatedMixin(HTMLElement) {
  
  formResetCallback() {
    this.value = '';
    this.#setFormValue('');
  }
  
}

I make this suggestion only because the Elix project I lead has gotten so much mileage out of the extensive set of functional mixins; we’ve defined this way. After several years of creating components, I can say that it’s really nice to keep optional behavior out of base classes, and only bring them into play when needed. I think form participation is a good example of such optional behavior, so perhaps a mixin approach would work well for it.

@domenic
Copy link
Collaborator

domenic commented May 7, 2019

Hey, thanks for sharing this! My initial impression is that this is a good, opinionated pattern for frameworks and libraries to build on top of ElementInternals, but as a base primitive, ElementInternals is simpler and more flexible. For example, it allows you to create classes which don't have custom mixin-based inheritance hierarchies, instead deriving directly from HTMLElement like all the native form controls do.

I'm also having a bit of trouble understanding the stated advantages of this pattern, on the level of primitives. In particular, statements like

If someone wants form support, they extend FormAssociatedMixin(HTMLElement) if they don’t intend their component to be used with forms, they extend HTMLElement as they do now.

don't seem like advantages to me over the current proposal's "If someone wants form support, they add static formAssociated = true to their class". Requiring a specific inheritance hierarchy is a much more heavyweight mechanism than we've ever used before for this sort of thing.

The form-related properties and methods currently planned for ElementInternals could become private properties and methods:

This doesn't work with JavaScript private, because the private fields you declare only belong to the FormAssociated returned by your FormAssociatedMixin function; they don't belong to the MyControl which extends FormAssociated. You seem to be looking for "protected", but as discussed, that doesn't exist in JS, and ElementInternals is the way we accomplish that.

But that said, again, I really think there's room for an opinionated framework to make writing FACEs easier with mixins, or codegen, or the like. For example, it could automatically provide public properties/methods for type/disabled/form/labels/name/willValidate/validity/validationMessage/checkValidity/reportValidity/setCustomValidity, which are kind of cookie-cutter re-exposures if you don't use a framework. I know @justinfagnani was also interested in exploring this space.

Curious about your thoughts!

@justinfagnani
Copy link
Contributor

I don't have a lot to add to what @domenic said. The fact that we can write useful mixins in userland is great, and seems like the basis for a good standalone library.

I would emphasize that trying to automatically handle ElementInternals in a mixin, while also keeping it available to the concrete class is a little tricky because of the lack of protected. I think you'll always need some shared state besides the instance reference itself (say a public field, a WeakMap, etc.).

A mixin can override attachInternals, as ugly as that seems it probably composes better if multiple mixins try to get to internals. Something like this, though there may be a critical flaw in this idea:

const setFormValue = Symbol(FormAssociated.'setFormValue');

export const FormAssociated = (base) => class FormAssociated extends base {

  #internals;
  #internalsRetreived = false;

  constructor() {
    super();
    this.#internals = this.attachInternals();
    // allow another attachInternals() call, from the leaf class, or a mixin
    // that follows this pattern
    this.#internalsRetreived = false;
  }

 attachInternals() {
    if (this.#internalsRetreived) {
      throw new InvalidStateError('ElementInternals already requested');
    }
    this.#internalsRetreived = true;
    return this.#internals;
  }

  [setFormValue](value, state) {
    this.#internals.setFormValue(value, state);
  }
}
FormAssociated.setFormValue = setFormValue;
import {FormAssociated} from 'form-associated';

class MyControl = FormAssociated(HTMLElement) {
  #internals;

  constructor() {
    // maybe for AOM, pseudo-classes, etc.
    this.#internals = this.attachInternals();
  }

  formResetCallback() {
    this.value = '';
    this[FormAssociated.setFormValue](this.value);
  }
}

@annevk
Copy link
Collaborator

annevk commented May 9, 2019

I'm not sure I understand how this is a mixin. At least, given a hypothetical AriaDescribedByTargetMixin, how would my component use both that and FormAssociatedMixin?

@matthewp
Copy link

matthewp commented May 9, 2019

@annevk

class MyControl extends AriaDescribedByTargetMixin(FormAssociatedMixin(HTMLElement)) {

}

@JanMiksovsky
Copy link
Author

My original point has been heard, so closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants