stencil icon indicating copy to clipboard operation
stencil copied to clipboard

Support formAssociated components (using ElementInternals)

Open adriengibrat opened this issue 4 years ago • 15 comments

Stencil version:

 @stencil/[email protected]

I'm submitting a:

[ ] bug report [x] feature request [ ] support request => Please do not submit support requests here, use one of these channels: https://stencil-worldwide.herokuapp.com/ or https://forum.ionicframework.com/

Current behavior:

Stencil component with a static formAssociated property does not behave like a vanilla customElement, accessing form related internals methods throws "DOMException: Failed to read the 'form' property from 'ElementInternals': The target element is not a form-associated custom element." in chrome.

Expected behavior:

The static property formAssociated defined on a component should be attached to proxy class defining the customElement or at least Stencil could provide an idiomatic way to allow to attach static properties. It would be awesome to enventually provide a shim for unsuporting browser... but this may deserve it's own issue ;)

Steps to reproduce:

A click on the below component (when nested in a form) should submit the form and not throw an error on chrome. Related code:

@Component({
  tag: 'associated-to-form',
  shadow: true
})
export class Test {
  static formAssociated: true;
  @Element()
  private host: HTMLElement;
  constructor() {
    this._internals = this.host.attachInternals?.();
  }
  @Listen('click')
  submit() {
    ithis._internals.form?.submit();
  }
}

Other information:

More about formAssociated elements use cases: https://web.dev/more-capable-form-controls/

Specifications of formAssociated elements: https://html.spec.whatwg.org/multipage/custom-elements.html#custom-elements-face-example

A blog post about form related component with stencil \o/ https://www.hjorthhansen.dev/shadow-dom-form-participation/ , but formAssociated example seems to be pseudo code (not working) :/

Info about browser support https://www.chromestatus.com/feature/4708990554472448

adriengibrat avatar Mar 18 '20 20:03 adriengibrat

Are there any plans to implement this any time soon? As OP mentioned, it's already in the spec and is pretty stable at this point. While Chrome and Edge already support it, Firefox is actively working on implementing it (https://bugzilla.mozilla.org/show_bug.cgi?id=1552327).

jthannah avatar Nov 02 '21 12:11 jthannah

Hey all! I'm going to do my research and work on defining what this means to be able to ship. No timeline, but I added a request for comments. If anybody can provide the different use cases or their ideal interface - whether it's a decorator or a modification to a decorator - that would be fantastic!

splitinfinities avatar Nov 02 '21 18:11 splitinfinities

Before working out a higher level API I guess we'd need to know whether stencil wants to ship something 'official' that doesn't work cross-browser?

Until Safari gets on-board I can see the work to get something working cross-browser being a hot-mess (falling back to a hidden input sprouted to the lightDom / or utilising scoped: true?) ... (not to mention older browsers - although I'd agree that they shouldn't hold something like this back).

Perhaps until this feature is cross-browser, stencil should simply not get in way / work out what's disrupting devs using this low-level / native api?

johnjenkins avatar Nov 03 '21 12:11 johnjenkins

Having said what I just said ( :D ) ... it looks like there will need to be a hook to get this working with lazy loading - the static property formAssociated = true has to be set on the proxy / placeholder class, before being defined here ... which is as easy as

HostElement.formAssociated = true;

Then everything works.

I'd say the nicest way to implement this would be, via static analysis, to read in whether the user class has static formAssociated = true ... if so, set that as part of the component meta object. Then within bootstrapLazy add the property to the proxy class. This should have the added benefit of "just working"™ for the 'dist-custom-element*' bundles thus being a bit more future proof / less proprietary.

johnjenkins avatar Nov 03 '21 15:11 johnjenkins

As noted early, Chromium and Firefox already support the Element Internals API: https://caniuse.com/mdn-api_elementinternals. There is a polyfill for this as well (https://github.com/calebdwilliams/element-internals-polyfill) that could be used with Safari, but it is not small and I have not used it in production.

