fix(app-router): restore shallow history tree snapshots

[james-elicx]

Summary

• restore the exact App Router tree snapshot copied into raw history.pushState() / replaceState() entries
• keep snapshot identity stable across app navigation, hash traversal, refresh, and repeated back/forward traversal
• release retained snapshots when replace operations or provable forward-history truncation make them unreachable
• preserve captured app-owned entries as real traversal targets, matching Next.js internal History API bypass semantics
• clean up overwritten/truncated snapshot claims for captured app-owned writes without reclassifying their state as external
• add focused unit and browser regressions for same-URL traversal, redirects, identity reuse, refresh, and forward-stack cleanup

Failure mapping

Fixes the non-cache failure in Next.js deploy-suite run 31439707085, job 93624401572:

• test/e2e/app-dir/shallow-routing/shallow-routing.test.ts
• client-side pathname push/back-forward traversal timeout

The original targeted run reproduced the suite at 14 passed / 1 failed.

Validation

• vp check
• vp run knip
• vp run vinext#build
• focused history/browser/shim units: 1,549 passed
• tests/e2e/app-router/nextjs-compat/use-router-bfcache-id.spec.ts: 13 passed / 13 total
• CI=1 PLAYWRIGHT_PROJECT=app-router pnpm exec playwright test tests/e2e/app-router/advanced.spec.ts --grep "Shallow Routing" --retries=0: 11 passed / 1 pre-existing skipped
• targeted Next.js suite via scripts/run-targeted-nextjs-e2e.sh test/e2e/app-dir/shallow-routing/shallow-routing.test.ts: 15 passed / 15 total

The initial draft CI exposed and locally reproduced a traverse-redirect regression. This head fixes it by matching Next.js data.__NA bypass behavior for captured app-owned History state; the formerly failing test and its full 13-test file are green. Focused regressions also prove captured app-owned replace/push writes release overwritten or forward-truncated snapshot claims idempotently while preserving app-owned state.

[james-elicx]

/bigbonk review for issues

[pkg-pr-new]

Open in StackBlitz

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

npm i https://pkg.pr.new/create-vinext-app@2885

npm i https://pkg.pr.new/@vinext/types@2885

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

commit: c6b02d9

