fix(pages-router): preserve dynamic href history state

Author: james-elicx

packages/vinext/src/shims/router.ts

@@ -600,11 +600,13 @@ function normalizeHydrationNavigationUrl(url: string): string {
 
 class HrefInterpolationError extends Error {}
 
-function interpolateCurrentDynamicRoute(resolved: string): string {
-  if (typeof window === "undefined") return resolved;
+type CurrentDynamicRoute = { href: string; as: string };
+
+function resolveCurrentDynamicRoute(resolved: string): CurrentDynamicRoute | null {
+  if (typeof window === "undefined") return null;
 
   const routePattern = window.__NEXT_DATA__?.page;
-  if (!routePattern || extractRouteParamNames(routePattern).length === 0) return resolved;
+  if (!routePattern || extractRouteParamNames(routePattern).length === 0) return null;
 
   try {
     const target = new URL(resolved, "http://vinext.local");
@@ -614,14 +616,14 @@ function interpolateCurrentDynamicRoute(resolved: string): string {
       target.origin !== "http://vinext.local" &&
       target.origin !== currentOrigin
     ) {
-      return resolved;
+      return null;
     }
     const visiblePath = stripBasePath(window.location.pathname, __basePath);
     const visibleLocale = getLocalePathPrefix(visiblePath, window.__VINEXT_LOCALES__);
     const routePath = visibleLocale
       ? visiblePath.slice(visibleLocale.length + 1) || "/"
       : visiblePath;
-    if (extractRouteParamsFromPath(routePattern, routePath) === null) return resolved;
+    if (extractRouteParamsFromPath(routePattern, routePath) === null) return null;
 
     const query = parseQueryString(target.search);
     const missingParams = routePatternParts(routePattern)
@@ -641,7 +643,7 @@ function interpolateCurrentDynamicRoute(resolved: string): string {
     }
 
     const routeParams = getRouteParamsFromQuery(routePattern, query);
-    if (!routeParams) return resolved;
+    if (!routeParams) return null;
 
     const encodedRouteParams = Object.fromEntries(
       Object.entries(routeParams).map(([key, value]) => [
@@ -650,19 +652,21 @@ function interpolateCurrentDynamicRoute(resolved: string): string {
       ]),
     );
     const pathname = fillRoutePatternSegments(routePattern, encodedRouteParams);
-    if (!pathname) return resolved;
+    if (!pathname) return null;
 
     const targetLocale = getLocalePathPrefix(target.pathname, window.__VINEXT_LOCALES__);
+    const hrefPathname = targetLocale ? `/${targetLocale}${routePattern}` : routePattern;
+    const href = hrefPathname + target.search + target.hash;
     target.pathname = targetLocale ? `/${targetLocale}${pathname}` : pathname;
     for (const paramName of extractRouteParamNames(routePattern)) {
       target.searchParams.delete(paramName);
     }
-    return target.href.slice(target.origin.length);
+    return { href, as: target.href.slice(target.origin.length) };
   } catch (error) {
     if (error instanceof HrefInterpolationError) {
       throw error;
     }
-    return resolved;
+    return null;
   }
 }
 
@@ -3235,9 +3239,19 @@ async function performNavigation(
           typeof url.query === "object" &&
           Object.keys(url.query).length > 0) ||
           (typeof url.search === "string" && url.search.length > 0))));
+  let inheritedRouteHref: string | undefined;
   if (inheritsCurrentPath) {
-    resolved = interpolateCurrentDynamicRoute(resolved);
-    resolvedRoute = interpolateCurrentDynamicRoute(resolvedRoute);
+    const routeMatchesTarget = resolvedRoute === resolved;
+    const resolvedDynamicRoute = resolveCurrentDynamicRoute(resolved);
+    if (resolvedDynamicRoute) resolved = resolvedDynamicRoute.as;
+
+    const internalDynamicRoute = routeMatchesTarget
+      ? resolvedDynamicRoute
+      : resolveCurrentDynamicRoute(resolvedRoute);
+    if (internalDynamicRoute) {
+      resolvedRoute = internalDynamicRoute.as;
+      inheritedRouteHref = internalDynamicRoute.href;
+    }
   }
 
   // External URLs — delegate to browser (unless same-origin)
@@ -3435,7 +3449,11 @@ async function performNavigation(
   const navStateOptions: { locale?: string; shallow: boolean } = { shallow };
   if (navigationLocale !== undefined) navStateOptions.locale = navigationLocale;
   const resolvedNoHash = stripHash(resolved);
-  const resolvedRouteNoHash = stripHash(interpolatedRoute);
+  const resolvedRouteNoHash = stripHash(
+    inheritedRouteHref
+      ? normalizePathTrailingSlash(inheritedRouteHref, __trailingSlash)
+      : interpolatedRoute,
+  );
   const navState = {
     url: resolvedRouteNoHash,
     as: resolvedNoHash,
@@ -3991,7 +4009,9 @@ function handlePagesRouterPopState(e: PopStateEvent): void {
         typeof state.as === "string" &&
         state.url !== state.as
       ) {
-        return normalizePathTrailingSlash(withBasePath(state.url, __basePath), __trailingSlash);
+        const dynamicRoute = interpolateDynamicRouteHref(state.url, state.as);
+        const routeUrl = dynamicRoute?.href || state.url;
+        return normalizePathTrailingSlash(withBasePath(routeUrl, __basePath), __trailingSlash);
       }
       return browserUrl;
     })();

