bug/question: ref element validity is not up-to-date on Prop Watch trigger #5965

dpellier · 2024-09-03T08:04:18Z

Prerequisites

I have read the Contributing Guidelines.
I agree to follow the Code of Conduct.
I have searched for existing issues that already report this problem, without success.

Stencil Version

4.21

Current Behavior

We are implementing form elements components using ElementInternals.
On our component value change, we want to update the internals validity to match the validity of the native form element embedded.

Using this kind of setup:

@Watch('value')
  onValueChange(): void {
    this.internals.setFormValue(value?.toString() ?? '');
    this.internals.setValidity(...); // with a loop over the form element flags to copy its validityState
}

This does work fine when the trigger comes from a change on the form element directly (like an onInput callback), but not when the Prop value of the component change, unless we delay it using a setTimeout 0.
(see example below and repro code repo)

Expected Behavior

I can guess why this behaviour is actually happening as the Watch trigger in the case of Prop change does not care about waiting for other DOM changes to occurs.
But I'm not sure the setTimeout 0 is the best way to handle this.

Do you have any best practices about how to sync the internals validity without using such hack?
Or is it something that can actually be fixed on the Watch side?

System Info

System: node 18.17.1
    Platform: darwin (23.4.0)
   CPU Model: Apple M1 (8 cpus)
    Compiler: /.../node_modules/@stencil/core/compiler/stencil.js
       Build: 1724698030
     Stencil: 4.21.0 🐷
  TypeScript: 5.5.3
      Rollup: 2.56.3
      Parse5: 7.1.2
      jQuery: 4.0.0-pre
      Terser: 5.31.1

Steps to Reproduce

