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

In Vaadin 24.4.0.alpha18, RouterLink does not update the browser URL #19080

Closed
alexanoid opened this issue Apr 2, 2024 · 11 comments · Fixed by #19087
Closed

In Vaadin 24.4.0.alpha18, RouterLink does not update the browser URL #19080

alexanoid opened this issue Apr 2, 2024 · 11 comments · Fixed by #19087
Assignees

Comments

@alexanoid
Copy link

Description of the bug

In Vaadin 24.4.0.alpha17 everything is working fine, but in Vaadin 24.4.0.alpha18, clicking on RouterLinks does not update the browser URL.

For example, I stay on

http://localhost:8080/notifications?status=pending

Then, clicking on the RouterLink with the following target:

http://localhost:8080/?lang=en

After that, I successfully moved to http://localhost:8080/?lang=en, but in the browser address bar, I still see http://localhost:8080/notifications?status=pending

In Vaadin 24.4.0.alpha17, everything is working fine

Expected behavior

browser url in address bar should change to the correct one

Minimal reproducible example

n/a

Versions

  • Vaadin / Flow version: Vaadin 24.4.0.alpha18
  • Java version: 17
@caalador
Copy link
Contributor

caalador commented Apr 2, 2024

Could you add sample views with navigation that replicates the issue as I could not replicate this using RouterLinks

@alexanoid
Copy link
Author

You can see this issue on the website https://decisionwanted.com/jobs

Now click on the Main:

router_issue

The URL in the address bar will not change

@alexanoid
Copy link
Author

alexanoid commented Apr 2, 2024

@caalador additional information: I am using a RouterLink inside the Tab, not a plain RouterLink:

    RouterLink link = new RouterLink();
    link.setRoute(navigationTarget);
    link.add(icon, new Span(viewName));
    new Tab(link);

Could the combination of RouterLink with Tab be the reason for this issue in 24.4.0.alpha18?

UPDATED,

No, this is not the case.. the RouterLink without Tab also doesn't work

@alexanoid
Copy link
Author

alexanoid commented Apr 2, 2024

I see changes were made to the navigation logic at:

5c58117

It appears that the reason for this issue may be somewhere within those changes, potentially in com.vaadin.flow.component.UI#browserNavigate method

Also I see InternalRedirectHandler ( with some related logic in handle() method) in com.vaadin.flow.router.Router#handleNavigation but this method is not invoked when I click on RouterLinks

@caalador
Copy link
Contributor

caalador commented Apr 3, 2024

That change should specifically fix also this issue and looking at the site index I can not see in the sources the continue method that should be available there. So I see there is

{
        prevent() {
            window.history.pushState(window.history.state, "", gd),
            window.dispatchEvent(new PopStateEvent("popstate",{
                state: "vaadin-router-ignore"
            }))
        },
        redirect: n=>{
            Ci(n, {
                replace: !1
            })
        }
    }

but it should have a continue clause

{ 
    prevent() { 
        window.history.pushState(window.history.state, "", Nc), 
        window.dispatchEvent(new PopStateEvent("popstate", { state: "vaadin-router-ignore" })) 
    }, 
    redirect: r => { 
        ii(r, { replace: !1 }) 
    },
    continue: () => { 
        window.location.pathname !== e.detail.pathname && 
        (window.history.pushState(window.history.state, "", e.detail.pathname), 
        window.dispatchEvent(new PopStateEvent("popstate", { state: "vaadin-router-ignore" }))) 
    } 
}

It would seem there has been some missmatch in the sources on build time.
Try creating a new clean build and check that the plugin is also updated.

So after the build check the frontend/generated/flow/Flow.tsx file that it contains a continue on line 189 continue: () => {

@alexanoid
Copy link
Author

alexanoid commented Apr 3, 2024

I removed the 'generated' folder and rebuilt the project. After that, I verified that Flow.tsx contains:

continue: () => {
                        if(window.location.pathname !== event.detail.pathname) {
                            window.history.pushState(window.history.state, '', event.detail.pathname);
                            window.dispatchEvent(new PopStateEvent('popstate', {state: 'vaadin-router-ignore'}));
                        }
                    }

but it still doesn't work.

Also, during the application work, I may see the following warning:

.r.RouteUnifyingIndexHtmlRequestListener : Failed to find views.json under D:\Projects\decisionwanted-interview\DecisionWanted\ui\.\frontend\generated

I also do not see views.js there, only views.ts

@caalador
Copy link
Contributor

caalador commented Apr 3, 2024

Do you have a application bundle? It might not be rebuilt so that the new part would be added.
I'll test if the bundle logic works correctly on this part, but you could try removing the src/main/bundles to get a new bundle build.

@alexanoid
Copy link
Author

Thanks, I removed src/main/bundles and they were rebuilt now.

Now, when I click on the RouterLink, the URL in the address bar is changed correctly, but there is a new error: the view itself is not found - 'The requested page was not found.

And I still have the followig warning: .r.RouteUnifyingIndexHtmlRequestListener : Failed to find views.json under D:\Projects\decisionwanted-interview\DecisionWanted\ui\.\frontend\generated

@alexanoid
Copy link
Author

I downgraded to Vaadin 24.4.0.alpha17, and now everything is working fine again.

caalador added a commit that referenced this issue Apr 3, 2024
Fix mistaken add of query
parameters into the route
breaking route resolution
when navigating thrhough link.

Fixes #19080
caalador added a commit that referenced this issue Apr 3, 2024
Fix mistaken add of query
parameters into the route
breaking route resolution
when navigating thrhough link.

part of #19080
@caalador caalador self-assigned this Apr 3, 2024
caalador added a commit that referenced this issue Apr 3, 2024
Fix mistaken add of query
parameters into the route
breaking route resolution
when navigating thrhough link.

part of #19080
tltv pushed a commit that referenced this issue Apr 4, 2024
* fix: Query in path

Fix mistaken add of query
parameters into the route
breaking route resolution
when navigating thrhough link.

part of #19080

* update path normalizing, fix server replace state to use location

* change replace order
@alexanoid
Copy link
Author

@caalador Thanks for looking into this! In which version is this fix going to be released? in 24.4.0.alpha21 ?

@caalador
Copy link
Contributor

caalador commented Apr 5, 2024

It will be going out with Flow 24.4.0.alpha27 which should be in Vaadin 24.4.0.alpha21
Thanks for testing with alphas so we can catch these issues also.

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

Successfully merging a pull request may close this issue.

3 participants