Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 30 additions & 1 deletion packages/vinext/src/server/app-browser-entry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ import { createPopstateRestoreHandler } from "./app-browser-popstate.js";
import { DevRecoveryBoundary, RedirectBoundary } from "vinext/shims/error-boundary";
import { AppRouterContext } from "vinext/shims/internal/app-router-context";
import { ElementsContext, Slot } from "vinext/shims/slot";
import { stripBasePath } from "../utils/base-path.js";
import { createOnUncaughtError } from "./app-browser-error.js";
import {
devOnCaughtError,
Expand Down Expand Up @@ -643,6 +644,20 @@ function restorePopstateScrollPosition(state: unknown): void {
});
}

function isSameAppRoutePopstateTarget(href: string): boolean {
if (!hasBrowserRouterState()) return false;

const target = new URL(href, window.location.origin);
const routerState = getBrowserRouterState();
const targetPathname = stripBasePath(target.pathname, __basePath);
const targetSearch = new URLSearchParams(target.search).toString();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: new URLSearchParams(target.search).toString() round-trips the search string through URLSearchParams normalization (sorts nothing, but does strip a leading ? and re-encode). Since the committed snapshot's searchParams was also constructed via new ReadonlyURLSearchParams(url.search) (in createClientNavigationRenderSnapshot), both sides go through the same normalization, so this works correctly.

If you wanted to shave the allocation, target.search (which includes the ?) could be compared against a pre-stored raw search string, but the URLSearchParams approach is more robust against encoding differences, so this is fine as-is.

const currentSearch = routerState.navigationSnapshot.searchParams.toString();

return (
targetPathname === routerState.navigationSnapshot.pathname && targetSearch === currentSearch
);
}

// Set on pagehide so the RSC navigation catch block can distinguish expected
// fetch aborts (triggered by the unload itself) from real errors worth logging.
let isPageUnloading = false;
Expand Down Expand Up @@ -1320,7 +1335,21 @@ function bootstrapHydration(rscStream: ReadableStream<Uint8Array>): void {
},
});

window.addEventListener("popstate", handlePopstate);
window.addEventListener("popstate", (event) => {
// The browser has already applied the history entry by the time popstate
// fires. App Router state does not include hashes, so matching the
// committed pathname/search proves this traversal does not need a new RSC
// payload. This covers both /page#target -> /page and /page -> /page#target.
// Notify the transition start so observers still see the URL change, then
// restore scroll directly and skip the RSC dispatch.
const href = window.location.href;
if (isSameAppRoutePopstateTarget(href)) {
notifyAppRouterTransitionStart(href, "traverse");
restorePopstateScrollPosition(event.state);
return;
}
handlePopstate(event);
});

if (import.meta.hot) {
const handleRscUpdate = async (): Promise<void> => {
Expand Down
46 changes: 46 additions & 0 deletions tests/e2e/app-router/nextjs-compat/hash-popstate-scroll.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,52 @@ test.describe("Next.js compat: hash popstate scroll", () => {
await expect(page.locator("#content")).toBeInViewport();
});

// Next.js App Router handles popstate with ACTION_RESTORE and classifies
// same-path/search fragment changes as onlyHashChange in segment-cache
// navigation, avoiding a new RSC payload for hash-only traversal.
// Source:
// https://github.com/vercel/next.js/blob/canary/packages/next/src/client/components/app-router.tsx
// https://github.com/vercel/next.js/blob/canary/packages/next/src/client/components/segment-cache/navigation.ts
test("back/forward traversal between hash entries skips RSC navigation", async ({ page }) => {
await page.goto(`${BASE}/nextjs-compat/hash-popstate-scroll`);
await waitForAppRouterHydration(page);

await page.click("#hash-link");
await expect(page).toHaveURL(`${BASE}/nextjs-compat/hash-popstate-scroll#content`);

await page.evaluate(() => {
const testWindow = window as Window & { __vinextHashPopstateRscCalls?: number };
const originalNavigate = window.__VINEXT_RSC_NAVIGATE__;
if (typeof originalNavigate !== "function") {
throw new Error("__VINEXT_RSC_NAVIGATE__ is not installed");
}
window.__VINEXT_RSC_NAVIGATE__ = async (...args) => {
testWindow.__vinextHashPopstateRscCalls =
(testWindow.__vinextHashPopstateRscCalls ?? 0) + 1;
return originalNavigate(...args);
};
testWindow.__vinextHashPopstateRscCalls = 0;
});

await page.goBack();
await expect(page).toHaveURL(`${BASE}/nextjs-compat/hash-popstate-scroll`);
await expectScrollY(page, 0);

await page.goForward();
await expect(page).toHaveURL(`${BASE}/nextjs-compat/hash-popstate-scroll#content`);
await expect(page.locator("#content")).toBeInViewport();

await expect
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good use of expect.poll here — it handles the case where the assertion runs before the popstate handler has fully completed. The goBack/goForward awaits above ensure the URL has changed, but expect.poll makes the RSC-call-count assertion robust against any remaining async work.

.poll(() =>
page.evaluate(
() =>
(window as Window & { __vinextHashPopstateRscCalls?: number })
.__vinextHashPopstateRscCalls ?? 0,
),
)
.toBe(0);
});

test("forward traversal decodes non-latin hash fragments", async ({ page }) => {
await expectHashForwardTraversal(page, "#encoded-link", "#caf%C3%A9", '[id="café"]');
});
Expand Down
Loading