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

HasUrlParameter.setParameter is executed twice with vaadin.eagerServerLoad=true #16046

Closed
alexanoid opened this issue Feb 26, 2023 · 21 comments · Fixed by #16498
Closed

HasUrlParameter.setParameter is executed twice with vaadin.eagerServerLoad=true #16046

alexanoid opened this issue Feb 26, 2023 · 21 comments · Fixed by #16498

Comments

@alexanoid
Copy link

alexanoid commented Feb 26, 2023

Description of the bug

I have a View which implements HasUrlParameter<String>.

Also I configured:

vaadin.eagerServerLoad=true (for IndexHtmlRequestListener)

with such setup, HasUrlParameter.setParameter() method is executed twice during the first page load

Expected behavior

HasUrlParameter.setParameter() method is executed only one time during the first page load with vaadin.eagerServerLoad=true

Versions

  • Vaadin / Flow version: 23.3.6
@alexanoid
Copy link
Author

the same relates to BeforeEnterObserver.beforeEnter() https://stackoverflow.com/questions/75166305/vaadin23-vaadin-eagerserverload-true-and-beforeenterobserver

@alexanoid
Copy link
Author

alexanoid commented Mar 1, 2023

My biggest issue with this is that I have some pretty expensive business logic triggered by HasUrlParameter.setParameter() and thus - I access the database twice, which impacts the performance of the application :( Please help!

@knoobie
Copy link
Contributor

knoobie commented Mar 1, 2023

If you know it's always two times called (has to be checked), just a add counter or AtomicBoolean as class field that checks if this is the second call as temporary workaround?

@alexanoid
Copy link
Author

alexanoid commented Mar 1, 2023

Thank you for your reply. The problem with the proposed counter is that the setParameter() method is called twice only on the very first page load... then when the UI loaded and I use the router navigation, eg leave this page and come back - this is correctly called only once. But.. it may be the new instance of the View each time (on each page visit).. so potentially it may work! I have to verify it! Thanks!

@alexanoid
Copy link
Author

@knoobie thank you very much! Your workaround with AtomicBoolean did the trick, thanks!

@alexanoid
Copy link
Author

alexanoid commented Mar 3, 2023

@knoobie There is one side effect with this approach - the Router links don't work after the page is loaded. For example, I have a Product Catalog View with multiple pages. When I'm on the page #2 or more, it is very convenient to click on the Product catalog router link to quickly return to the page #1. The view.seParameter() method is executed correctly, but because of the AtomicBoolean == true, the logic inside is skipped, and the page is not updated in this case.

I'll really appreciate if you could fix this issue on your end.

@alexanoid
Copy link
Author

Just FYI - I don't know is it related somehow, but during initial start up of the application I may very often see resync issue:

2023-03-16T00:23:51.078+02:00 WARN 21484 --- [ XNIO-1 task-6] c.v.f.s.communication.ServerRpcHandler : Resynchronizing UI by client's request. A network message was lost before reaching the client and the client is reloading the full UI state. This typically happens because of a bad network connection with packet loss or because of some part of the network infrastructure (load balancer, proxy) terminating a push (websocket or long-polling) connection. If you are using push with a proxy, make sure the push timeout is set to be smaller than the proxy connection timeout

@alexanoid
Copy link
Author

any chance to fix this problem soon?

@knoobie
Copy link
Contributor

knoobie commented Mar 31, 2023

I don't think it has any priority at the moment looking at the impact because it occurs only with eagerServerLoad=true which in turn is a rarely used feature.

@alexanoid
Copy link
Author

That's sad ( Is there any other way right now to dynamically change meta tags other than with help of vaadin.eagerServerLoad=true and IndexHtmlRequestListener ?

@Artur-
Copy link
Member

Artur- commented Mar 31, 2023

I tried a fix for this but there seems to be another underlying problem of the eagerly provided UIDL not being used at all..

@alexanoid
Copy link
Author

I tried a fix for this but there seems to be another underlying problem of the eagerly provided UIDL not being used at all..

thank you! I'll really appreciate if you could fix it. It causes a lot of issues to me right now (

@knoobie
Copy link
Contributor

knoobie commented Mar 31, 2023

@Artur- #13421 this is the cause of the usage of eager=true if you wanna try to fix this instead.. it would probably help more people as well 😅

@Artur-
Copy link
Member

Artur- commented Mar 31, 2023

Sure, if you tell how. If the routing happens only on the second request, it is a bit too late to write the response code for the first..

Artur- added a commit that referenced this issue Apr 1, 2023
Changes bootstrap for eager mode so that the index page contains the <flow-container-[appId]> div and the virtual child for this element is created eagerly on the server. This allows the initial render on the client side to put the content inside the flow-container instead of document body as happened before.

By using a convention on how the container div is created and id is set, we no longer need to communicate the element tag and id from the client.

Fixes #16046
Artur- added a commit that referenced this issue Apr 1, 2023
Changes bootstrap for eager mode so that the index page contains the <flow-container-[appId]> div and the virtual child for this element is created eagerly on the server. This allows the initial render on the client side to put the content inside the flow-container instead of document body as happened before.

By using a convention on how the container div is created and id is set, we no longer need to communicate the element tag and id from the client.

Fixes #16046
Artur- added a commit that referenced this issue Apr 1, 2023
Changes bootstrap for eager mode so that the index page contains the <flow-container-[appId]> div and the virtual child for this element is created eagerly on the server. This allows the initial render on the client side to put the content inside the flow-container instead of document body as happened before.

By using a convention on how the container div is created and id is set, we no longer need to communicate the element tag and id from the client.

Fixes #16046
Artur- added a commit that referenced this issue Apr 1, 2023
Changes bootstrap for eager mode so that the index page contains the <flow-container-[appId]> div and the virtual child for this element is created eagerly on the server. This allows the initial render on the client side to put the content inside the flow-container instead of document body as happened before.

By using a convention on how the container div is created and id is set, we no longer need to communicate the element tag and id from the client.

Fixes #16046
Artur- added a commit that referenced this issue Apr 1, 2023
Changes bootstrap for eager mode so that the index page contains the <flow-container-[appId]> div and the virtual child for this element is created eagerly on the server. This allows the initial render on the client side to put the content inside the flow-container instead of document body as happened before.

By using a convention on how the container div is created and id is set, we no longer need to communicate the element tag and id from the client.

Fixes #16046
@alexanoid
Copy link
Author

@Artur- I see the number of associated commits, what a great news, thank you! Does this mean that potentially this issue will be fixed with help of them? If so, will it be included into Vaadin Flow 24.0.3?

Artur- added a commit that referenced this issue Apr 2, 2023
Changes bootstrap for eager mode so that the index page contains the <flow-container-[appId]> div and the virtual child for this element is created eagerly on the server. This allows the initial render on the client side to put the content inside the flow-container instead of document body as happened before.

By using a convention on how the container div is created and id is set, we no longer need to communicate the element tag and id from the client.

Fixes #16046
Artur- added a commit that referenced this issue Apr 2, 2023
Changes bootstrap for eager mode so that the index page contains the <flow-container-[appId]> div and the virtual child for this element is created eagerly on the server. This allows the initial render on the client side to put the content inside the flow-container instead of document body as happened before.

By using a convention on how the container div is created and id is set, we no longer need to communicate the element tag and id from the client.

Fixes #16046
@Artur-
Copy link
Member

Artur- commented Apr 2, 2023

#16498 fixes this indeed. It will not make it to 24.0.3 but hopefully the following release

@alexanoid
Copy link
Author

Awesome news! Thank you very much!

Artur- added a commit that referenced this issue Apr 3, 2023
Changes bootstrap for eager mode so that the index page contains the <flow-container-[appId]> div and the virtual child for this element is created eagerly on the server. This allows the initial render on the client side to put the content inside the flow-container instead of document body as happened before.

By using a convention on how the container div is created and id is set, we no longer need to communicate the element tag and id from the client.

Fixes #16046
mcollovati pushed a commit that referenced this issue Apr 4, 2023
)

Changes bootstrap for eager mode so that the index page contains the <flow-container-[appId]> div and the virtual child for this element is created eagerly on the server. This allows the initial render on the client side to put the content inside the flow-container instead of document body as happened before.

By using a convention on how the container div is created and id is set, we no longer need to communicate the element tag and id from the client.

Fixes #16046
OLD Vaadin Flow bugs & maintenance (Vaadin 10+) automation moved this from New P2 to Closed Apr 4, 2023
@alexanoid
Copy link
Author

Hi @Artur- I just updated to Vaadin 24.0.4 Looks like the fix still hasn't been added there?

@knoobie
Copy link
Contributor

knoobie commented Apr 11, 2023

It's not backported

@alexanoid
Copy link
Author

Sorry to bother you with this, but do you have any information on when this will be released? I'm just waiting for this fix more than my own birthday :)

@vaadin-bot
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment