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

Server roundtrip on every key stroke in invalid fields #4986

Closed
leluna opened this issue Apr 25, 2023 · 19 comments
Closed

Server roundtrip on every key stroke in invalid fields #4986

leluna opened this issue Apr 25, 2023 · 19 comments
Assignees
Labels
enhancement New feature or request validation

Comments

@leluna
Copy link

leluna commented Apr 25, 2023

Description

Typing in a invalid field triggers a server request on each key stroke, until next blur/enter. This even happens if the invalid attribute is manually set in the DOM in the browser.

Expected outcome

I would expect to have a server request on value change/blur, same behavior as if the field is not invalid.

Minimal reproducible example

Go to https://vaadin.com/docs/latest/components/number-field or https://vaadin.com/docs/latest/components/text-field with constraints

Steps to reproduce

In the number field:

  • Type in 123ef, leave the field. Now the field is red (invalid)
  • Click in the field again and type backspace. Observe the network traffic. Each key stroke now triggers a request.
  • This continues to happen, even if there are only valid characters left.

Environment

Vaadin version(s): 24.0.3
OS: Windows

Browsers

Chrome, Firefox

@yuriy-fix
Copy link
Contributor

Related ticket: #4645.
Dear @leluna, are you facing the issue with the performance or are there any other consequences?

@yuriy-fix yuriy-fix added the waiting for author Further information is requested label Apr 26, 2023
@leluna
Copy link
Author

leluna commented Apr 28, 2023

@yuriy-fix Not yet in my small test application. However, I don't know if this will be a problem for our bigger applications.

Is there any issue which explains the necessity for the round trips? As stated in the description, the validations are not even updated

@yuriy-fix yuriy-fix removed the waiting for author Further information is requested label May 17, 2023
@vursen
Copy link
Contributor

vursen commented May 23, 2023

I checked and the web component does indeed request the server to revalidate with each keystroke when invalid. However, the validation has no effect as the new value is only sent to the server on blur.

We discussed it internally and reached a consensus to remove this revalidation logic since it currently brings more inconsistency than improvement.

The right way to achieve eager revalidation is to use ValueChangeMode.EAGER that will both update and revalidate the value on each key stroke.

@jouni
Copy link
Member

jouni commented May 23, 2023

Shouldn't the fix only affect Flow, not the web component behavior? I mean, it has been 100% intentional, that when a field is invalid, every change to the input causes a revalidation, to give immediate feedback to the user when the input is valid, without the need to press enter or blur the field.

@web-padawan
Copy link
Member

I mean, it has been 100% intentional, that when a field is invalid, every change to the input causes a revalidation, to give immediate feedback to the user when the input is valid, without the need to press enter or blur the field.

Yes, I can confirm this. That's handled by validated event fired by the web component and used by the server.

@jouni
Copy link
Member

jouni commented May 23, 2023

Let me try again to get this straight. Please correct me if these two statements are incorrect:

  • The issue reported here is that there are too many server request when an invalid field is edited (one for each changed character).
  • The proposed fix is to change the behavior of the web component, to not validate an invalid field on every changed character (the input event).

I’m asking, if the fix should rather be to throttle the validated event (e.g., after a fixed timeout like 1–3 seconds), or perhaps postpone it completely when used together with Flow, and only rely on the change event to trigger the validated event.

I would not want to change the web component behavior so that the user does not get immediate feedback when a field becomes valid when they are editing it.

@yuriy-fix yuriy-fix added the enhancement New feature or request label Jul 11, 2023
@simasch
Copy link

simasch commented Aug 2, 2023

We have the same issue.

If the content is invalid, we open a Dialog where the user can choose the correct value. Usually, this happens on blur, but if the field is invalid with every key stroke.

This isn't very pleasant because this prevents me from migrating to Vaadin 24!

@jcgueriaud1
Copy link
Contributor

If you try this project: https://github.com/jcgueriaud1/spring-proxy-issue/tree/field-issue

Type ee in the name field then blur then type 12345 then the validator attached to the field is called like this:

Validate name
Validate name
Validate ee
Validate ee
Validate ee
Validate ee
Validate ee
Validate ee
Validate ee
Validate ee
Validate ee
Validate ee
Validate ee
Validate ee
Validate ee
Validate ee12345
Validate ee12345

This should probably be called like this:

Validate name
Validate ee
Validate ee12345

@TatuLund
Copy link
Contributor

TatuLund commented Aug 3, 2023

One option to optimize would to fire validated only when necessary

vaadin/web-components#6294

@jcgueriaud1
Copy link
Contributor

The validator is called multiple times, for example twice Validate name at the beginning instead of one.

It's also called multiple times Validate ee with the wrong value (the value on the client side is ee1 then ee12,...) since the server is yet synchronized (the textfield is not eagerly synchronized).
A debouncer won't help since the server need the correct value to make the validation.

The validator should validate the correct value and to have the correct value on the server side it must run when the value is updated on the server side.

@jcgueriaud1
Copy link
Contributor

One option to optimize would to fire validated only when necessary

But the fix will run the validation on the outdated value?

@vursen
Copy link
Contributor

vursen commented Aug 3, 2023

One option to optimize would to fire validated only when necessary

This optimization doesn't seem correct because, while the field remains invalid, it can be caused by different reasons, requiring different error messages.

Example:

  1. Enter foo into a required date-picker => invalid due to bad input
  2. Remove "foo" => invalid due to an empty value

@TatuLund
Copy link
Contributor

TatuLund commented Aug 3, 2023

requiring different error messages.

Potentially yes, but in our current implementation the user indication is empty error message, and field shakes and goes invalid state

@mstahv
Copy link
Member

mstahv commented Aug 14, 2023

I don't think this is "enhancement". This is a bug and a regression.

@yuriy-fix
Copy link
Contributor

We are currently refactoring the validation logic of triggering the validation from the client side. The aim is to reduce the number of roundtrips and validation invocations.

@mstahv
Copy link
Member

mstahv commented Aug 23, 2023

Excellent! These are important things for both actual performance and marketing (evaluators are afraid of "chattiness" because of our architecture, and they do check what goes between client & server).

BTW. Related Flow bugfix went in yesterday: vaadin/flow#17410 . I hope that helps your efforts.

@yuriy-fix
Copy link
Contributor

I have opened a discussion on the related topic: https://github.com/orgs/vaadin/discussions/4434#discussion-5571216

@vursen
Copy link
Contributor

vursen commented Nov 6, 2023

The issue will be resolved as part of #5537.

@yuriy-fix
Copy link
Contributor

Issue is resolved as a part of #5537. Fix should be available in 24.3.0-beta1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request validation
Projects
None yet
9 participants