fix(router): ignore stale initial popstate with i18n

[james-elicx]

What this changes

• Normalizes the tracked browser URL to Next.js's app-relative asPath shape before applying the first-popstate stale-state guard.
• Tracks the previous hash separately so identical no-hash popstate events are not misclassified as hash-only navigation.
• Adds a focused Pages Router i18n fixture covering the three upstream cases: no query, query present, and a different locale.

Why

Next.js ignores only the first stale popstate event when its locale matches the active locale and its state.as matches the current app-relative asPath. vinext compared that app-relative value with a browser pathname that still contained the locale prefix, so the stale event was not ignored for i18n routes.

Next.js references

Pinned Next.js reference commit: ee6e79b1792a4d401ddf2480f40a83549fe8e722

• Upstream i18n regression coverage
• Pages Router popstate implementation

Targeted local validation

Completed before review, with no full suite or raw Next.js E2E run:

• 3 focused Playwright assertions in tests/e2e/pages-router-invalid-popstate-i18n/invalid-popstate.spec.ts, one worker.
• 3 nearby focused unit assertions covering popstate/hash-only behavior.
• Independent exact-head review completed with no actionable findings.

[james-elicx]

/bigbonk review for issues

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/@vinext/cloudflare@2230

npm i https://pkg.pr.new/vinext@2230

commit: c7a3203

[github-actions]

Example	Preview	Production	Original
app-router-cloudflare	preview	production
pages-router-cloudflare	preview	production
app-router-playground	preview	production	original
realworld-api-rest	preview	production
nextra-docs-template	preview	production
benchmarks	preview	production
hackernews	preview	production	original
workers-cache	preview	production
web	preview	production

[github-actions]

Performance benchmarks

Compared c7a3203 against base 82d5f76 using alternating same-runner rounds. Next.js was unchanged and skipped.

0 improved · 0 regressed · 6 within ±1.5%

Scenario	Framework	Baseline	Current	Change
Client bundle size (gzip)	vinext	132.8 KB	132.9 KB	⚫ +0.0%
Client entry size (gzip)	vinext	118.7 KB	118.7 KB	⚫ -0.0%
Dev server cold start	vinext	2.38 s	2.36 s	⚫ -0.6%
Production build time	vinext	2.94 s	2.90 s	⚫ -1.3%
RSC entry closure size (gzip)	vinext	92.2 KB	92.2 KB	⚫ -0.0%
Server bundle size (gzip)	vinext	166.9 KB	166.9 KB	⚫ +0.0%

View detailed results and traces

_{🟢 improvement · 🔴 regression · ⚫ change below 1.5% · paired base/head}

[ask-bonk]

@james-elicx Bonk workflow was cancelled.

View workflow run · To retry, trigger Bonk again.

[james-elicx]

/bigbonk review for issues

[james-elicx]

/bigbonk review for issues

[ask-bonk]

/bigbonk review — PR #2230

Reviewed the popstate i18n stale-state fix. No actionable issues found. The change is correct, well-scoped, and consistent with the existing tracking pattern. Notes below.

Correctness — i18n guard normalization ✅

getTrackedPagesRouterAsPath() (router.ts:2867) now derives the comparison value the same way Next.js does — strip basePath, then strip the active locale prefix — so it compares apples to apples with state.as (which is app-relative, no basePath, no locale). The old code compared withBasePath(state.as, ...) against lastPathnameAndSearch, which still carried the locale prefix, so the stale-event guard never matched for i18n routes. Tracing the fixture: goto("/sv/static") → tracked /sv/static → strip basePath (none) → strip sv → /static, which matches state.as: "/static". Guard fires only when locale + asPath both match, matching the upstream isFirstPopStateEvent && this.locale === state.options.locale && state.as === this.asPath check.

Correctness — separate lastHash tracking ✅

The new isHashOnly condition:

browserUrl === lastPathnameAndSearch && (currentHash !== lastHash || currentHash !== "")