When I tried to get this to work with Stencil a few months back, the main issue was the static formAssociated property that @johnjenkins referred to. However, there are a whole lot of other read-only properties and lifecycle methods (synchronous) that can be used on the element (see https://web.dev/more-capable-form-controls).

Here is a link to the docs on the lifecycle methods: https://web.dev/more-capable-form-controls/#lifecycle-callbacks

Here is an example of some of that stuff:

  get form() { return this.internals_.form; }
  get type() { return this.localName; }
  get validity() {return this.internals_.validity; }
  get validationMessage() {return this.internals_.validationMessage; }
  get willValidate() {return this.internals_.willValidate; }

  checkValidity() { return this.internals_.checkValidity(); }
  reportValidity() {return this.internals_.reportValidity(); }

In my experience, we can handle enhancement of the element class in the constructor for adding these properties and lifecycle methods. So as @johnjenkins suggested, the main issue is the static formAssociated property. However, in the long term and after Safari adds support, it would be nice to be able to add the read-only properties and lifecycle events for validation in a more ergonomic way.

nborelli avatar Nov 03 '21 19:11 nborelli

Kind of unrelated, but here is the TS decorator I would love to have in Stencil: @ExposeOnElement() (name needs work 😉). In my imaginary world, it could be used on instance level property getters and setters as well as synchronous or asynchronous methods to expose the properties or read-only properties and methods on the element. In addition, we could even use this decorator on static properties or methods to expose them on the element as well. The documentation system would use the type of the member to determine if it should be documented as a method, property or static member of the element class. This would be a way for us to introduce this sort of stuff from the controller class that we write in a consistent way that can be statically analyzed and documented.

I imagine it would be a lot of work though and some of this could be done with the decorators that are already provided if they were a little more flexible (synchronous methods and read-only properties for example).

nborelli avatar Nov 03 '21 20:11 nborelli

@splitinfinities we can't use ElementInternals with Stencil field components? Since web components cannot participate in forms are we required to repeat form field code within a Stencil form or is there a way to create reusable field components that work?

BenRacicot avatar Nov 15 '21 20:11 BenRacicot

@BenRacicot I'd suggest looking at using scoped: true instead of shadow: true. This removes the shadowDom (so you get native form participation / validation so long as you include an <input /> or similar in your render()) but still gives you style scoping.

johnjenkins avatar Nov 15 '21 20:11 johnjenkins

Hey @johnjenkins thanks for the reply. Unfortunately turning off encapsulation is not an option in my case.

BenRacicot avatar Nov 15 '21 20:11 BenRacicot

In order to fully realize this I believe we need to first ship #2899, does that seem to track? Especially for the getters that @nborelli posted above (https://github.com/ionic-team/stencil/issues/2284#issuecomment-959904765)

splitinfinities avatar Nov 15 '21 23:11 splitinfinities

Hi @splitinfinities, we would probably also need a way of adding synchronous methods like the checkValidity() and reportValidity() methods that I posted above. I know we can add these methods to the host element in the constructor, but this seems a little awkward.

nborelli avatar Nov 16 '21 00:11 nborelli

Kind of unrelated, but here is the TS decorator I would love to have in Stencil: @ExposeOnElement() (name needs work 😉). In my imaginary world, it could be used on instance level property getters and setters as well as synchronous or asynchronous methods to expose the properties or read-only properties and methods on the element. In addition, we could even use this decorator on static properties or methods to expose them on the element as well. The documentation system would use the type of the member to determine if it should be documented as a method, property or static member of the element class. This would be a way for us to introduce this sort of stuff from the controller class that we write in a consistent way that can be statically analyzed and documented.

I imagine it would be a lot of work though and some of this could be done with the decorators that are already provided if they were a little more flexible (synchronous methods and read-only properties for example).

Just my own thoughts but I love this idea. A decorator to implement either a decided-solution, or perhaps even the applicable ElementInternals methods to expose field component's attributes to its form's FormData.

Perhaps even @ElementInternals for a name?

To have this done for us would be a big win for Stencil IMHO.

BenRacicot avatar Nov 16 '21 00:11 BenRacicot

In order to fully realize this I believe we need to first ship #2899, does that seem to track? Especially for the getters that @nborelli posted above (#2284 (comment))

It isn't clear to me that #2889 is required to support attachInternals strictly speaking. While getters and setter would be nice, I don't see what they have to do with exposing additional properties and methods on the component. What would be required is having the stencil compiler recognize attachInternals as a part of the component definition. Maybe it could be a setting in the @Component decorator?

tx-tim avatar Dec 01 '21 16:12 tx-tim

As a workaround I believe you can temporarily intercept the constructor with a global monkey patch like this.

const FORM_ASSOCIATED_ELEMENTS = ['my-input', 'my-select'];

const define = customElements.define;

customElements.define = (name, constructor, options) => {
  // This could probably be changed to read a static `formAssociated` field on the associated
  // Stencil component class to avoid needing `FORM_ASSOCIATED_ELEMENTS`.
  if (FORM_ASSOCIATED_ELEMENTS.includes(name)) {
    constructor.formAssociated = true;
  }

  define(name, constructor, options);
};

defineCustomElements(); // from your loader

delete customElements.define; // revert to the prototype implementation

Is there a better way?

mmun avatar Feb 14 '22 20:02 mmun

Any update around this 2y discussion? :grin:

I saw some libs (like LitElement) already working with form association for some time ago: https://css-tricks.com/creating-custom-form-controls-with-elementinternals/

For now, in my projects I workaround creating a simple input[type=hidden] when a parameter with the name form -associated is true like:

<my-input name="test" form-associated></my-input>
// Call in componentDidLoad
private appendInputHidden(): void {
  if (this.formAssociated) {
    const input = document.createElement('input');
    input.name = this.name;
    input.id = this.name;
    input.value = this.value;
    input.type = 'hidden';
    this.element.appendChild(input);
    }
}

and updating the value when the onInput (in the case of input-text) like:

private updateInputHidden(value: string = this.value): void {
  if (this.formAssociated) {
    this.element.querySelector<HTMLInputElement>(`input[name=${this.name}]`).value = value;
  }
}

But ofc is just a workaround, because now I have more things to manage and "two" inputs (into the DOM and ShadowDOM)


My idea/proposal, it will be very interesting to have a property into the @Component like:

@Component({
   tag: 'my-input',
   styleUrl: 'my-input.scss',
   shadow: true,
   formAssociated: true // <-- \o/
})

With the flag, the component can inherit the internal behaviors of form association :yum:

What you guys think? I really want to remove all the code that I have in a loot of my components that creates the input hidden :sweat:

alvarocjunq avatar Jul 15 '22 10:07 alvarocjunq

What with Safari getting on-board with this, this feature is now an imperative, so I've just bashed something out very quickly in order to get the ball rolling.

https://github.com/ionic-team/stencil/pull/3519 - essentially does what I mentioned above - reads in static formAssociated = true; from your component and adds it to the proxy component. I opted for that route just to match the native / vanilla api but am more than open to add it instead to

@Component({
   ...
   formAssociated: true // <-- \o/
})

If people have strong feelings I can look at that instead - lemme know.

Regarding 'readonly' props - I suspect this one needs to be merged Regarding synchronous @Methods (for use with things like reportValidity etc) this is a trickier prospect... the reason they're all async is due to the lazy-loader.

But both these issues can be worked around for now (as mentioned) by using constructor to add these properties and methods. It's mainly formAssociated that's getting in the way.

johnjenkins avatar Aug 12 '22 11:08 johnjenkins

Tks @johnjenkins It will be great =)

