fix: honor the as parameter in router.push/replace (#453)

* fix: stabilize instrumentation state and hydration E2E tests

Move Cloudflare instrumentation example state from module-level
variables to globalThis so startup and request paths share one owner
across different module instances.

Add waitForHydration() helper to Pages Router E2E tests to eliminate
flaky button-click assertions that raced against React hydration.

* fix: honor the `as` parameter in router.push/replace

The `as` parameter in push() and replace() was silently ignored
(prefixed as _as) in both the useRouter() hook and the Router
singleton. This broke masked URLs and legacy dynamic-route flows.

Extract resolveNavigationTarget() helper that uses `as` when provided,
falling back to resolveUrl(url) otherwise. Apply to all four call
sites: useRouter().push, useRouter().replace, Router.push, Router.replace.

* fix: address PR review feedback

- Add doc comment to resolveNavigationTarget explaining the url/as
  simplification vs Next.js's two-value approach
- Extract waitForHydration to shared tests/e2e/helpers.ts
- Add pathname assertion to verify router.pathname stays as route
  pattern (/posts/[id]) after as-based navigation

* fix: remove pathname assertion that tests unimplemented behavior

router.pathname currently returns the resolved path after client-side
navigation, not the route pattern. The correct behavior (returning
/posts/[id] instead of /posts/42) requires the two-value url/as
approach tracked in #462.

Author: NathanDrake2406

examples/app-router-cloudflare/instrumentation-state.ts

@@ -1,17 +1,9 @@
 /**
  * Shared state for instrumentation.ts testing in app-router-cloudflare.
  *
- * ## Why plain module-level variables work here
- *
- * register() is now emitted as a top-level `await` inside the generated RSC
- * entry module (see `generateRscEntry` in `entries/app-rsc-entry.ts`). This means it
- * runs inside the Cloudflare Worker process — the same process and module graph
- * as the API routes. Plain module-level variables are therefore visible to both
- * the instrumentation code and the API route that reads them.
- *
- * This is different from the old approach (writing to a temp file on disk) which
- * was needed when register() ran in the host Node.js process and API routes ran
- * in the miniflare Worker subprocess (two separate processes, no shared memory).
+ * Cloudflare dev and worker paths can evaluate instrumentation and route modules
+ * through different module instances. Store state on globalThis so the startup
+ * register() path and the request-handling path observe the same values.
  */
 
 export type CapturedRequestError = {
@@ -23,29 +15,44 @@ export type CapturedRequestError = {
   routeType: string;
 }
 
-/** Set to true when instrumentation.ts register() is called. */
-let registerCalled = false;
+type InstrumentationState = {
+  capturedErrors: CapturedRequestError[];
+  registerCalled: boolean;
+};
+
+function getInstrumentationState(): InstrumentationState {
+  const scopedGlobal = globalThis as typeof globalThis & {
+    __VINEXT_CLOUDFLARE_INSTRUMENTATION_STATE__?: InstrumentationState;
+  };
 
-/** List of errors captured by onRequestError(). */
-const capturedErrors: CapturedRequestError[] = [];
+  if (!scopedGlobal.__VINEXT_CLOUDFLARE_INSTRUMENTATION_STATE__) {
+    scopedGlobal.__VINEXT_CLOUDFLARE_INSTRUMENTATION_STATE__ = {
+      capturedErrors: [],
+      registerCalled: false,
+    };
+  }
+
+  return scopedGlobal.__VINEXT_CLOUDFLARE_INSTRUMENTATION_STATE__;
+}
 
 export function isRegisterCalled(): boolean {
-  return registerCalled;
+  return getInstrumentationState().registerCalled;
 }
 
 export function getCapturedErrors(): CapturedRequestError[] {
-  return [...capturedErrors];
+  return [...getInstrumentationState().capturedErrors];
 }
 
 export function markRegisterCalled(): void {
-  registerCalled = true;
+  getInstrumentationState().registerCalled = true;
 }
 
 export function recordRequestError(entry: CapturedRequestError): void {
-  capturedErrors.push(entry);
+  getInstrumentationState().capturedErrors.push(entry);
 }
 
 export function resetInstrumentationState(): void {
-  registerCalled = false;
-  capturedErrors.length = 0;
+  const state = getInstrumentationState();
+  state.registerCalled = false;
+  state.capturedErrors.length = 0;
 }

packages/vinext/src/shims/router.ts

@@ -118,6 +118,21 @@ function resolveUrl(url: string | UrlObject): string {
   return result;
 }
 
+/**
+ * When `as` is provided, use it as the navigation target. This is a
+ * simplification: Next.js keeps `url` and `as` as separate values (url for
+ * data fetching, as for the browser URL). We collapse them because vinext's
+ * navigateClient() fetches HTML from the target URL, so `as` must be a
+ * server-resolvable path. Purely decorative `as` values are not supported.
+ */
+function resolveNavigationTarget(
+  url: string | UrlObject,
+  as: string | undefined,
+  locale: string | undefined,
+): string {
+  return applyNavigationLocale(as ?? resolveUrl(url), locale);
+}
+
 /**
  * Apply locale prefix to a URL for client-side navigation.
  * Same logic as Link's applyLocaleToHref but reads from window globals.
@@ -469,12 +484,8 @@ export function useRouter(): NextRouter {
   }, []);
 
   const push = useCallback(
-    async (
-      url: string | UrlObject,
-      _as?: string,
-      options?: TransitionOptions,
-    ): Promise<boolean> => {
-      let resolved = applyNavigationLocale(resolveUrl(url), options?.locale);
+    async (url: string | UrlObject, as?: string, options?: TransitionOptions): Promise<boolean> => {
+      let resolved = resolveNavigationTarget(url, as, options?.locale);
 
       // External URLs — delegate to browser (unless same-origin)
       if (isExternalUrl(resolved)) {
@@ -524,12 +535,8 @@ export function useRouter(): NextRouter {
   );
 
   const replace = useCallback(
-    async (
-      url: string | UrlObject,
-      _as?: string,
-      options?: TransitionOptions,
-    ): Promise<boolean> => {
-      let resolved = applyNavigationLocale(resolveUrl(url), options?.locale);
+    async (url: string | UrlObject, as?: string, options?: TransitionOptions): Promise<boolean> => {
+      let resolved = resolveNavigationTarget(url, as, options?.locale);
 
       // External URLs — delegate to browser (unless same-origin)
       if (isExternalUrl(resolved)) {
@@ -671,8 +678,8 @@ export function wrapWithRouterContext(element: ReactElement): ReactElement {
 
 // Also export a default Router singleton for `import Router from 'next/router'`
 const Router = {
-  push: async (url: string | UrlObject, _as?: string, options?: TransitionOptions) => {
-    let resolved = applyNavigationLocale(resolveUrl(url), options?.locale);
+  push: async (url: string | UrlObject, as?: string, options?: TransitionOptions) => {
+    let resolved = resolveNavigationTarget(url, as, options?.locale);
 
     // External URLs (unless same-origin)
     if (isExternalUrl(resolved)) {
@@ -715,8 +722,8 @@ const Router = {
     window.dispatchEvent(new CustomEvent("vinext:navigate"));
     return true;
   },
-  replace: async (url: string | UrlObject, _as?: string, options?: TransitionOptions) => {
-    let resolved = applyNavigationLocale(resolveUrl(url), options?.locale);
+  replace: async (url: string | UrlObject, as?: string, options?: TransitionOptions) => {
+    let resolved = resolveNavigationTarget(url, as, options?.locale);
 
     // External URLs (unless same-origin)
     if (isExternalUrl(resolved)) {

tests/e2e/helpers.ts

@@ -0,0 +1,5 @@
+import type { Page } from "@playwright/test";
+
+export async function waitForHydration(page: Page) {
+  await page.waitForFunction(() => Boolean((window as any).__VINEXT_ROOT__));
+}

tests/e2e/pages-router/hydration.spec.ts

@@ -1,4 +1,5 @@
 import { test, expect } from "../fixtures";
+import { waitForHydration } from "../helpers";
 
 const BASE = "http://localhost:4173";
 
@@ -12,7 +13,8 @@ test.describe("Hydration", () => {
     // SSR should render the initial count
     await expect(page.locator('[data-testid="count"]')).toContainText("Count:");
 
-    // Wait for hydration — button should become interactive
+    await waitForHydration(page);
+
     await page.click('[data-testid="increment"]');
     await expect(page.locator('[data-testid="count"]')).toHaveText("Count: 1");
 
@@ -57,6 +59,7 @@ test.describe("Hydration", () => {
   test("state is preserved within client navigation", async ({ page, consoleErrors }) => {
     // Start at counter page, increment, then navigate away and back
     await page.goto(`${BASE}/counter`);
+    await waitForHydration(page);
     await page.click('[data-testid="increment"]');
     await page.click('[data-testid="increment"]');
     await expect(page.locator('[data-testid="count"]')).toHaveText("Count: 2");

tests/e2e/pages-router/navigation.spec.ts

@@ -1,11 +1,13 @@
 import { test, expect } from "@playwright/test";
+import { waitForHydration } from "../helpers";
 
 const BASE = "http://localhost:4173";
 
 test.describe("Client-side navigation", () => {
   test("Link click navigates without full page reload", async ({ page }) => {
     await page.goto(`${BASE}/`);
     await expect(page.locator("h1")).toHaveText("Hello, vinext!");
+    await waitForHydration(page);
 
     // Store a marker on the window to detect if page fully reloaded
     await page.evaluate(() => {
@@ -29,6 +31,7 @@ test.describe("Client-side navigation", () => {
   test("Link navigates back to home from about", async ({ page }) => {
     await page.goto(`${BASE}/about`);
     await expect(page.locator("h1")).toHaveText("About");
+    await waitForHydration(page);
 
     await page.evaluate(() => {
       (window as any).__NAV_MARKER__ = true;
@@ -45,6 +48,7 @@ test.describe("Client-side navigation", () => {
   test("router.push navigates to a new page", async ({ page }) => {
     await page.goto(`${BASE}/nav-test`);
     await expect(page.locator("h1")).toHaveText("Navigation Test");
+    await waitForHydration(page);
 
     await page.evaluate(() => {
       (window as any).__NAV_MARKER__ = true;
@@ -58,6 +62,24 @@ test.describe("Client-side navigation", () => {
     expect(page.url()).toBe(`${BASE}/about`);
   });
 
+  test("router.push(url, as) uses the masked URL while resolving the real route", async ({
+    page,
+  }) => {
+    await page.goto(`${BASE}/nav-test`);
+    await expect(page.locator("h1")).toHaveText("Navigation Test");
+    await waitForHydration(page);
+
+    await page.click('[data-testid="push-post-as-hook"]');
+    await expect(page.locator('[data-testid="post-title"]')).toHaveText("Post: 42");
+    await expect(page.locator('[data-testid="query"]')).toHaveText("Query ID: 42");
+    await expect(page.locator('[data-testid="as-path"]')).toHaveText(
+      "As Path: /posts/42?from=hook",
+    );
+    // TODO(#462): after implementing two-value url/as navigation, assert that
+    // router.pathname stays as the route pattern (/posts/[id]) not the resolved path.
+    expect(page.url()).toBe(`${BASE}/posts/42?from=hook`);
+  });
+
   test("router.replace navigates without adding history entry", async ({ page }) => {
     // Start at home, then go to nav-test, then replace to SSR
     await page.goto(`${BASE}/`);
@@ -66,6 +88,7 @@ test.describe("Client-side navigation", () => {
     // Navigate to nav-test via direct navigation
     await page.goto(`${BASE}/nav-test`);
     await expect(page.locator("h1")).toHaveText("Navigation Test");
+    await waitForHydration(page);
 
     await page.click('[data-testid="replace-ssr"]');
     await expect(page.locator("h1")).toHaveText("Server-Side Rendered");
@@ -76,9 +99,30 @@ test.describe("Client-side navigation", () => {
     await expect(page.locator("h1")).not.toHaveText("Navigation Test");
   });
 
+  test("Router.replace(url, as) uses the masked URL for singleton navigation", async ({ page }) => {
+    await page.goto(`${BASE}/`);
+    await expect(page.locator("h1")).toHaveText("Hello, vinext!");
+
+    await page.goto(`${BASE}/nav-test`);
+    await expect(page.locator("h1")).toHaveText("Navigation Test");
+    await waitForHydration(page);
+
+    await page.click('[data-testid="replace-post-as-singleton"]');
+    await expect(page.locator('[data-testid="post-title"]')).toHaveText("Post: 84");
+    await expect(page.locator('[data-testid="query"]')).toHaveText("Query ID: 84");
+    await expect(page.locator('[data-testid="as-path"]')).toHaveText(
+      "As Path: /posts/84?from=singleton",
+    );
+    expect(page.url()).toBe(`${BASE}/posts/84?from=singleton`);
+
+    await page.goBack();
+    await expect(page.locator("h1")).toHaveText("Hello, vinext!");
+  });
+
   test("browser back/forward buttons work after client navigation", async ({ page }) => {
     await page.goto(`${BASE}/`);
     await expect(page.locator("h1")).toHaveText("Hello, vinext!");
+    await waitForHydration(page);
 
     // Navigate: Home -> About via link
     await page.click('a[href="/about"]');
@@ -98,6 +142,7 @@ test.describe("Client-side navigation", () => {
   test("multiple sequential navigations work", async ({ page }) => {
     await page.goto(`${BASE}/nav-test`);
     await expect(page.locator("h1")).toHaveText("Navigation Test");
+    await waitForHydration(page);
 
     await page.evaluate(() => {
       (window as any).__NAV_MARKER__ = true;
@@ -123,6 +168,7 @@ test.describe("Client-side navigation", () => {
   test("navigating to SSR page fetches fresh server data", async ({ page }) => {
     await page.goto(`${BASE}/nav-test`);
     await expect(page.locator("h1")).toHaveText("Navigation Test");
+    await waitForHydration(page);
 
     await page.click('[data-testid="link-ssr"]');
     await expect(page.locator("h1")).toHaveText("Server-Side Rendered");

tests/fixtures/pages-basic/pages/nav-test.tsx

@@ -1,4 +1,4 @@
-import { useRouter } from "next/router";
+import Router, { useRouter } from "next/router";
 import Link from "next/link";
 
 export default function NavTestPage() {
@@ -16,6 +16,34 @@ export default function NavTestPage() {
       <button data-testid="push-counter" onClick={() => router.push("/counter")}>
         Push to Counter
       </button>
+      <button
+        data-testid="push-post-as-hook"
+        onClick={() =>
+          router.push(
+            {
+              pathname: "/posts/[id]",
+              query: { id: "42", from: "hook" },
+            },
+            "/posts/42?from=hook",
+          )
+        }
+      >
+        Push masked Post via hook
+      </button>
+      <button
+        data-testid="replace-post-as-singleton"
+        onClick={() =>
+          Router.replace(
+            {
+              pathname: "/posts/[id]",
+              query: { id: "84", from: "singleton" },
+            },
+            "/posts/84?from=singleton",
+          )
+        }
+      >
+        Replace masked Post via singleton
+      </button>
       <Link href="/" data-testid="link-home">
         Link to Home
       </Link>

tests/fixtures/pages-basic/pages/posts/[id].tsx

@@ -11,6 +11,7 @@ export default function Post({ id }: PostProps) {
     <div>
       <h1 data-testid="post-title">Post: {id}</h1>
       <p data-testid="pathname">Pathname: {router.pathname}</p>
+      <p data-testid="as-path">As Path: {router.asPath}</p>
       <p data-testid="query">Query ID: {router.query.id}</p>
     </div>
   );