fix: update url after route change (#1459)

For #1437.

---------

Co-authored-by: Daishi Kato <[email protected]>

Author: rmarscher

e2e/fixtures/broken-links/src/redirects.ts

@@ -1,22 +1,25 @@
 import type { Middleware } from 'waku/config';
 
+// BASE_RSC_PATH + encodeRscPath(rscPath)
+const redirects = {
+  '/redirect': '/exists',
+  '/RSC/R/redirect.txt': '/RSC/R/exists.txt',
+  '/broken-redirect': '/broken',
+  '/RSC/R/broken-redirect.txt': '/RSC/R/broken.txt',
+};
+
 const redirectsMiddleware: Middleware = () => async (ctx, next) => {
-  switch (ctx.req.url.pathname) {
-    case '/redirect':
-      ctx.res.status = 302;
-      ctx.res.headers = {
-        Location: '/exists',
-      };
-      break;
-    case '/broken-redirect':
-      ctx.res.status = 302;
-      ctx.res.headers = {
-        Location: '/broken',
-      };
-      break;
-    default:
-      return await next();
+  if (ctx.req.url.pathname in redirects) {
+    const pathname = ctx.req.url.pathname as keyof typeof redirects;
+    const url = new URL(ctx.req.url);
+    url.pathname = redirects[pathname];
+    ctx.res.status = 302;
+    ctx.res.headers = {
+      Location: url.toString(),
+    };
+    return;
   }
+  return await next();
 };
 
 export default redirectsMiddleware;

packages/waku/src/router/client.ts

@@ -151,18 +151,21 @@ export function useRouter() {
       },
     ) => {
       const url = new URL(to, window.location.href);
-      const newPath = url.pathname !== window.location.pathname;
-      window.history.pushState(
-        {
-          ...window.history.state,
-          waku_new_path: newPath,
-        },
-        '',
-        url,
-      );
+      const currentPath = window.location.pathname;
+      const newPath = url.pathname !== currentPath;
       await changeRoute(parseRoute(url), {
         shouldScroll: options?.scroll ?? newPath,
       });
+      if (window.location.pathname === currentPath) {
+        window.history.pushState(
+          {
+            ...window.history.state,
+            waku_new_path: newPath,
+          },
+          '',
+          url,
+        );
+      }
     },
     [changeRoute],
   );
@@ -180,11 +183,14 @@ export function useRouter() {
       },
     ) => {
       const url = new URL(to, window.location.href);
-      const newPath = url.pathname !== window.location.pathname;
-      window.history.replaceState(window.history.state, '', url);
+      const currentPath = window.location.pathname;
+      const newPath = url.pathname !== currentPath;
       await changeRoute(parseRoute(url), {
         shouldScroll: options?.scroll ?? newPath,
       });
+      if (window.location.pathname === currentPath) {
+        window.history.replaceState(window.history.state, '', url);
+      }
     },
     [changeRoute],
   );
@@ -331,19 +337,26 @@ export function Link({
       const route = parseRoute(url);
       prefetchRoute(route);
       startTransitionFn(async () => {
-        const newPath = url.pathname !== window.location.pathname;
-        window.history.pushState(
-          {
-            ...window.history.state,
-            waku_new_path: newPath,
-          },
-          '',
-          url,
-        );
-        await changeRoute(route, {
-          shouldScroll: scroll ?? newPath,
-          unstable_startTransition: startTransitionFn,
-        });
+        const currentPath = window.location.pathname;
+        const newPath = url.pathname !== currentPath;
+        try {
+          await changeRoute(route, {
+            shouldScroll: scroll ?? newPath,
+            unstable_startTransition: startTransitionFn,
+          });
+        } finally {
+          if (window.location.pathname === currentPath) {
+            // Update history if it wasn't already updated
+            window.history.pushState(
+              {
+                ...window.history.state,
+                waku_new_path: newPath,
+              },
+              '',
+              url,
+            );
+          }
+        }
       });
     }
   };
@@ -474,15 +487,8 @@ const Redirect = ({
       window.location.replace(to);
       return;
     }
-    const newPath = url.pathname !== window.location.pathname;
-    window.history.pushState(
-      {
-        ...window.history.state,
-        waku_new_path: newPath,
-      },
-      '',
-      url,
-    );
+    const currentPath = window.location.pathname;
+    const newPath = url.pathname !== currentPath;
     changeRoute(parseRoute(url), { shouldScroll: newPath })
       .then(() => {
         // FIXME: As we understand it, we should have a proper solution.
@@ -492,6 +498,18 @@ const Redirect = ({
       })
       .catch((err) => {
         console.log('Error while navigating to redirect:', err);
+      })
+      .finally(() => {
+        if (window.location.pathname === currentPath) {
+          window.history.replaceState(
+            {
+              ...window.history.state,
+              waku_new_path: newPath,
+            },
+            '',
+            url,
+          );
+        }
       });
   }, [error, to, reset, changeRoute]);
   return null;
@@ -613,6 +631,7 @@ const handleScroll = () => {
 const InnerRouter = ({ initialRoute }: { initialRoute: RouteProps }) => {
   const elementsPromise = useElementsPromise();
   const [has404, setHas404] = useState(false);
+  const requestedRouteRef = useRef<RouteProps>(initialRoute);
   const staticPathSetRef = useRef(new Set<string>());
   const cachedIdSetRef = useRef(new Set<string>());
   useEffect(() => {
@@ -679,11 +698,9 @@ const InnerRouter = ({ initialRoute }: { initialRoute: RouteProps }) => {
                 elements;
               if (routeData) {
                 const [path, query] = routeData as [string, string];
-                // FIXME this check here seems ad-hoc (less readable code)
                 if (
-                  window.location.pathname !== path ||
-                  (!isStatic &&
-                    window.location.search.replace(/^\?/, '') !== query)
+                  requestedRouteRef.current.path !== path ||
+                  (!isStatic && requestedRouteRef.current.query !== query)
                 ) {
                   locationListeners.forEach((listener) =>
                     listener(path, query),
@@ -767,6 +784,7 @@ const InnerRouter = ({ initialRoute }: { initialRoute: RouteProps }) => {
   const refetching = useRef<[onFinish?: () => void] | null>(null);
   const changeRoute: ChangeRoute = useCallback(
     async (route, options) => {
+      requestedRouteRef.current = route;
       executeListeners('start', route);
       const startTransitionFn =
         options.unstable_startTransition || ((fn: TransitionFunction) => fn());
@@ -827,22 +845,25 @@ const InnerRouter = ({ initialRoute }: { initialRoute: RouteProps }) => {
         url.pathname = path;
         url.search = query;
         url.hash = '';
-        if (path !== '/404') {
-          window.history.pushState(
-            {
-              ...window.history.state,
-              waku_new_path: url.pathname !== window.location.pathname,
-            },
-            '',
-            url,
-          );
-        }
         changeRoute(parseRoute(url), {
           skipRefetch: true,
           shouldScroll: false,
-        }).catch((err) => {
-          console.log('Error while navigating to new route:', err);
-        });
+        })
+          .catch((err) => {
+            console.log('Error while handling location listeners:', err);
+          })
+          .finally(() => {
+            if (path !== '/404') {
+              window.history.pushState(
+                {
+                  ...window.history.state,
+                  waku_new_path: url.pathname !== window.location.pathname,
+                },
+                '',
+                url,
+              );
+            }
+          });
       };
       if (refetching.current) {
         refetching.current.push(fn);