[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

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: fix(app-router): restore shallow history tree snapshots

Reviewed the full diff, ran the targeted unit suite (app-browser-history-controller, app-browser-entry, shims → 1547 passed), vp check on all 7 modified source files (clean), and vp run vinext#build (success). The change is well-structured, the reference-counted claim/release model is sound, and the test coverage (unit + 8 new E2E scenarios ported from Next.js shallow-routing.test.ts) is thorough.

Findings

No blocking issues found. A few observations, none requiring a change:

1. #treeSnapshots retention is intentionally unbounded for claimed entries (not a leak).
Unlike the traversal-index cache (maxHistoryStateSnapshots), snapshots claimed by raw pushState/replaceState/hash entries are retained for the document lifetime until forward-truncation or a same-index replace releases them. This is correct and deliberate — the browser gives no signal that a copied external entry is unreachable, and the retains reachable external tree snapshots across traversal-cache eviction test locks it in. Worth calling out only because the growth is bounded by user pushState count, which is inherent to shallow routing.

2. Map mutation during iteration in #releaseForwardTreeSnapshotClaims (line 306-314).
The loop iterates this.#treeSnapshotClaimByHistoryIndex.keys() and calls #releaseTreeSnapshotClaimAtHistoryIndex, which .delete()s from the same Map mid-iteration. This is well-defined and safe per the JS spec for Map (deleting the current or already-visited key doesn't skip remaining entries), so no bug — just flagging the pattern for future readers since it's subtle. The releases raw replace claims/forward-truncation tests exercise it.

Dev/prod parity ✅

The client history logic lives entirely in app-browser-entry.ts and the shared server/app-browser-* helpers. App Router production delegates to the built RSC entry, which reuses this browser entry, so the fix is inherited. No Pages Router equivalent exists for this App Router shallow-routing behavior, so no separate prod-server update is needed.

Behavioral correctness spot-checks ✅

• shouldCommitPopstateUrlWithoutNavigation preserves the prior isSameAppRoutePopstateTarget behavior for non-external states (no regression), and only short-circuits external entries when the live tree already is that external tree.
• #claimTreeSnapshotAtHistoryIndex releases any prior claim at the index before re-claiming, so replace-at-same-index doesn't double-count.
• createHistoryStateWithTreeSnapshotId clears the CLAIMED marker when the id changes, keeping the serialized marker consistent with the controller's authoritative claim maps.

LGTM.

[ask-bonk]

Review posted to PR #2885.

Summary

I reviewed PR #2885 (restore shallow history tree snapshots). Verification performed:

• Targeted unit tests: 1547 passed (app-browser-history-controller, app-browser-entry, shims)
• vp check on all 7 modified source files: clean (format + lint + types)
• vp run vinext#build: success

Verdict: LGTM, no blocking issues.

The reference-counted claim/release model for tree snapshots is sound, and coverage is thorough (unit + 8 new E2E scenarios ported from Next.js shallow-routing.test.ts).

Two non-blocking observations flagged for future readers:

1. Unbounded retention of claimed snapshots is intentional (not a leak) — the browser gives no signal that a copied external entry is unreachable; growth is bounded by user pushState count.
2. Map mutation during iteration in #releaseForwardTreeSnapshotClaims is well-defined/safe per the JS spec, just a subtle pattern.

I also confirmed dev/prod parity (client history logic is shared via the browser entry the App Router prod server inherits; no Pages Router equivalent) and spot-checked the popstate short-circuit for non-regression, the same-index re-claim release, and the serialized CLAIMED marker consistency.

github run

[github-actions]

Performance benchmarks

Compared c6b02d9 against base d558d62 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	135.2 KB	136.3 KB	⚫ +0.8%
Client entry size (gzip)	vinext	122.7 KB	123.8 KB	⚫ +0.9%
Dev server cold start	vinext	3.03 s	3.00 s	⚫ -1.0%
Production build time	vinext	3.24 s	3.24 s	⚫ +0.1%
RSC entry closure size (gzip)	vinext	114.2 KB	114.2 KB	⚫ -0.0%
Server bundle size (gzip)	vinext	193.0 KB	193.0 KB	⚫ +0.0%

View detailed results and traces

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

[james-elicx]

/bigbonk review for issues

[ask-bonk]

UnknownError: ProviderInitError

github run

[ask-bonk]

@james-elicx Bonk workflow failed. Check the logs for details.

View workflow run · To retry, trigger Bonk again.

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: fix(app-router): restore shallow history tree snapshots

Reviewed the full diff across all 7 source files and 4 test files. Verification performed:

• tests/app-browser-history-controller.test.ts + tests/app-browser-entry.test.ts: 266 passed
• tests/shims.test.ts -t shallow: 5 passed (the ported Next.js shallow-routing shim cases)
• vp check on the four largest changed source files: clean (format + lint + types)

Verdict: LGTM, no blocking issues. The reference-counted claim/release model is sound and the dev/prod parity story holds (client history logic lives in the shared app-browser-* helpers that the App Router prod server inherits via the built RSC entry; Pages Router has no equivalent shallow-routing path).

Correctness spot-checks that hold up

1. data?.__NA bypass parity is faithful. isAppOwnedHistoryState gates on !external && traversalIndex !== null, and the traversal index is only ever stamped by App Router bootstrap. The shims.test.ts case that writes { __vinext_historyIndex: 3, captured: true } and asserts history.state equals it verbatim confirms captured app-owned entries pass through untouched (no external marker, no re-wrapping), matching Next.js.

2. No Pages Router regression from the shared patch. The patched pushState/replaceState is installed globally, but the App-Router-only runtime hooks are optional-chained (getNavigationRuntime()?.functions.X?.()), so they no-op without an App runtime. I confirmed Pages Router (router.ts) never stamps __vinext_historyIndex, so its redirect replaceState(window.history.state ?? {}, ...) calls (router.ts:2249/2488/2568/2649) always take the external branch and still run commitClientNavigationState(). The bypass path's skip of URL-notify is therefore only reached on runtime-issued traversal re-writes, where the popstate handler already drives notification (app-browser-entry.ts:2476).

3. Same-URL replace now detaches a shared claimed tree. The new replacesClaimedOrExternalTree term in commitNavigationHistory makes a same-href replace actually write history + release the claim at the current index, which previously fell through to the no-write branch. This is exactly what the same-URL app replace detaches one shared pushState tree E2E test locks in.

4. Graceful degradation on evicted snapshots. When restoreTreeSnapshot() returns false (snapshot missing) or approval is rejected, restoreHistorySnapshot returns false and the popstate handler falls through to handlePopstate(event) for a real RSC traversal. No dead-end.

Non-blocking observations (agree with the earlier automated pass)

• Unbounded #treeSnapshots retention for claimed entries is deliberate, not a leak. Growth is bounded by user pushState count and mirrors Next.js keeping copied trees for the document lifetime. The traversal-index cache remains separately LRU-bounded.
• Map mutation during iteration in #releaseForwardTreeSnapshotClaims (deleting the current key mid-for...of over .keys()) is well-defined per the ECMAScript Map iteration spec — safe, just subtle.

Fixture and testid dependencies (/shallow-test, push-path, data-testid=pathname, /shallow-test/sub shallow pathname, #content hash) all pre-exist and are consistent with shallow-routing semantics (no real sub/ route or anchor is required — the copied tree stays on /shallow-test).

[ask-bonk]

Review posted to PR #2885.

Summary

I reviewed PR #2885 (restore shallow history tree snapshots) in depth. Verification:

• app-browser-history-controller + app-browser-entry tests: 266 passed
• shims.test.ts -t shallow: 5 passed
• vp check on the four largest changed source files: clean (format + lint + types)

Verdict: LGTM, no blocking issues.

The reference-counted claim/release model for tree snapshots is sound and thoroughly tested (unit + 6 new E2E scenarios ported from Next.js shallow-routing.test.ts).

Key correctness checks I independently confirmed:

1. data?.__NA bypass parity is faithful — isAppOwnedHistoryState gates on !external && traversalIndex !== null, and the traversal index is only stamped by App Router bootstrap. The shim test asserting captured state passes through verbatim confirms it.
2. No Pages Router regression — the shared pushState/replaceState patch no-ops its App-Router-only hooks via optional chaining; Pages Router never stamps __vinext_historyIndex, so its redirect replaceState calls stay on the external branch and still run commitClientNavigationState(). The bypass path's skipped URL-notify is only reached on runtime-issued traversal re-writes, where the popstate handler already notifies.
3. Same-URL replace detaches a claimed tree — the new replacesClaimedOrExternalTree term makes a same-href replace actually write + release the claim (previously it fell through), matching the corresponding E2E test.
4. Graceful degradation — evicted/rejected snapshot restores fall through to a real RSC traversal via handlePopstate.

Non-blocking observations (concur with the earlier pass): unbounded #treeSnapshots retention for claimed entries is deliberate (bounded by user pushState count, mirrors Next.js), and the Map-mutation-during-iteration in #releaseForwardTreeSnapshotClaims is spec-safe but subtle.

I also verified all fixture/testid dependencies pre-exist and are consistent with shallow-routing semantics.

github run

[james-elicx]

Independent targeted Next.js E2E verification on immutable head c6b02d9214b3d4cd4c3ac9fadfc7ddd87edd7664:

• Suite: test/e2e/app-dir/shallow-routing/shallow-routing.test.ts
• Command: required scripts/run-targeted-nextjs-e2e.sh wrapper
• Result: PASS — 15 passed, 0 skipped, 0 failed (15 total)
• Retries: 0/0

The checkout remained clean and the local/remote PR head SHA matched after the run.

[james-elicx]

Independent exact-head review of c6b02d9214b3d4cd4c3ac9fadfc7ddd87edd7664: NO FINDINGS.

I reviewed the full diff independently, including App Router raw/hash pushState and replaceState, copied-tree claim/refcount lifecycle, forward-history truncation, same-entry replacement, app-owned-state bypass parity with Next.js, popstate restoration/URL publication, refresh/cache invalidation, BFCache identity, and dev/production shared browser-runtime behavior.

Validation considered:

• immutable targeted upstream proof: test/e2e/app-dir/shallow-routing/shallow-routing.test.ts passed 15/15, retries 0/0 (proof);
• all 266 focused history-controller and browser-entry unit tests passed;
• an additional history/snapshot/push/replace/popstate/shallow filtered run passed 81/81;
• exact local and remote head SHA matched and the worktree remained clean.

I found no actionable parity gap, lifecycle bug, regression, or missing high-value test at this head.