fix: do not use popstate event (#19505)

When pushing or replacing history send a custom event 'vaadin-navigate' instead of popstate event. Fixes #19513
vaadin · Jun 5, 2024 · 0453da2 · 0453da2
1 parent e6dce6d
commit 0453da2
Show file tree

Hide file tree

Showing 12 changed files with 228 additions and 122 deletions.
diff --git a/flow-server/src/main/java/com/vaadin/flow/component/UI.java b/flow-server/src/main/java/com/vaadin/flow/component/UI.java
@@ -1865,8 +1865,14 @@ public void browserNavigate(BrowserNavigateEvent event) {
      */
     private void replaceStateIfDiffersAndNoReplacePending(String route,
             Location location) {
-        if (!location.getPath().equals(route) && !getInternals()
-                .containsPendingJavascript("window.history.replaceState")) {
+        boolean locationChanged =
+                !location.getPath().equals(route) && route.startsWith("/") &&
+                        !location.getPath().equals(route.substring(1));
+        boolean containsPendingReplace = !getInternals().containsPendingJavascript(
+                "window.history.replaceState") &&
+                !getInternals().containsPendingJavascript(
+                        "'vaadin-navigate', { detail: { state: $0, url: $1, replace: true } }");
+        if (locationChanged && containsPendingReplace) {
             // See InternalRedirectHandler invoked via Router.
             getPage().getHistory().replaceState(null, location);
         }

diff --git a/flow-server/src/main/java/com/vaadin/flow/component/page/History.java b/flow-server/src/main/java/com/vaadin/flow/component/page/History.java
@@ -186,9 +186,13 @@ public void pushState(JsonValue state, Location location) {
                 location);
         // Second parameter is title which is currently ignored according to
         // https://developer.mozilla.org/en-US/docs/Web/API/History_API
-        ui.getPage().executeJs(
-                "setTimeout(() => { window.history.pushState($0, '', $1); window.dispatchEvent(new PopStateEvent('popstate', {state: 'vaadin-router-ignore'})); })",
-                state, pathWithQueryParameters);
+        if(ui.getSession().getConfiguration().isReactEnabled()) {
+            ui.getPage().executeJs("window.dispatchEvent(new CustomEvent('vaadin-navigate', { detail: { state: $0, url: $1, replace: false } }));",
+                    state, pathWithQueryParameters);
+        } else {
+            ui.getPage().executeJs("setTimeout(() => { window.history.pushState($0, '', $1); window.dispatchEvent(new CustomEvent('vaadin-navigate')); })",
+                    state, pathWithQueryParameters);
+        }
     }
 
     /**
@@ -225,9 +229,13 @@ public void replaceState(JsonValue state, Location location) {
                 location);
         // Second parameter is title which is currently ignored according to
         // https://developer.mozilla.org/en-US/docs/Web/API/History_API
-        ui.getPage().executeJs(
-                "setTimeout(() => { window.history.replaceState($0, '', $1); window.dispatchEvent(new PopStateEvent('popstate', {state: 'vaadin-router-ignore'})); })",
-                state, pathWithQueryParameters);
+        if(ui.getSession().getConfiguration().isReactEnabled()) {
+            ui.getPage().executeJs("window.dispatchEvent(new CustomEvent('vaadin-navigate', { detail: { state: $0, url: $1, replace: true } }));",
+                    state, pathWithQueryParameters);
+        } else {
+            ui.getPage().executeJs("setTimeout(() => { window.history.replaceState($0, '', $1); window.dispatchEvent(new CustomEvent('vaadin-navigate')); })",
+                    state, pathWithQueryParameters);
+        }
     }
 
     /**

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
@@ -147,18 +147,10 @@ type RouterContainer = Awaited<ReturnType<typeof flow.serverSideRoutes[0]["actio
 
 function Flow() {
     const ref = useRef<HTMLOutputElement>(null);
-    const prevHistoryState = useRef<any>(null);
     const navigate = useNavigate();
-    const blocker = useBlocker(({nextLocation, historyAction}) => {
-        const reactRouterHistory = !!prevHistoryState.current && typeof prevHistoryState.current === "object" && "idx" in prevHistoryState.current;
-        const reactRouterNavigation = !!window.history.state && typeof window.history.state === "object" && "idx" in window.history.state;
-        prevHistoryState.current = window.history.state;
-        // @ts-ignore
-        if(event && event.state && event.state === "vaadin-router-ignore") {
-            prevHistoryState.current = {"idx":0};
-            return historyAction === "POP";
-        }
-        return !(historyAction === "POP" && reactRouterHistory && reactRouterNavigation);
+    const blocker = useBlocker(({ currentLocation, nextLocation }) => {
+        navigated.current = nextLocation.pathname === currentLocation.pathname && nextLocation.search === currentLocation.search && nextLocation.hash === currentLocation.hash;
+        return true;
     });
     const {pathname, search, hash} = useLocation();
     const navigated = useRef<boolean>(false);
@@ -175,6 +167,7 @@ function Flow() {
             event.preventDefault();
         }
 
+        navigated.current = false;
         navigate(path);
     }, [navigate]);
 
@@ -189,6 +182,12 @@ function Flow() {
         navigate(path);
     }, [navigate]);
 
+    const vaadinNavigateEventHandler = useCallback((event: CustomEvent<{state: unknown, url: string, replace?: boolean}>) => {
+        const path = '/' + event.detail.url;
+        navigated.current = true;
+        navigate(path, { state: event.detail.state, replace: event.detail.replace});
+    }, [navigate]);
+
     const redirect = useCallback((path: string) => {
         return (() => {
             navigate(path, {replace: true});
@@ -198,12 +197,16 @@ function Flow() {
     useEffect(() => {
         // @ts-ignore
         window.addEventListener('vaadin-router-go', vaadinRouterGoEventHandler);
+        // @ts-ignore
+        window.addEventListener('vaadin-navigate', vaadinNavigateEventHandler);
 
         return () => {
             // @ts-ignore
             window.removeEventListener('vaadin-router-go', vaadinRouterGoEventHandler);
+            // @ts-ignore
+            window.removeEventListener('vaadin-navigate', vaadinNavigateEventHandler);
         };
-    }, [vaadinRouterGoEventHandler]);
+    }, [vaadinRouterGoEventHandler, vaadinNavigateEventHandler]);
 
     useEffect(() => {
         return () => {
@@ -214,6 +217,10 @@ function Flow() {
 
     useEffect(() => {
         if (blocker.state === 'blocked') {
+            if(navigated.current) {
+                blocker.proceed();
+                return;
+            }
             const {pathname, search} = blocker.location;
             let matched = matchRoutes(Array.from(routes), window.location.pathname);
 
@@ -222,7 +229,10 @@ function Flow() {
             if (matched && matched.filter(path => path.route?.element?.type?.name === Flow.name).length != 0) {
                 containerRef.current?.onBeforeEnter?.call(containerRef?.current,
                     {pathname,search}, {
-                        prevent,
+                        prevent() {
+                            blocker.reset();
+                            navigated.current = false;
+                        },
                         redirect,
                         continue() {
                             blocker.proceed();

diff --git a/flow-server/src/main/resources/com/vaadin/flow/server/frontend/routes-flow.tsx b/flow-server/src/main/resources/com/vaadin/flow/server/frontend/routes-flow.tsx
@@ -12,7 +12,7 @@ import { serverSideRoutes } from 'Frontend/generated/flow/Flow';
 function build() {
     const routes = [...serverSideRoutes] as RouteObject[];
     return {
-        router: createBrowserRouter(routes),
+        router: createBrowserRouter([...routes], { basename: new URL(document.baseURI).pathname }),
         routes
     };
 }

diff --git a/flow-server/src/test/java/com/vaadin/flow/component/internal/JavaScriptBootstrapUITest.java b/flow-server/src/test/java/com/vaadin/flow/component/internal/JavaScriptBootstrapUITest.java
@@ -49,7 +49,8 @@
 
 public class JavaScriptBootstrapUITest {
 
-    private static final String CLIENT_PUSHSTATE_TO = "setTimeout(() => { window.history.pushState($0, '', $1); window.dispatchEvent(new PopStateEvent('popstate', {state: 'vaadin-router-ignore'})); })";
+    private static final String CLIENT_PUSHSTATE_TO = "setTimeout(() => { window.history.pushState($0, '', $1); window.dispatchEvent(new CustomEvent('vaadin-navigate')); })";
+    private static final String REACT_PUSHSTATE_TO = "window.dispatchEvent(new CustomEvent('vaadin-navigate', { detail: { state: $0, url: $1, replace: false } }));";
 
     private MockServletServiceSessionSetup mocks;
     private UI ui;
@@ -411,7 +412,7 @@ public void should_update_pushState_when_navigationHasBeenAlreadyStarted() {
 
         UIInternals internals = mockUIInternals();
 
-        VaadinSession session = Mockito.mock(VaadinSession.class);
+        VaadinSession session = mocks.getSession();
         DeploymentConfiguration configuration = Mockito
                 .mock(DeploymentConfiguration.class);
         Mockito.when(internals.getSession()).thenReturn(session);
@@ -437,12 +438,20 @@ public void should_update_pushState_when_navigationHasBeenAlreadyStarted() {
         ui.navigate("clean/1");
         Mockito.verify(page).executeJs(execJs.capture(), execArg.capture());
 
-        assertEquals(CLIENT_PUSHSTATE_TO, execJs.getValue());
+        boolean reactEnabled = ui.getSession().getConfiguration()
+                .isReactEnabled();
 
         final Serializable[] execValues = execArg.getValue();
-        assertEquals(2, execValues.length);
-        assertNull(execValues[0]);
-        assertEquals("clean/1", execValues[1]);
+        if (reactEnabled) {
+            assertEquals(REACT_PUSHSTATE_TO, execJs.getValue());
+            assertEquals(1, execValues.length);
+            assertEquals("clean/1", execValues[0]);
+        } else {
+            assertEquals(CLIENT_PUSHSTATE_TO, execJs.getValue());
+            assertEquals(2, execValues.length);
+            assertNull(execValues[0]);
+            assertEquals("clean/1", execValues[1]);
+        }
     }
 
     @Test
@@ -483,9 +492,15 @@ public void server_should_not_doClientRoute_when_navigatingToServer() {
         assertEquals(Tag.SPAN, ui.wrapperElement.getChild(0).getTag());
         Mockito.verify(page).executeJs(execJs.capture(), execArg.capture());
 
-        assertEquals(CLIENT_PUSHSTATE_TO, execJs.getValue());
+        boolean reactEnabled = ui.getSession().getConfiguration()
+                .isReactEnabled();
 
         final Serializable[] execValues = execArg.getValue();
+        if (reactEnabled) {
+            assertEquals(REACT_PUSHSTATE_TO, execJs.getValue());
+        } else {
+            assertEquals(CLIENT_PUSHSTATE_TO, execJs.getValue());
+        }
         assertEquals(2, execValues.length);
         assertNull(execValues[0]);
         assertEquals("dirty", execValues[1]);

diff --git a/flow-server/src/test/java/com/vaadin/flow/component/page/HistoryTest.java b/flow-server/src/test/java/com/vaadin/flow/component/page/HistoryTest.java
@@ -21,8 +21,11 @@
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
+import org.mockito.Mockito;
 
 import com.vaadin.flow.component.UI;
+import com.vaadin.flow.function.DeploymentConfiguration;
+import com.vaadin.flow.server.VaadinSession;
 
 import elemental.json.Json;
 import elemental.json.JsonString;
@@ -34,6 +37,11 @@ private class TestUI extends UI {
         public Page getPage() {
             return page;
         }
+
+        @Override
+        public VaadinSession getSession() {
+            return session;
+        }
     }
 
     private class TestPage extends Page {
@@ -55,16 +63,25 @@ public PendingJavaScriptResult executeJs(String expression,
         }
     }
 
-    private UI ui = new TestUI();
+    private TestUI ui = new TestUI();
     private TestPage page = new TestPage(ui);
     private History history;
 
-    private static final String PUSH_STATE_JS = "setTimeout(() => { window.history.pushState($0, '', $1); window.dispatchEvent(new PopStateEvent('popstate', {state: 'vaadin-router-ignore'})); })";
-    private static final String REPLACE_STATE_JS = "setTimeout(() => { window.history.replaceState($0, '', $1); window.dispatchEvent(new PopStateEvent('popstate', {state: 'vaadin-router-ignore'})); })";
+    private VaadinSession session = Mockito.mock(VaadinSession.class);
+    private DeploymentConfiguration configuration;
+
+    private static final String PUSH_STATE_JS = "setTimeout(() => { window.history.pushState($0, '', $1); window.dispatchEvent(new CustomEvent('vaadin-navigate')); })";
+    private static final String REPLACE_STATE_JS = "setTimeout(() => { window.history.replaceState($0, '', $1); window.dispatchEvent(new CustomEvent('vaadin-navigate')); })";
+
+    private static final String PUSH_STATE_REACT = "window.dispatchEvent(new CustomEvent('vaadin-navigate', { detail: { state: $0, url: $1, replace: false } }));";
+    private static final String REPLACE_STATE_REACT = "window.dispatchEvent(new CustomEvent('vaadin-navigate', { detail: { state: $0, url: $1, replace: true } }));";
 
     @Before
     public void setup() {
         history = new History(ui);
+        configuration = Mockito.mock(DeploymentConfiguration.class);
+        Mockito.when(session.getConfiguration()).thenReturn(configuration);
+        Mockito.when(configuration.isReactEnabled()).thenReturn(false);
     }
 
     @Test
@@ -154,4 +171,96 @@ public void replaceState_locationEmpty_pushesPeriod() {
         Assert.assertEquals("location should be '.'", ".", page.parameters[1]);
     }
 
+
+    @Test
+    public void pushState_locationWithQueryParameters_queryParametersRetained_react() {
+        Mockito.when(configuration.isReactEnabled()).thenReturn(true);
+        history.pushState(Json.create("{foo:bar;}"), "context/view?param=4");
+
+        Assert.assertEquals("push state JS not included", PUSH_STATE_REACT,
+                page.expression);
+        Assert.assertEquals("push state not included", "{foo:bar;}",
+                ((JsonString) page.parameters[0]).getString());
+        Assert.assertEquals("invalid location", "context/view?param=4",
+                page.parameters[1]);
+
+        history.pushState(Json.create("{foo:bar;}"), "context/view/?param=4");
+
+        Assert.assertEquals("push state JS not included", PUSH_STATE_REACT,
+                page.expression);
+        Assert.assertEquals("push state not included", "{foo:bar;}",
+                ((JsonString) page.parameters[0]).getString());
+        Assert.assertEquals("invalid location", "context/view/?param=4",
+                page.parameters[1]);
+    }
+
+    @Test
+    public void pushState_locationWithFragment_fragmentRetained_react() {
+        Mockito.when(configuration.isReactEnabled()).thenReturn(true);
+        history.pushState(null, "context/view#foobar");
+
+        Assert.assertEquals("push state JS not included", PUSH_STATE_REACT,
+                page.expression);
+        Assert.assertEquals(null, page.parameters[0]);
+        Assert.assertEquals("fragment not retained", "context/view#foobar",
+                page.parameters[1]);
+
+        history.pushState(null, "context/view/#foobar");
+
+        Assert.assertEquals("push state JS not included", PUSH_STATE_REACT,
+                page.expression);
+        Assert.assertEquals(null, page.parameters[0]);
+        Assert.assertEquals("fragment not retained", "context/view/#foobar",
+                page.parameters[1]);
+    }
+
+    @Test // #11628
+    public void pushState_locationWithQueryParametersAndFragment_QueryParametersAndFragmentRetained_react() {
+        Mockito.when(configuration.isReactEnabled()).thenReturn(true);
+        history.pushState(null, "context/view?foo=bar#foobar");
+
+        Assert.assertEquals("push state JS not included", PUSH_STATE_REACT,
+                page.expression);
+        Assert.assertEquals(null, page.parameters[0]);
+        Assert.assertEquals("invalid location", "context/view?foo=bar#foobar",
+                page.parameters[1]);
+
+        history.pushState(null, "context/view/?foo=bar#foobar");
+
+        Assert.assertEquals("push state JS not included", PUSH_STATE_REACT,
+                page.expression);
+        Assert.assertEquals(null, page.parameters[0]);
+        Assert.assertEquals("invalid location", "context/view/?foo=bar#foobar",
+                page.parameters[1]);
+    }
+
+    @Test // #11628
+    public void replaceState_locationWithQueryParametersAndFragment_QueryParametersAndFragmentRetained_react() {
+        Mockito.when(configuration.isReactEnabled()).thenReturn(true);
+        history.replaceState(null, "context/view?foo=bar#foobar");
+
+        Assert.assertEquals("replace state JS not included",
+                REPLACE_STATE_REACT, page.expression);
+        Assert.assertEquals(null, page.parameters[0]);
+        Assert.assertEquals("invalid location", "context/view?foo=bar#foobar",
+                page.parameters[1]);
+
+        history.replaceState(null, "context/view/?foo=bar#foobar");
+
+        Assert.assertEquals("replace state JS not included",
+                REPLACE_STATE_REACT, page.expression);
+        Assert.assertEquals(null, page.parameters[0]);
+        Assert.assertEquals("invalid location", "context/view/?foo=bar#foobar",
+                page.parameters[1]);
+    }
+
+    @Test // #11628
+    public void replaceState_locationEmpty_pushesPeriod_react() {
+        Mockito.when(configuration.isReactEnabled()).thenReturn(true);
+        history.replaceState(null, "");
+        Assert.assertEquals("replace state JS not included",
+                REPLACE_STATE_REACT, page.expression);
+        Assert.assertEquals(null, page.parameters[0]);
+        Assert.assertEquals("location should be '.'", ".", page.parameters[1]);
+    }
 }