From c7e9fd3db97bba39684a182313b48f7529f2ab5a Mon Sep 17 00:00:00 2001 From: Anton Platonov Date: Mon, 30 Sep 2024 07:38:06 +0300 Subject: [PATCH] fix: queued navigate with React Router (#19985) Fixes #19839 --------- Co-authored-by: caalador Co-authored-by: Teppo Kurki --- .../com/vaadin/flow/server/frontend/Flow.tsx | 94 ++++++++++++-- .../com/vaadin/flow/BackNavFirstView.java | 30 +++++ .../com/vaadin/flow/BackNavSecondView.java | 45 +++++++ .../com/vaadin/flow/ForwardTargetView.java | 16 +++ .../flow/ForwardTargetWithParametersView.java | 16 +++ .../flow/ForwardingToParametersView.java | 16 +++ .../java/com/vaadin/flow/ForwardingView.java | 16 +++ .../test/java/com/vaadin/flow/BackNavIT.java | 121 ++++++++++++++++++ 8 files changed, 344 insertions(+), 10 deletions(-) create mode 100644 flow-tests/test-react-router/src/main/java/com/vaadin/flow/BackNavFirstView.java create mode 100644 flow-tests/test-react-router/src/main/java/com/vaadin/flow/BackNavSecondView.java create mode 100644 flow-tests/test-react-router/src/test/java/com/vaadin/flow/BackNavIT.java diff --git a/flow-server/src/main/resources/com/vaadin/flow/server/frontend/Flow.tsx b/flow-server/src/main/resources/com/vaadin/flow/server/frontend/Flow.tsx index 1883c734aac..5c9e29c1cb2 100644 --- a/flow-server/src/main/resources/com/vaadin/flow/server/frontend/Flow.tsx +++ b/flow-server/src/main/resources/com/vaadin/flow/server/frontend/Flow.tsx @@ -20,13 +20,15 @@ import React, { useEffect, useReducer, useRef, + useState, type ReactNode } from "react"; import { matchRoutes, useBlocker, useLocation, - useNavigate + useNavigate, + type NavigateOptions, } from "react-router-dom"; import type { AgnosticRouteObject } from '@remix-run/router'; import { createPortal } from "react-dom"; @@ -200,6 +202,68 @@ function portalsReducer(portals: readonly PortalEntry[], action: PortalAction) { } } + +type NavigateOpts = { + to: string, + callback: boolean, + opts?: NavigateOptions +}; + +type NavigateFn = (to: string, callback: boolean, opts?: NavigateOptions) => void; + +/** + * A hook providing the `navigate(path: string, opts?: NavigateOptions)` function + * with React Router API that has more consistent history updates. Uses internal + * queue for processing navigate calls. + */ +function useQueuedNavigate(waitReference: React.MutableRefObject | undefined>, navigated: React.MutableRefObject): NavigateFn { + const navigate = useNavigate(); + const navigateQueue = useRef([]).current; + const [navigateQueueLength, setNavigateQueueLength] = useState(0); + + const dequeueNavigation = useCallback(() => { + const navigateArgs = navigateQueue.shift(); + if (navigateArgs === undefined) { + // Empty queue, do nothing. + return; + } + + const blockingNavigate = async () => { + if (waitReference.current) { + await waitReference.current; + waitReference.current = undefined; + } + navigated.current = !navigateArgs.callback; + navigate(navigateArgs.to, navigateArgs.opts); + setNavigateQueueLength(navigateQueue.length); + } + blockingNavigate(); + }, [navigate, setNavigateQueueLength]); + + const dequeueNavigationAfterCurrentTask = useCallback(() => { + queueMicrotask(dequeueNavigation); + }, [dequeueNavigation]); + + const enqueueNavigation = useCallback((to: string, callback: boolean, opts?: NavigateOptions) => { + navigateQueue.push({to: to, callback: callback, opts: opts}); + setNavigateQueueLength(navigateQueue.length); + if (navigateQueue.length === 1) { + // The first navigation can be started right after any pending sync + // jobs, which could add more navigations to the queue. + dequeueNavigationAfterCurrentTask(); + } + }, [setNavigateQueueLength, dequeueNavigationAfterCurrentTask]); + + useEffect(() => () => { + // The Flow component has rendered, but history might not be + // updated yet, as React Router does it asynchronously. + // Use microtask callback for history consistency. + dequeueNavigationAfterCurrentTask(); + }, [navigateQueueLength, dequeueNavigationAfterCurrentTask]); + + return enqueueNavigation; +} + function Flow() { const ref = useRef(null); const navigate = useNavigate(); @@ -207,10 +271,12 @@ function Flow() { navigated.current = navigated.current || (nextLocation.pathname === currentLocation.pathname && nextLocation.search === currentLocation.search && nextLocation.hash === currentLocation.hash); return true; }); - const {pathname, search, hash} = useLocation(); + const location = useLocation(); const navigated = useRef(false); const fromAnchor = useRef(false); const containerRef = useRef(undefined); + const roundTrip = useRef | undefined>(undefined); + const queuedNavigate = useQueuedNavigate(roundTrip, navigated); // portalsReducer function is used as state outside the Flow component. const [portals, dispatchPortalAction] = useReducer(portalsReducer, []); @@ -261,9 +327,8 @@ function Flow() { // @ts-ignore window.Vaadin.Flow.navigation = true; const path = '/' + event.detail.url; - navigated.current = !event.detail.callback; fromAnchor.current = false; - navigate(path, { state: event.detail.state, replace: event.detail.replace}); + queuedNavigate(path, event.detail.callback, { state: event.detail.state, replace: event.detail.replace }); }, [navigate]); const redirect = useCallback((path: string) => { @@ -296,9 +361,13 @@ function Flow() { useEffect(() => { if (blocker.state === 'blocked') { + let blockingPromise: any; + roundTrip.current = new Promise((resolve,reject) => blockingPromise = {resolve:resolve,reject:reject}); + // Do not skip server round-trip if navigation originates from a click on a link if (navigated.current && !fromAnchor.current) { blocker.proceed(); + blockingPromise.resolve(); return; } fromAnchor.current = false; @@ -310,14 +379,16 @@ function Flow() { // @ts-ignore if (matched && matched.filter(path => path.route?.element?.type?.name === Flow.name).length != 0) { containerRef.current?.onBeforeEnter?.call(containerRef?.current, - {pathname,search}, { + {pathname, search}, { prevent() { blocker.reset(); + blockingPromise.resolve(); navigated.current = false; }, redirect, continue() { blocker.proceed(); + blockingPromise.resolve(); } }, router); navigated.current = true; @@ -333,13 +404,16 @@ function Flow() { containerRef.current.serverConnected = (cancel) => { if (cancel) { blocker.reset(); + blockingPromise.resolve(); } else { blocker.proceed(); + blockingPromise.resolve(); } } } else { // permitted navigation: proceed with the blocker blocker.proceed(); + blockingPromise.resolve(); } }); } @@ -349,10 +423,10 @@ function Flow() { useEffect(() => { if (navigated.current) { navigated.current = false; - fireNavigated(pathname,search); + fireNavigated(location.pathname,location.search); return; } - flow.serverSideRoutes[0].action({pathname, search}) + flow.serverSideRoutes[0].action({pathname: location.pathname, search: location.search}) .then((container) => { const outlet = ref.current?.parentNode; if (outlet && outlet !== container.parentNode) { @@ -361,15 +435,15 @@ function Flow() { window.addEventListener('click', navigateEventHandler); containerRef.current = container } - return container.onBeforeEnter?.call(container, {pathname, search}, {prevent, redirect, continue() { - fireNavigated(pathname,search);}}, router); + return container.onBeforeEnter?.call(container, {pathname: location.pathname, search: location.search}, {prevent, redirect, continue() { + fireNavigated(location.pathname,location.search);}}, router); }) .then((result: unknown) => { if (typeof result === "function") { result(); } }); - }, [pathname, search, hash]); + }, [location]); return <> diff --git a/flow-tests/test-react-router/src/main/java/com/vaadin/flow/BackNavFirstView.java b/flow-tests/test-react-router/src/main/java/com/vaadin/flow/BackNavFirstView.java new file mode 100644 index 00000000000..93d47748d8d --- /dev/null +++ b/flow-tests/test-react-router/src/main/java/com/vaadin/flow/BackNavFirstView.java @@ -0,0 +1,30 @@ +/* + * Copyright 2000-2024 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ + +package com.vaadin.flow; + +import com.vaadin.flow.component.html.Div; +import com.vaadin.flow.component.html.NativeButton; +import com.vaadin.flow.router.Route; + +@Route("com.vaadin.flow.BackNavFirstView") +public class BackNavFirstView extends Div { + + public BackNavFirstView() { + add(new NativeButton("Navigate", event -> getUI() + .ifPresent(ui -> ui.navigate(BackNavSecondView.class)))); + } +} diff --git a/flow-tests/test-react-router/src/main/java/com/vaadin/flow/BackNavSecondView.java b/flow-tests/test-react-router/src/main/java/com/vaadin/flow/BackNavSecondView.java new file mode 100644 index 00000000000..88b76d16cdd --- /dev/null +++ b/flow-tests/test-react-router/src/main/java/com/vaadin/flow/BackNavSecondView.java @@ -0,0 +1,45 @@ +/* + * Copyright 2000-2024 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ + +package com.vaadin.flow; + +import com.vaadin.flow.component.UI; +import com.vaadin.flow.component.html.Div; +import com.vaadin.flow.component.html.Span; +import com.vaadin.flow.router.AfterNavigationEvent; +import com.vaadin.flow.router.AfterNavigationObserver; +import com.vaadin.flow.router.Route; + +@Route("com.vaadin.flow.BackNavSecondView") +public class BackNavSecondView extends Div implements AfterNavigationObserver { + + public static final String CALLS = "calls"; + private int count = 0; + Span text = new Span("Second view: " + count); + + public BackNavSecondView() { + text.setId(CALLS); + add(text); + } + + @Override + public void afterNavigation(AfterNavigationEvent event) { + count++; + text.setText("Second view: " + count); + UI.getCurrent().getPage().getHistory().replaceState(null, + "com.vaadin.flow.BackNavSecondView?param"); + } +} diff --git a/flow-tests/test-react-router/src/main/java/com/vaadin/flow/ForwardTargetView.java b/flow-tests/test-react-router/src/main/java/com/vaadin/flow/ForwardTargetView.java index e8348d217ed..8eb9893e40c 100644 --- a/flow-tests/test-react-router/src/main/java/com/vaadin/flow/ForwardTargetView.java +++ b/flow-tests/test-react-router/src/main/java/com/vaadin/flow/ForwardTargetView.java @@ -1,3 +1,19 @@ +/* + * Copyright 2000-2024 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ + package com.vaadin.flow; import com.vaadin.flow.component.html.Div; diff --git a/flow-tests/test-react-router/src/main/java/com/vaadin/flow/ForwardTargetWithParametersView.java b/flow-tests/test-react-router/src/main/java/com/vaadin/flow/ForwardTargetWithParametersView.java index e60a1929806..1fac454f4ea 100644 --- a/flow-tests/test-react-router/src/main/java/com/vaadin/flow/ForwardTargetWithParametersView.java +++ b/flow-tests/test-react-router/src/main/java/com/vaadin/flow/ForwardTargetWithParametersView.java @@ -1,3 +1,19 @@ +/* + * Copyright 2000-2024 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ + package com.vaadin.flow; import com.vaadin.flow.component.html.Div; diff --git a/flow-tests/test-react-router/src/main/java/com/vaadin/flow/ForwardingToParametersView.java b/flow-tests/test-react-router/src/main/java/com/vaadin/flow/ForwardingToParametersView.java index 08dc78fdbff..f2e253a7935 100644 --- a/flow-tests/test-react-router/src/main/java/com/vaadin/flow/ForwardingToParametersView.java +++ b/flow-tests/test-react-router/src/main/java/com/vaadin/flow/ForwardingToParametersView.java @@ -1,3 +1,19 @@ +/* + * Copyright 2000-2024 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ + package com.vaadin.flow; import com.vaadin.flow.component.html.Div; diff --git a/flow-tests/test-react-router/src/main/java/com/vaadin/flow/ForwardingView.java b/flow-tests/test-react-router/src/main/java/com/vaadin/flow/ForwardingView.java index b0a00195097..c94c4d31872 100644 --- a/flow-tests/test-react-router/src/main/java/com/vaadin/flow/ForwardingView.java +++ b/flow-tests/test-react-router/src/main/java/com/vaadin/flow/ForwardingView.java @@ -1,3 +1,19 @@ +/* + * Copyright 2000-2024 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ + package com.vaadin.flow; import com.vaadin.flow.component.html.Div; diff --git a/flow-tests/test-react-router/src/test/java/com/vaadin/flow/BackNavIT.java b/flow-tests/test-react-router/src/test/java/com/vaadin/flow/BackNavIT.java new file mode 100644 index 00000000000..35717877ce1 --- /dev/null +++ b/flow-tests/test-react-router/src/test/java/com/vaadin/flow/BackNavIT.java @@ -0,0 +1,121 @@ +package com.vaadin.flow; + +import org.junit.Assert; +import org.junit.Test; +import org.openqa.selenium.TimeoutException; + +import com.vaadin.flow.component.html.testbench.NativeButtonElement; +import com.vaadin.flow.component.html.testbench.SpanElement; +import com.vaadin.flow.testutil.ChromeBrowserTest; + +public class BackNavIT extends ChromeBrowserTest { + + public static final String BACK_NAV_FIRST_VIEW = "/view/com.vaadin.flow.BackNavFirstView"; + public static final String BACK_NAV_SECOND_VIEW = "/view/com.vaadin.flow.BackNavSecondView?param"; + + // Test for https://github.com/vaadin/flow/issues/19839 + @Test + public void testBackButtonAfterHistoryStateChange() { + getDriver().get(getTestURL(getRootURL(), BACK_NAV_FIRST_VIEW, null)); + + try { + waitUntil(arg -> driver.getCurrentUrl() + .endsWith(BACK_NAV_FIRST_VIEW)); + } catch (TimeoutException e) { + Assert.fail("URL wasn't updated to expected one: " + + BACK_NAV_FIRST_VIEW); + } + + $(NativeButtonElement.class).first().click(); + + try { + waitUntil(arg -> driver.getCurrentUrl() + .endsWith(BACK_NAV_SECOND_VIEW)); + } catch (TimeoutException e) { + Assert.fail("URL wasn't updated to expected one: " + + BACK_NAV_SECOND_VIEW); + } + + // Navigate back; ensure we get the first URL again + getDriver().navigate().back(); + try { + waitUntil(arg -> driver.getCurrentUrl() + .endsWith(BACK_NAV_FIRST_VIEW)); + } catch (TimeoutException e) { + Assert.fail("URL wasn't updated to expected one: " + + BACK_NAV_FIRST_VIEW); + } + + Assert.assertTrue("Expected button is not available.", + $(NativeButtonElement.class).first().isDisplayed()); + } + + @Test + public void backButtonWorksAndContentUpdatesAfterPageRefresh() { + getDriver().get(getTestURL(getRootURL(), BACK_NAV_FIRST_VIEW, null)); + + try { + waitUntil(arg -> driver.getCurrentUrl() + .endsWith(BACK_NAV_FIRST_VIEW)); + } catch (TimeoutException e) { + Assert.fail("URL wasn't updated to expected one: " + + BACK_NAV_FIRST_VIEW); + } + + $(NativeButtonElement.class).first().click(); + + try { + waitUntil(arg -> driver.getCurrentUrl() + .endsWith(BACK_NAV_SECOND_VIEW)); + } catch (TimeoutException e) { + Assert.fail("URL wasn't updated to expected one: " + + BACK_NAV_SECOND_VIEW); + } + // Refresh page + getDriver().navigate().refresh(); + + waitUntil(driver -> $(SpanElement.class).id(BackNavSecondView.CALLS) + .isDisplayed()); + + // Navigate back; ensure we get the first URL again + getDriver().navigate().back(); + + try { + waitUntil(arg -> driver.getCurrentUrl() + .endsWith(BACK_NAV_FIRST_VIEW)); + } catch (TimeoutException e) { + Assert.fail("URL wasn't updated to expected one: " + + BACK_NAV_FIRST_VIEW); + } + + Assert.assertTrue("Expected button is not available.", + $(NativeButtonElement.class).first().isDisplayed()); + } + + @Test + public void validateNoAfterNavigationForReplaceState() { + getDriver().get(getTestURL(getRootURL(), BACK_NAV_FIRST_VIEW, null)); + + try { + waitUntil(arg -> driver.getCurrentUrl() + .endsWith(BACK_NAV_FIRST_VIEW)); + } catch (TimeoutException e) { + Assert.fail("URL wasn't updated to expected one: " + + BACK_NAV_FIRST_VIEW); + } + + $(NativeButtonElement.class).first().click(); + + try { + waitUntil(arg -> driver.getCurrentUrl() + .endsWith(BACK_NAV_SECOND_VIEW)); + } catch (TimeoutException e) { + Assert.fail("URL wasn't updated to expected one: " + + BACK_NAV_SECOND_VIEW); + } + + Assert.assertEquals("Second view: 1", + $(SpanElement.class).id(BackNavSecondView.CALLS).getText()); + } + +}