Using a basic input component (or clone https://github.com/dpellier/stencil-set-validity-issue):

import { AttachInternals, Component, Host, Method, Prop, Watch, h } from '@stencil/core';

@Component({
  formAssociated: true,
  shadow: true,
  tag: 'my-component',
})
export class MyComponent {
  private inputElement?: HTMLInputElement;

  @AttachInternals() private internals!: ElementInternals;

  @Prop({ mutable: true, reflect: true }) public value: string = null;

  @Method()
  reset() {
    // Here the <input> has not been updated yet as we modify the Prop directly
    this.value = null;
  }

  @Watch('value')
  onValueChange(): void {
    // Here we want to update the internals value and validity using
    // this.internals.setFormValue(value?.toString() ?? '');
    // this.internals.setValidity(...); // with a loop over the inputElement flags to copy its validityState

    // If we arrive from the onInput, <input> is already updated so the value is accurate
    // But if we arrive from the reset Method, the <input> is not yet updated, so the it's still the old validityState
    console.log('onWatch, isValid?', this.inputElement.validity.valid);

    setTimeout(() => {
      // If we delay it, we can see the new expected value
      console.log('onWatch after timeout, isValid?', this.inputElement.validity.valid);
    }, 0);
  }

  private onInput(): void {
    // Here the <input> validityState has been updated already
    console.log('onInput, isValid?', this.inputElement.validity.valid);

    // So the Watch trigger will be able to set the internals validity with the <input> value correctly
    this.value = this.inputElement?.value ?? null;
  }

  render() {
    return (
      <Host>
        <input
          onInput={ (): void => this.onInput() }
          ref={ (el): HTMLInputElement => this.inputElement = el as HTMLInputElement }
          required={ true }
          type="text"
          value={ this.value || '' }>
        </input>
      </Host>
    );
  }
}

Run the example of the component with a reset button and check the console.log

<body>
    <label>Required input</label>

    <my-component>
    </my-component>

    <button>Reset value</button>

    <script>
      const component = document.querySelector('my-component');
      const resetButton = document.querySelector('button');

      resetButton.addEventListener('click', () => {
        component.reset();
      })
    </script>
  </body>

Type a value on the input => all validity checks return true, as expected
Click the reset button => first Watch log return true, which is wrong as the input is required, but the delayed log return true

NB: the reset Method is just an example, other usage can be problematic, like global form reset:

async formResetCallback(): Promise<void> {
  this.value = null; // will cause the same behaviour
}

Code Reproduction URL

https://github.com/dpellier/stencil-set-validity-issue

Additional Information

No response

The text was updated successfully, but these errors were encountered:

christian-bromann · 2024-09-05T11:19:30Z

Thanks for raising the issue @dpellier the Stencil team currently has a lot of competing priorities and can't promise to be able to take a look at this anytime soon. We recommend to get involved and help us resolve this bug by providing a pull request with a fix. We are happy to provide necessary guidance and appreciate any help!

MatteoFeltrin · 2024-10-18T19:11:16Z

@dpellier I have understood the problem here, but when you link a @prop to an element, you are re-rendering it every time, and I think that's not what you want.
value={ this.value || '' }>
This means that everytime you write something in the input, you update your local this.value and then the component is re-rendered uselessly.
I don't know your complete use-case, but from what I see here basically you just want to reset the component through an external call.

Try this approch, just rely on the native input event and to reset the value, act directly on the element it-self.
May this solve your use-case issue?

import { AttachInternals, Component, Host, Method, Prop, Watch, h, State } from '@stencil/core';

@Component({
  formAssociated: true,
  shadow: true,
  tag: 'my-component',
})
export class MyComponent {
  private inputElement?: HTMLInputElement;

  @AttachInternals() private internals!: ElementInternals;

  @Method()
  reset() {
    this.inputElement.value = null;
    // Directly acting on element value does not trigger the event automatically, so dispatch it yourself
    this.inputElement.dispatchEvent(new Event('input', { bubbles: true }));
  }

  private onInput(): void {
    // Do what you want here. Assign the value to a local variable if needed.
    console.log('onInput, isValid?', this.inputElement.validity.valid);
  }

  render() {
    return (
      <Host>
        <input
          onInput={(): void => this.onInput()}
          ref={(el): HTMLInputElement => (this.inputElement = el as HTMLInputElement)}
          required={true}
          type="text"
        ></input>
      </Host>
    );
  }
}

Sukaato · 2024-11-05T14:22:41Z

@dpellier I think your problem comes from understanding the stencil life cycle.

In the reset method, you try to reset the component value, but when the value property is updated, it must go through the onValueChange watch process, but this method only has access to the current rendered input, not the updated one, so it's normal that your validity status is still valid.

Your validity status will not be invalidated until after the render.

To reset the current input value, simply proceed as follows:

import { AttachInternals, Component, Host, Method, Prop, Watch, h } from '@stencil/core';

@Component({
  formAssociated: true,
  shadow: true,
  tag: 'my-component',
})
export class MyComponent {
  private inputElement?: HTMLInputElement;

  @AttachInternals() private internals!: ElementInternals;

  @Prop({ mutable: true, reflect: true }) public value: string = null;

  @Method()
  reset() {
    if (this.inputElement) {
      this.inputElement.value = null;
    }
    this.value = null;
  }

  @Watch('value')
  onValueChange(): void {
    console.log('onWatch, isValid?', this.inputElement.validity.valid);
  }

  private onInput(): void {
    this.value = this.inputElement?.value ?? null;
  }

  render() {
    return (
      <Host>
        <input
          onInput={ (): void => this.onInput() }
          ref={ (el): HTMLInputElement => this.inputElement = el as HTMLInputElement }
          required={ true }
          type="text"
          value={ this.value || '' }>
        </input>
      </Host>
    );
  }
}

And that will solve your problem.

ionitron-bot bot added the triage label Sep 3, 2024

christian-bromann added Bug: Validated This PR or Issue is verified to be a bug within Stencil and removed triage labels Sep 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bug/question: ref element validity is not up-to-date on Prop Watch trigger #5965

bug/question: ref element validity is not up-to-date on Prop Watch trigger #5965

dpellier commented Sep 3, 2024 •

edited

Loading

christian-bromann commented Sep 5, 2024

MatteoFeltrin commented Oct 18, 2024

Sukaato commented Nov 5, 2024

bug/question: ref element validity is not up-to-date on Prop Watch trigger #5965

bug/question: ref element validity is not up-to-date on Prop Watch trigger #5965

Comments

dpellier commented Sep 3, 2024 • edited Loading

Prerequisites

Stencil Version

Current Behavior

Expected Behavior

System Info

Steps to Reproduce

Code Reproduction URL

Additional Information

christian-bromann commented Sep 5, 2024

MatteoFeltrin commented Oct 18, 2024

Sukaato commented Nov 5, 2024

dpellier commented Sep 3, 2024 •

edited

Loading