The only behavioral change vs. main is the no-hash↔no-hash case: previously a same-path popstate with no hash was misclassified as hash-only (skipping the page fetch); now ("" !== "" || "" !== "") → false, so it correctly falls through to a full navigation. This is exactly what makes the second popstate in the fixture reach the [dynamic] page after the first stale event is ignored. The hash↔no-hash and non-empty-hash cases all still resolve to hash-only as before — no regression.

Tracker parity ✅

lastHash is updated alongside lastPathnameAndSearch at every write site the diff touches: createPagesRouterRuntimeState (init), navigateClientData, navigateClientHtml, navigateClient (redirect), updateHistory (incl. the hash-only push path at router.ts:2592), and the popstate tracker update at router.ts:2985. I checked the hash-only push path specifically — it routes through updateHistory, so lastHash stays in sync, meaning a later goBack from /page#section → /page is still correctly detected as hash-only.

Tests / config ✅

• New Playwright project pages-router-invalid-popstate-i18n follows the existing fixture pattern (symlinked node_modules, unique port 4191, auto-registers in activeProjects).
• The spec covers the three upstream cases (no query, query, different locale), mirroring test/e2e/ignore-invalid-popstateevent/with-i18n.test.ts.
• vp check passes on router.ts; full tests/shims.test.ts (1142 tests) passes.

Minor / non-blocking

• The PR description claims "3 nearby focused unit assertions covering popstate/hash-only behavior" but no unit-test changes appear in the diff — only the e2e spec is added. The popstate/hash-only logic lives in a browser-only handler, so e2e is the right level; just flagging the description/diff mismatch in case dedicated unit coverage was intended.

[ask-bonk]

Review posted to PR #2230. Summary:

No actionable issues found. The fix is correct and well-scoped.

What I verified:

• i18n guard normalization (router.ts:2867) — getTrackedPagesRouterAsPath() now strips both basePath and locale prefix, so it compares correctly against the app-relative state.as. The old code left the locale prefix on the tracked value, which is why the stale-event guard never matched for i18n routes.
• Separate lastHash tracking — the only behavioral change in isHashOnly is the no-hash↔no-hash case, which now correctly falls through to a full navigation instead of being misclassified as hash-only. This is what lets the second popstate reach the dynamic page. No regression in the hash↔no-hash or non-empty-hash cases.
• Tracker parity — lastHash is updated at every write site that updates lastPathnameAndSearch, including the hash-only push path through updateHistory.
• Config/tests — new Playwright project follows existing patterns (unique port 4191, auto-registers), spec mirrors upstream's three cases. vp check passes and all 1142 shims.test.ts tests pass.

One non-blocking note: the PR description mentions "3 nearby focused unit assertions" but the diff only adds the e2e spec — flagged the description/diff mismatch, though e2e is the appropriate level for browser-only popstate logic.

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

/bigbonk review — PR #2230

Independent deep review of the popstate i18n stale-state fix. No actionable issues found. Verified correctness against the pinned Next.js reference (ee6e79b) including the exact onlyAHashChange truth table, and confirmed full tracker parity.

isHashOnly matches Next.js onlyAHashChange exactly ✅

The new condition browserUrl === lastPathnameAndSearch && (currentHash !== lastHash || currentHash !== "") produces a truth table identical to upstream onlyAHashChange (router.ts L2301) for the same-path case:

currentHash	lastHash	vinext result	Next.js onlyAHashChange
""	""	full nav	oldHash !== newHash → false → full nav ✅
"#a"	""	hash-only	oldHash !== newHash → true → hash-only ✅
""	"#a"	hash-only	oldHash !== newHash → true → hash-only ✅
"#a"	"#a"	hash-only	newHash && oldUrlNoHash===newUrlNoHash && oldHash===newHash → true → hash-only ✅
"#a"	"#b"	hash-only	oldHash !== newHash → true → hash-only ✅

The only behavioral change vs. main is the no-hash↔no-hash row, which previously misclassified a same-path popstate as hash-only (skipping the page fetch). This is precisely what now lets the second stale popstate in the fixture reach the [dynamic] page. The non-empty-hash identity case (#a→#a) staying hash-only also matches upstream's explicit cond-1 branch — not a regression.

i18n guard normalization is correct ✅

getTrackedPagesRouterAsPath() (router.ts:2867) reconstructs the upstream this.asPath shape — strip basePath, then removeNavigationLocalePrefix — so it compares apples-to-apples with state.as (app-relative, no basePath/locale). Mirrors upstream isFirstPopStateEvent && this.locale === state.options.locale && state.as === this.asPath (router.ts:928-935). The old code compared withBasePath(state.as, ...) against lastPathnameAndSearch, which retained the locale prefix, so the guard never matched for i18n routes. Tracing the fixture: /sv/static tracked → strip basePath (none) → strip sv → /static === state.as: "/static" → guard fires. removeNavigationLocalePrefix reuses the established getLocalePathPrefix helper.

Tracker parity verified ✅

All 5 lastPathnameAndSearch = ... write sites (1701, 2024, 2118, 2309, 2984) have a paired lastHash write (1702, 2025, 2120, 2310, 2985), plus the init in createPagesRouterRuntimeState. The hash-only push path (router.ts:2592) routes through updateHistory, which writes both, so a later goBack from /page#section → /page stays correctly classified. No write site updates one without the other.

vinext-specific !routerDidNavigate guard is sound ✅

The added && !routerRuntimeState.routerDidNavigate condition (beyond upstream) is safe for the upstream test cases: the first (ignored) popstate returns early before routerDidNavigate is set, and isFirstPopStateEvent is cleared at the top, so the second popstate skips the guard and navigates. The early return correctly leaves trackers unchanged for a true replay.

Tests / config ✅

• New Playwright project pages-router-invalid-popstate-i18n: unique port 4191, follows the existing symlink-node_modules pattern, auto-registers via projectServers/activeProjects.
• Spec mirrors the three upstream cases (no query, query, different locale) from test/e2e/ignore-invalid-popstateevent/with-i18n.test.ts.
• Fixture next.config.mjs i18n format matches pages-basic.
• vp check packages/vinext/src/shims/router.ts passes; tests/shims.test.ts popstate + hash subsets (38 tests) pass.

Minor / non-blocking

• Stale comment phrasing: the comment block at router.ts:2898-2900 still describes the tracker as "browser-shaped, with basePath" immediately before the new note (L2905-2907) about normalizing it. It's technically consistent (the tracker is browser-shaped; we normalize a copy for comparison), but the two paragraphs read as mildly contradictory on a quick scan. Optional cleanup.
• PR description mismatch: the description claims "3 nearby focused unit assertions" but the diff only adds the e2e spec. e2e is the right level for browser-only popstate logic, so this is just a description/diff discrepancy worth noting.

[ask-bonk]

Review posted to PR #2230.

Summary

No actionable issues found. I independently verified the fix against the pinned Next.js reference (ee6e79b).

Key findings:

• isHashOnly logic is correct — the new condition (currentHash !== lastHash || currentHash !== "") produces a truth table identical to Next.js's onlyAHashChange. The only behavioral change is the no-hash↔no-hash case (now a full navigation instead of being misclassified as hash-only), which is exactly what lets the second stale popstate reach the dynamic page.
• i18n guard normalization is correct — getTrackedPagesRouterAsPath() strips basePath + locale prefix so the tracked value compares apples-to-apples with state.as, matching upstream's state.as === this.asPath.
• Tracker parity verified — all 5 lastPathnameAndSearch write sites have a paired lastHash write, including the hash-only push path through updateHistory.
• vinext-specific !routerDidNavigate guard is sound for the upstream test cases.
• Config/tests follow existing patterns; vp check and the popstate/hash shim tests pass.

Two non-blocking notes: a mildly contradictory comment block at router.ts:2898-2907, and the PR description's "3 unit assertions" claim doesn't match the diff (only the e2e spec is added — which is the appropriate level for browser-only popstate logic).

github run