fix(app-router): keep refresh transitions pending

[NathanDrake2406]

Overview

Field	Detail
Goal	Keep useTransition() pending while router.refresh() is fetching and committing the refreshed RSC tree.
Core change	Programmatic refresh navigations now create the same pending browser router state used by other programmatic App Router navigations.
Primary files	packages/vinext/src/server/app-browser-entry.ts, tests/e2e/app-router/nextjs-compat/router-push-pending.spec.ts
Expected impact	Apps that wrap router.refresh() in startTransition can show loading feedback until the refreshed route commits.

Why

A programmatic App Router navigation that starts inside a React transition must publish a pending router state before the async RSC work begins, then resolve it when the committed tree is ready. Next.js does this for ACTION_REFRESH through the same async action queue path as other router actions, so excluding refresh in vinext lets isPending drop to idle during the refresh request.

Area	Principle / invariant	What this PR changes
Browser App Router	Programmatic navigations should keep the caller's transition pending until commit.	Removes the refresh-only exclusion from pending browser router state creation.
Next.js parity	router.refresh() is an async App Router action, not a fire-and-forget fetch from React's perspective.	Aligns refresh scheduling with Next.js' ACTION_REFRESH dispatch and action queue behavior.
Regression coverage	The observable contract is browser-visible useTransition state.	Adds a slow same-route refresh fixture and Playwright assertion for pending then idle.

What changed

Scenario	Before	After
startTransition(() => router.refresh())	isPending could return to false while the refresh RSC request was still in flight.	isPending becomes true during the refresh and returns to false after the refreshed tree commits.
router.push() pending behavior	Already covered and unchanged.	Existing push pending tests still pass.

Maintainer review path

1. packages/vinext/src/server/app-browser-entry.ts: confirm navigateRsc now treats refresh like other programmatic transitions when deciding whether to create pending browser router state.
2. tests/fixtures/app-basic/app/nextjs-compat/router-refresh-pending/*: inspect the minimal fixture that delays the server response outside Suspense so pending state is observable.
3. tests/e2e/app-router/nextjs-compat/router-push-pending.spec.ts: review the new refresh regression alongside the existing push pending coverage.

Validation

• vp test run tests/app-browser-entry.test.ts
• vp run vinext#build
• PLAYWRIGHT_PROJECT=app-router npx playwright test tests/e2e/app-router/nextjs-compat/router-push-pending.spec.ts --project=app-router
• vp check

Note: the browser regression failed when run against the pre-rebuild package dist, which still contained the refresh exclusion. After rebuilding vinext so the fixture served the patched runtime, it passed.

Risk / compatibility

• Public API: no API shape changes.
• Runtime: refresh now publishes a deferred router-state promise for programmatic transitions, matching the existing push/replace path.
• Loading UI: the refresh request still uses the existing refresh preserve-UI render mode, so route loading boundaries remain suppressed for refresh.
• Cache behavior: unchanged. Refresh cache invalidation still happens before dispatch in the navigation shim.

References

Reference	Why it matters
Next.js public app router instance	refresh() dispatches ACTION_REFRESH inside startTransition.
Next.js action queue	Non-restore router actions publish a deferred state promise to React and resolve it when the action completes.
Next.js useRouter().refresh() docs	Documents refresh as a new server request that re-renders Server Components and merges the payload.
Next.js navigation pending coverage	Existing compat reference for keeping pending state across async navigations.

[pkg-pr-new]

Open in StackBlitz

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

commit: 81cc68d

[james-elicx]

/bigbonk review

[ask-bonk]

Clean, well-scoped fix. The one-line change is correct: refresh is a programmatic navigation dispatched through React.startTransition (navigation.ts:1420) with programmaticTransition=true (navigation.ts:1418), so it should participate in the pending browser router state mechanism like push and replace already do.

The fixture and E2E test are well-structured — the 1s server delay gives reliable headroom for the 1s assertion timeout, the server stamp change confirms the refresh actually committed new data, and the test lives in the right spec file alongside the existing push-pending coverage.

Two minor observations below, neither blocking.

[ask-bonk]

Nit: The 1s server delay and the 1s Playwright assertion timeout for observing "pending" are exactly matched. If the dev server is slow to start the RSC response (e.g. CI load spike), the delay might not provide enough headroom and the test could flake. The existing push-pending fixture also uses 1s, so this is consistent, but worth noting — if either ever flakes, bumping the delay to 1.5–2s (or increasing the assertion timeout to 2s) would add margin.

[ask-bonk]

Nit: Date.now() in the text is used both as a human-readable label ("server stamp: ...") and as the signal that refresh committed new data. This works because every render gets a new timestamp. Just noting: if this page ever gets static/ISR caching treatment in a future test run, the stamp would stop changing — the test would time out at the not.toHaveText assertion. Fine for now since the fixture is dynamic by default.

[ask-bonk]

Review submitted. Here's the summary:

Approved. The one-line change at app-browser-entry.ts:1039 correctly removes the && navigationKind !== "refresh" exclusion from pending browser router state creation. This aligns refresh with push/replace — both are programmatic navigations dispatched inside React.startTransition with programmaticTransition=true, so they should all keep isPending true until the committed tree resolves.

The test fixture and Playwright assertion are well-structured and consistent with the existing push-pending coverage. Left two non-blocking nits about test timing margins and the Date.now() stamp's implicit dependency on dynamic rendering.

github run