Well, about use inside of the @Component and a static

I really understand the argument and I like it:

bit more future proof / less proprietary.

However, stencil defined some rules and use a static property looks a "rule breaker" Because the configurations of the component are inside of @Component, the documentation will be a bit strange to this case

But maybe it's about the plans of Stencil and how much effort will be expended in the future to keep alive :muscle:

So, if the plan is more aligned in have less Stencil stuff inside Stencil (more vanilla I mean), I agree with the static property If not, I think it's better a property inside of @Component, to keep the configurations together (easier to document, find or even try with ctrl+space and find a new feature)

I'm glad in in see this important issue walking, hope I helped in think about it :metal::metal::metal:

alvarocjunq avatar Aug 12 '22 13:08 alvarocjunq

#3519 - essentially does what I mentioned above - reads in static formAssociated = true; from your component and adds it to the proxy component. I opted for that route just to match the native / vanilla api but am more than open to add it instead to

@Component({
   ...
   formAssociated: true // <-- \o/
})

If people have strong feelings I can look at that instead - lemme know.

Is there a way to support both? I'm cool with adding it as a compiler option, but might be cool to also have it support the static way of declaring it too.

eriklharper avatar Aug 18 '22 22:08 eriklharper

Any update on this?

erichstark avatar Oct 21 '22 15:10 erichstark

Any updates here?

SleeplessFox avatar Jan 17 '23 14:01 SleeplessFox

https://webkit.org/blog/13703/release-notes-for-safari-technology-preview-162/ Element Internals just dropped in Safari Tech Preview.

markacianfrani avatar Jan 26 '23 16:01 markacianfrani

Any update on this?

prashanthsasidharan avatar Feb 03 '23 07:02 prashanthsasidharan

Please add this feature :)

konrad-kinesso avatar Mar 26 '23 19:03 konrad-kinesso

Any update on this?

Deepakrishikesh avatar Mar 27 '23 05:03 Deepakrishikesh

Any update? This would be really great since we cannot use shadow dom in a reasonable way without it

jeffrey-kinesso avatar Mar 28 '23 15:03 jeffrey-kinesso

We are using web components built with stenciljs in a react application. Because this feature is not available, we are forced to create controlled component instead of a an uncontrolled one. As this issue is open for over 3 year's and requested by many developer, I kindly request the maintainers to add this feature

prashanthsasidharan avatar Mar 28 '23 15:03 prashanthsasidharan

In our case, we've been forced to opt Lit only for form elements (we much prefer keep working with stencil), this is very time consuming for us and we still hope this feature will be implemented as soon as possibile.

Please let us know if at least there is an intention to integrate this feature.

vitto avatar Mar 31 '23 10:03 vitto

Just adding to this request to get some visibility. ElementInternals was released in Safari 16.4 in March. Would be great to see this feature 😄.

davididas avatar May 17 '23 00:05 davididas

+1 anything new regarding the issue?

danielleroux avatar Jun 12 '23 05:06 danielleroux

👋 ElementInternals are the next "thing" for us to investigate support for Stencil. While I don't have any timeline, it is the next Web Component ecosystem feature that we have our sights set on!

rwaskiewicz avatar Jun 12 '23 13:06 rwaskiewicz