vaadin / flow

Vaadin Flow is a Java framework binding Vaadin web components to Java. This is part of Vaadin 10+.
Apache License 2.0
618 stars 167 forks source link

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

Closed alexanoid closed 3 weeks ago

alexanoid commented 2 months ago

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 beforeEntermethod, 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 https://github.com/vaadin/flow/issues/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

alexanoid commented 1 month ago

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

tepi commented 1 month ago

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 commented 1 month ago

Thank you! Unfortunately, along with this issue, I am also blocked from updating with this one as well https://github.com/vaadin/flow/issues/19823

alexanoid commented 1 month ago

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

tepi commented 1 month ago

@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 commented 1 month ago

@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 commented 1 month ago

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

vaadin-bot commented 1 month ago

This ticket/PR has been released with Vaadin 24.4.13.

alexanoid commented 1 month ago

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#setParameteris still present. Instead of 3 times, the number of calls has been reduced to 2. Something is still not right.

tepi commented 1 month ago

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#setParameteris 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 commented 1 month ago

@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—setParameteris only called once.

But I also have public class MainLayout extends AppLayout, and in it, I create Tabsin 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 setParametermethod.

I hope this helps you localize the issue.

tepi commented 1 month ago

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.

alexanoid commented 3 weeks ago

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

tepi commented 3 weeks ago

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.