tests/e2e/pages-router/link-advanced.spec.ts

@@ -446,30 +446,31 @@ test.describe("Link href/as bracket-pattern interpolation (Pages Router)", () =>
     expect(sawConcrete).toBe(true);
   });
 
-  // popstate (back/forward) reads `state.url` (set on push) as the route URL
-  // to fetch. If push stamped the bracket pattern into history state, forward
-  // traversal would re-issue the unservable URL even though the original push
-  // worked. Asserts state.url was interpolated, not the raw pattern.
-  test("forward popstate after dynamic Router.push fetches concrete URL", async ({ page }) => {
-    await page.goto(`${BASE}/link-test`);
+  // Query-only navigation preserves Next.js' bracket-pattern `state.url`, while
+  // popstate still has to fetch and render the concrete route on traversal.
+  test("forward popstate resolves query-only dynamic route history", async ({ page }) => {
+    await page.goto(`${BASE}/posts/1`);
     await page.waitForFunction(() => (window as any).__VINEXT_ROOT__);
 
-    // Push the dynamic destination, then go back to /link-test.
+    // Push the query-only destination, then traverse away from it.
     await page.evaluate(() => {
       const router = (window as any).next?.router;
-      return router?.push("/posts/[id]", "/posts/11");
+      return router?.push({ query: { id: "2" } });
     });
-    await expect(page.locator('[data-testid="post-title"]')).toHaveText("Post: 11");
+    await expect(page.locator('[data-testid="post-title"]')).toHaveText("Post: 2");
+    await expect
+      .poll(() => page.evaluate(() => ({ url: history.state?.url, as: history.state?.as })))
+      .toEqual({ url: "/posts/[id]?id=2", as: "/posts/2" });
     await page.goBack();
-    await expect(page).toHaveURL(`${BASE}/link-test`);
+    await expect(page.locator('[data-testid="post-title"]')).toHaveText("Post: 1");
 
     // Start capturing AFTER the back, so requests are scoped to the forward.
     const requests: string[] = [];
     page.on("request", (req) => requests.push(req.url()));
     await page.goForward();
 
-    await expect(page.locator('[data-testid="post-title"]')).toHaveText("Post: 11");
-    expect(page.url()).toBe(`${BASE}/posts/11`);
+    await expect(page.locator('[data-testid="post-title"]')).toHaveText("Post: 2");
+    expect(page.url()).toBe(`${BASE}/posts/2`);
 
     const routingRequests = requests.filter(
       (u) => /\/_next\/data\//.test(u) || /\/posts\/(?!.*\.tsx)/.test(u),

tests/shims.test.ts

@@ -17939,14 +17939,23 @@ describe("Pages Router concurrent navigation", () => {
   async function expectPagesRouterPushTrailingSlashNormalization({
     target,
     expectedBrowserUrl,
+    dynamicRoute,
+    expectedStateUrl,
   }: {
-    target: string;
+    target: string | { query: Record<string, string> };
     expectedBrowserUrl: string;
+    dynamicRoute?: { page: string; pathname: string };
+    expectedStateUrl?: string;
   }): Promise<void> {
     const previousWindow = (globalThis as any).window;
     const previousTrailingSlash = process.env.__VINEXT_TRAILING_SLASH;
     const originalFetch = globalThis.fetch;
     const { win } = createNavWindow();
+    if (dynamicRoute) {
+      win.location.pathname = dynamicRoute.pathname;
+      win.location.href = `http://localhost${dynamicRoute.pathname}`;
+      win.__NEXT_DATA__.page = dynamicRoute.page;
+    }
     (globalThis as any).window = win;
     process.env.__VINEXT_TRAILING_SLASH = "true";
     vi.resetModules();
@@ -17965,7 +17974,10 @@ describe("Pages Router concurrent navigation", () => {
       // History state now follows Next.js shape ({ url, as, options, __N, key });
       // assert via partial match so test stays focused on URL normalization.
       expect(win.history.pushState).toHaveBeenCalledWith(
-        expect.objectContaining({ __N: true }),
+        expect.objectContaining({
+          __N: true,
+          ...(expectedStateUrl ? { url: expectedStateUrl } : {}),
+        }),
         "",
         expectedBrowserUrl,
       );
@@ -17999,6 +18011,15 @@ describe("Pages Router concurrent navigation", () => {
     });
   });
 
+  it("normalizes query-only dynamic route history when trailingSlash is true", async () => {
+    await expectPagesRouterPushTrailingSlashNormalization({
+      target: { query: { id: "2" } },
+      dynamicRoute: { page: "/posts/[id]", pathname: "/posts/1" },
+      expectedBrowserUrl: "/posts/2/",
+      expectedStateUrl: "/posts/[id]/?id=2",
+    });
+  });
+
   it("Pages Router push strips file-looking path slashes when trailingSlash is true", async () => {
     await expectPagesRouterPushTrailingSlashNormalization({
       target: "/catch-all/hello.world/",