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

Multiple redundant invocations of the method com.vaadin.flow.router.HasUrlParameter#setParameter in Vaadin 24.4.9 #19822

Closed
alexanoid opened this issue Aug 24, 2024 · 14 comments · Fixed by #19959 or #20187

Comments

@alexanoid
Copy link

alexanoid commented Aug 24, 2024

Description of the bug

I have an AdministrationView:

@Route(value = "administration", layout = AdministrationLayout.class)
@PermitAll
public class AdministrationView extends BaseView implements BeforeEnterObserver {

    @Override
    public void beforeEnter(BeforeEnterEvent event) {

        removeAll();

        event.forwardTo(AdministrationRecruitersView.class);
    }

}

Available at the following URL:

http://localhost:8080/administration

As you can see, in the beforeEnter method, there is a forward to AdministrationRecruitersView.class.

The AdministrationRecruitersView.class itself is implemented as:

@Route(value = "administration/recruiters", layout = AdministrationLayout.class)
@PermitAll
public class AdministrationRecruitersView extends CatalogContainer<DecisionMatrix> implements HasUrlParameter<String> {

    @Override
    public void setParameter(BeforeEvent event, @OptionalParameter String jobIdParameter) {
    }

}

So, when navigating to AdministrationView (by http://localhost:8080/administration) and subsequently auto forwarding to AdministrationRecruitersView, the method AdministrationRecruitersView.setParameter() is called 3 times in a row. If I navigate directly to AdministrationRecruitersView, i.e., to http://localhost:8080/administration/recruiters, the method AdministrationRecruitersView.setParameter() is correctly called only once, as it should be.

This issue started appearing in version 24.4.9, while in version 24.4.0.beta5 everything works correctly. I haven't changed anything in my code.

Also, in version 24.4.9, the bug from #18736 has reappeared, but it’s not present in version 24.4.0.beta5, where everything works fine.

Expected behavior

no redundant invocation of com.vaadin.flow.router.HasUrlParameter#setParameter

Minimal reproducible example

n/a

Versions

  • Vaadin / Flow version: 24.4.9
  • Java version: 17
@alexanoid
Copy link
Author

Sorry to bother you, but is there a chance this will be fixed anytime soon?

@tepi
Copy link
Contributor

tepi commented Sep 11, 2024

Seems this is another issue with the react-router performing the full navigation cycle on server side when forwarding is used, and after that doing a ui-navigate event callback to the server which triggers the setParameter for a second time. That said, in my testing I only got two calls to setParameter, not three.

We are looking into these problems with react-router. In the meantime, if it is an option for you, please try setting system parameter vaadin.react.enable=false to revert back to using vaadin router. That seems to work correctly and produce only one of each calls as it should.

@alexanoid
Copy link
Author

Thank you! Unfortunately, along with this issue, I am also blocked from updating with this one as well #19823

caalador pushed a commit that referenced this issue Sep 18, 2024
Fixes #19794
Fixes #19822

Also fixes #19813 in a more coherent way. Relevant test is com.vaadin.flow.uitest.ui.PreserveOnRefreshForwardingIT
@alexanoid
Copy link
Author

@tepi Hi! Could you please tell me in which version this fix will be released?

@tepi
Copy link
Contributor

tepi commented Sep 25, 2024

@tepi Hi! Could you please tell me in which version this fix will be released?

Hi! The fix is released in 24.5.0.beta2 and will be released in Flow 24.4.9 which should be released within a week as part of Vaadin 24.4.13.

@alexanoid
Copy link
Author

@tepi Thank you! Sorry for bothering you with additional questions – but do I need to change anything in the code to switch from version 24.4.* to 24.5.* or should it go smoothly?

@tepi
Copy link
Contributor

tepi commented Sep 25, 2024

There should not be breaking changes between those versions, version update is enough.

@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 24.4.13.

@alexanoid
Copy link
Author

Hi @tepi

I just upgraded to Vaadin 24.4.13, and unfortunately, the issue with the repeated triggering of com.vaadin.flow.router.HasUrlParameter#setParameter#setParameter is still present. Instead of 3 times, the number of calls has been reduced to 2. Something is still not right.

@tepi
Copy link
Contributor

tepi commented Oct 2, 2024

Hi @tepi

I just upgraded to Vaadin 24.4.13, and unfortunately, the issue with the repeated triggering of com.vaadin.flow.router.HasUrlParameter#setParameter#setParameter is still present. Instead of 3 times, the number of calls has been reduced to 2. Something is still not right.

Could you provide some more context on how to reproduce the issue? Using Vaadin 24.4.13 and the original example from this ticket, as well as the test built for this issue, there is only one call. So I suspect that there is something in your case that has not been revealed in this ticket yet.

@alexanoid
Copy link
Author

alexanoid commented Oct 2, 2024

@tepi I think I've localized the problem. In my previous example (with which I created this ticket), everything remains the same. If I simply directly go to localhost:8080/administration in the browser address bar, everything works correctly, even with forwarding—setParameter is only called once.

But I also have public class MainLayout extends AppLayout, and in it, I create Tabs in the Drawer, where I add, for example:

RouterLink link = new RouterLink("Test Link", AdministrationView.class);
tabs.add(new Tab(link));
addToDrawer(tabs);

And now, when I click on this RouterLink, I get two calls to the setParameter method.

I hope this helps you localize the issue.

@tepi tepi reopened this Oct 2, 2024
@tepi
Copy link
Contributor

tepi commented Oct 4, 2024

Confirmed. Judging from browser console, all seems to be fine in the uidl response: a vaadin-navigate event is dispatched with target first and callback false. Yet after this, something anyways makes the callback to server, causing the second round of beforeEnter/setParameter calls. I'll investigate this sometime next week.

vaadin-bot pushed a commit that referenced this issue Oct 9, 2024
@alexanoid
Copy link
Author

@tepi Thank you very much for correcting this! Could you please let me know in which release version this fix is expected?

@tepi
Copy link
Contributor

tepi commented Oct 9, 2024

Thanks to @mcollovati for the latest fix. This will be released in next 24.4 and 24.5 branch releases; Vaadin 24.4.14 and the first RC for Vaadin 24.5. Both should happen within a week.

vaadin-bot added a commit that referenced this issue Oct 9, 2024
vaadin-bot added a commit that referenced this issue Oct 9, 2024
…tion is completed (#20187) (CP: 24.4) (#20196)

* fix: perform server navigation roundtrip only when client side navigation is completed (#20187)

Fixes #19822

* format

---------

Co-authored-by: Marco Collovati <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment