fix(app-router): canonicalize initial history URLs

[james-elicx]

Summary

• canonicalize initial App Router history URLs so a trailing bare ? is removed
• match Next.js reload behavior for default error pages
• add production browser coverage for the reload-button path

Next.js parity

Targets the retained failure in test/e2e/app-dir/default-error-page-ui/default-error-page-ui.test.ts.

Validation

• targeted Chrome and WebKit production tests passed
• 15 focused unit tests passed
• vp run vinext#build passed
• scoped format/lint/types passed
• independent review found no actionable issues

[james-elicx]

/bigbonk review for issues

[pkg-pr-new]

Open in StackBlitz

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

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

commit: a19c3fd

[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

[ask-bonk]

Only one production caller, fed by window.location.href (always absolute). The function is safe in all real usages.

Review Summary

I reviewed PR #2250 (fix(app-router): canonicalize initial history URLs). No actionable issues found. The change is correct, well-scoped, and well-tested.

Findings

Correctness — verified sound:

• createCanonicalBrowserHistoryHref uses ${url.pathname}${url.search}${url.hash}, which collapses a bare trailing ? (since new URL("...?").search === ""). I confirmed this empirically across several inputs. This pattern is consistent with ~15 existing call sites in the codebase.
• The fix is applied at exactly the right point. The only place a raw bare-? href leaked into history was the replaceState href in writeBootstrapHistoryMetadata. Later writes (writeHydratedHistoryMetadata, syncCurrentHistoryStatePreviousNextUrl) re-read window.location.href after the bootstrap replaceState has already canonicalized it, so they inherit the fix without modification. Leaving those call sites unchanged is correct, not an oversight.
• The pre-canonical snapshot capture at app-browser-entry.ts:1640 (one line before the canonicalizing write at 1643) is not a bug: createClientNavigationRenderSnapshot builds searchParams from url.search, which already normalizes the bare ? away independently.

Safety:

• new URL(href) requires an absolute URL and would throw on a relative input. The sole production caller feeds it window.location.href (always absolute), so this is safe. No other callers exist.
• replaceState only swaps the path portion against the same origin, and window.location.pathname already includes any base path, so no base-path stripping is needed or lost.

Tests — well-constructed:

• Unit tests cover both the bare-? strip and preservation of real query strings (15 tests pass).
• The e2e test correctly validates the real bug: the Reload button's empty GET form submits to .../reload-error?, and the assertion page.url() === urlBeforeReload (.../reload-error, no ?) would fail without the fix.

Verification run:

• vp test run tests/app-browser-history-controller.test.ts → 15 passed
• vp check on both changed source/test files → format, lint, types all pass

The PR matches Next.js's documented reload behavior, the comments accurately link back to the upstream test, and the implementation is minimal and clean.

github run

[github-actions]

Performance benchmarks

Compared a19c3fd against base cf9d601 using alternating same-runner rounds. Next.js was unchanged and skipped.

1 improved · 0 regressed · 5 within ±1.5%

Scenario	Framework	Baseline	Current	Change
Client bundle size (gzip)	vinext	132.8 KB	132.8 KB	⚫ +0.0%
Client entry size (gzip)	vinext	118.7 KB	118.7 KB	⚫ +0.0%
Dev server cold start	vinext	2.34 s	2.30 s	🟢 -1.8%
Production build time	vinext	2.82 s	2.79 s	⚫ -1.1%
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}