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.

Sign up for GitHub

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

Jump to bottom

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

Open
3 tasks done
dpellier opened this issue Sep 3, 2024 · 1 comment
Open
3 tasks done

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

dpellier opened this issue Sep 3, 2024 · 1 comment
Labels
Bug: Validated This PR or Issue is verified to be a bug within Stencil

Comments

@dpellier
Copy link
Contributor

dpellier commented Sep 3, 2024 •
edited
Loading

Prerequisites

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>
  1. Type a value on the input => all validity checks return true, as expected
  2. 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
@ionitron-bot ionitron-bot bot added the triage label Sep 3, 2024
@christian-bromann
Copy link
Member

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!

@christian-bromann christian-bromann added Bug: Validated This PR or Issue is verified to be a bug within Stencil and removed triage labels Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug: Validated This PR or Issue is verified to be a bug within Stencil
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@christian-bromann @dpellier