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

Infinite Loop with .modelValue and @model-value-changed with requestUpdate() #1908

Closed
GeloCode opened this issue Feb 9, 2023 · 5 comments
Closed

Comments

@GeloCode
Copy link

GeloCode commented Feb 9, 2023

Expected behavior

The value of the input or datepicker updates properly without having an infinite loop
Example working as expected (you've this in the actual behaviour working):

<lion-input 
  .modelValue=${'Prefilled value'}
  @model-value-changed="${(e) => {
    console.log(`Input model value changed: ${e.target.value}`);
    this.requestUpdate();
  }}"
  label="Prefilled"
></lion-input>

Actual Behavior

When you try to make it for a datepicker as for the documentation you need to use the new Date in the modelValue:

<lion-input-datepicker
        label="MinMaxDate"
        .modelValue=${new Date('2023-01-19')}
        @model-value-changed=${(e) => {
          console.log(`Datepicker model value changed: ${e.target.value}`);
          this.requestUpdate();
        }}
      >

This new is breaking the component when you do a requestUpdate inside the model-value-changed entering in an infinite loop, you can see the difference with the input.

If we do a new String('test'), it breaks, if we let it without the new String and istead we put the string directly it works as expected

https://studio.webcomponents.dev/edit/xf3CaMMUCC9iAawJFI0q/README.md?branch=model-value-changed-bug%40klQNCGeRw3P6ibMcdqjGGS5ApcG2&p=README.md

Additional context

Currently using the version of Lion provided by studio.webcomponents.dev (it's happening in all versions I think)

@tlouisse
Copy link
Member

tlouisse commented Feb 10, 2023

@GeloCode I think what happens is this:

  • <lion-input .modelValue is set
  • @model-value-changed event is triggered (synchronously, not after a microtask like the regular update cycle)
  • this.requestUpdate(); is called, rerendering the template containing <lion-input .modelValue

Now, there are two scenarios:

  1. the hasChanged function (dirty check) returns true (in case of new Date(..) and new String())
  2. the hasChanged function (dirty check) returns false (in case when same string is provided)

In case 1, @model-value-changed is fired again. Now we enter an infinite, sync loop.

How to solve this?

<lion-input-datepicker
        label="MinMaxDate"
        .modelValue=${new Date('2023-01-19')}
        @model-value-changed=${({detail}) => {
          if (detail.isTriggeredByUser) {
            this.requestUpdate();
          }
        }}
      ></lion-input-datepicker>

See: https://lion-web.netlify.app/fundamentals/systems/form/model-value/#event-meta

@GeloCode
Copy link
Author

@tlouisse Hi there, even if you try the solution you'll see that the input-datepicker it's not getting his value updated, I'll share a screen trying your solution putting a debugger to check the value
image

As you can see I've tried to put 1/02/2023 but instead the value it's not getting updated check this 2nd image
image

@tlouisse
Copy link
Member

tlouisse commented Feb 10, 2023

Yeah, this makes sense actually:

  • we do: <input-x .modelValue=${new ValueNeverPassingDirtyCheck()}
  • on every render (triggered by requestUpdate), this will set this modelValue again... in your case new Date('2023-01-19') will be set again

A way to work around this:

const myModel = {
   myDatePicker: new Date('2023-01-19'),
}

// inside your component render...
html`
<input-datepicker 
  name="myDatePicker" 
  .modelValue="${myModel.myDatePicker}" 
  @model-value-changed="${(event) => {
    myModel.myDatePicker = event.target.modelValue;
  }}">
</input-datepicker>
`

Reminder for ourselves: we should create a whole set of best practices on writing forms... 😄
For best DX, we can provide a model directive that does the above.
Smth like:

html`<input-datepicker 
  name="myDatePicker" 
  ${model(myModel.myDatePicker)}>
</input-datepicker>`

Another approach

If you're just looking for a non dynamic initialization value, you could do:

html`<input-datepicker serialized-value="2023-01-19" ...>`

@GeloCode
Copy link
Author

It worked with your first work around I'm still doing the request update because I need it but seems fine right now.

Later on I'll check it on my application.

Thank you :)

@tlouisse
Copy link
Member

Nice that it worked for you!

Another reminder for ourselves:

  • make sure input-datepicker cannot generate infinite loops by applying hasChanged on modelValue with an equalsDate check

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

No branches or pull requests

2 participants