Synchronized property unexpectedly reverts to last synchronized state

[vursen]

Description of the bug

Flow reverts the property’s value to the last synchronized state if the property changes again while the previous update is scheduled or is still being processed on the server. This behavior has blocked progress on DatePicker fallback parser feature.

Screen.Recording.2024-10-28.at.18.12.51.mov

Expected behavior

I'd expect Flow to keep the new value and wait for the next synchronization event to send it to the server.

Minimal reproducible example

// frontend/my-field/my-field.ts

import { html, LitElement } from 'lit';
import { customElement, query } from 'lit/decorators.js';

@customElement("my-field")
class MyField extends LitElement {
  @query('input')
  inputElement!: HTMLInputElement;

  get inputElementValue() {
    return this.inputElement.value;
  }

  set inputElementValue(value) {
    console.warn('set inputElementValue', value);
    this.inputElement.value = value;
  }

  onChange() {
    this.dispatchEvent(new CustomEvent('change', { bubbles: true, composed: true }));
    this.inputElementValue = 'foobar'; // <------ Change the property right after the synchronization event
  }

  render() {
    return html`<input @change="${this.onChange}" />`
  }
}

@Tag("my-field")
@JsModule("my-field/my-field.ts")
public class MyField extends Component {
    @Synchronize(property = "inputElementValue", value = { "change" })
    public String getInputElementValue() {
        return getElement().getProperty("inputElementValue", "");
    }
}

@Route("my-field")
public class MyFieldView extends Div {
    public MyFieldView() {
        MyField myField = new MyField();
        add(myField);
    }
}

Versions

• Vaadin / Flow version: 24.5 and possibly earlier
• Java version: 17
• OS version: Mac OS

[vursen]

Here is a better example. I can reproduce the bug using a TextField on a slow internet connection:

TextField textField = new TextField();
add(textField);

1. Enable network throttling in browser devtools
2. Type "test" and press Enter
3. After a round-trip begins, press Backspace
4. Result: The backspace is discarded when the round-trip is completed

Screen.Recording.2024-11-04.at.14.26.58.mov

However, if I follow Backspace with Enter while the previous round-trip is going on, another round-trip is scheduled, which applies the backspace correctly.

[tepi]

The client-side property binding in Flow works in this way. The server round-trip is done asynchronously, and once it's done, flow client does a flush to make sure all the bound properties match the values that were sent to the server. It has been this way for years and I'm pretty sure I've seen that slow network example in something like Vaadin 6/7/8 (provided that network is slow enough or user is fast enough to type). I don't think we can change the basic functionality without breaking a lot of things in the process. So I'd say we need to provide some kind of workaround for the use case needed here.

I have a clarifying question though, in the first example of

this.dispatchEvent(new CustomEvent('change', { bubbles: true, composed: true }));
this.inputElementValue = 'foobar';

what should happen? The dispatched event triggers a server round-trip. Should the value change on the next line trigger another server round-trip right after the first one has finished? Or should it not trigger a server round-trip, leading to missynchronized values between server and client? Or some other option I did not think of?

[vursen]

Or should it not trigger a server round-trip, leading to missynchronized values between server and client

It should not trigger an immediate round-trip and rather keep the new value until the next change event. There will be temporary desync, but the property will sync eventually. This is how I see it.

[tepi]

Alright, so that means we would need to remove the whole synced property flush process that is done after the server round-trip finishes. I can try that and see what all breaks. Pinging @Legioth as well as he might have some better ideas here as well

[tepi]

@vursen Could you try your case(s) using https://github.com/vaadin/flow/tree/fix/flush-logic? Locally tested seems to fix the slow network case and did not break any flow-client tests.

Edit: some shortcut key test is failing but everything else is passing

[mshabarov]

The second showcase gives a better presentation of the issues. It's looks now end-user affecting especially with the slow internet. I'm leaning towards this is the bug in Flow, no matter what the solution would be. As a end-user, I don't want my new value of the component to be overridden by the previous one.

[tepi]

The second showcase gives a better presentation of the issues. It's looks now end-user affecting especially with the slow internet. I'm leaning towards this is the bug in Flow, no matter what the solution would be. As a end-user, I don't want my new value of the component to be overridden by the previous one.

This also opens another can of worms since arbitrary business logic can do whatever kind of processing for the input value on server side and send it back in the response. I guess in such case server-sent value should still override whatever was input by the user after the round-trip started?

[vursen]

Could you try your case(s) using https://github.com/vaadin/flow/tree/fix/flush-logic? Locally tested seems to fix the slow network case and did not break any flow-client tests.

Unfortunately, I don't see any changes with your branch. I tested both DatePicker and TextField.

This also opens another can of worms since arbitrary business logic can do whatever kind of processing for the input value on server side and send it back in the response. I guess in such case server-sent value should still override whatever was input by the user after the round-trip started?

I would expect it to override.

[tepi]

Thanks @vursen, I'll keep working on a fix. Is it enough to test with vaadin/flow-components#6743 or do I need a custom version of web-components as well?

[vursen]

I'll keep working on a fix. Is it enough to test with vaadin/flow-components#6743 or do I need a custom version of web-components as well?

Thank you. Yes, that's enough. This feature affects only the Flow component. Here is the test that reproduces the issue: enterParsableValue_enterShortcutValue

[mshabarov]

I guess in such case server-sent value should still override whatever was input by the user after the round-trip started?

Yes, I think so.
Server sends no changes -> keep the value on client, no override
Server sends a change -> override, even if user changed the value meanwhile.

[tepi]

Re-pinging @Legioth in case he has any fresh ideas.

My current problem is that when the value is changed during round-trip but before flush, we do not get any kind of event about that to flow-client. Due to that I could not figure out a way to detect if the value was changed by the user (and should not be recomputed) or if it is invalid/stale for some other reason (and should be recomputed). Could not find anything in client side binding or flush/post-flush listeners to help with this either.

[Legioth]

I don't remember what the implementation does but one potential approach might be to keep a timestamp for when a synchronized value was last sent to the server and when an updated value was last received from the server. We should re-apply only if the received value is newer than the sent value.

Might actually be overkill to have a timestamp. Could just as well have a boolean to track whichever has happened most recently.

[vaadin-bot]

This ticket/PR has been released with Vaadin 24.6.0.alpha3 and is also targeting the upcoming stable 24.6.0 version.

[vaadin-bot]

This ticket/PR has been released with Vaadin 24.4.17.

[vaadin-bot]

This ticket/PR has been released with Vaadin 24.5.5.