fix(router): superseded push no longer follows a stale redirect; client redirect loops stop at 20 hops - #1058

Two loose ends from #1052 (issue #1047), both in packages/rangojs-router/src/browser/navigation-bridge.ts. A link click or router.push() that was superseded by a newer navigation could still follow its own server redirect and replace the page the user had moved on to; and two pages whose middleware redirect to each other made the client navigate back and forth forever, with no error and no end.

Problem

1. Click link A, then link B before A's response arrives. If A's response turns out to be a redirect (route middleware sending a signed-out visitor to /login), the router navigated to /login and replaced B. Back/Forward (handlePopstate, fixed in fix(router): follow a server redirect on Back/Forward to a fetched entry (#1047) #1052) already checked aborted/superseded first; the push path (navigate) checked ServerRedirect first.
2. /redirect-loop/ping redirects to /redirect-loop/pong and back. The client re-navigated forever. Browsers stop a document redirect chain at about 20 hops with an error.

Before vs. after

// router.push("/redirect-loop/ping"), ping and pong redirect to each other
// before: navigates ping -> pong -> ping -> ... without end, no error
// after:  after 20 followed redirects the root error boundary renders and the console shows
//         [rango] Server redirect loop: stopped after following 20 redirects, at <url>

// click A (response held), click B, A's response is a redirect to /login
// before: lands on /login, B discarded
// after:  B's result stands, A's redirect is dropped

Consumer example

export const urls = urls(({ path, middleware }) => [
  path("/ping", () => <div>ping</div>, { name: "ping" }, () => [
    middleware(() => redirect("/redirect-loop/pong", 302)),
  ]),
  path("/pong", () => <div>pong</div>, { name: "pong" }, () => [
    middleware(() => redirect("/redirect-loop/ping", 302)),
  ]),
]);

A misconfigured gate like this now surfaces as the error boundary (the dev boundary shows the message; production shows the generic title) instead of a tab that spins. No option changes the limit.

Semantics

Code change

navigation-bridge.ts: in navigate's catch, the ServerRedirect branch moved below the aborted/superseded return. Both paths now call a local followServerRedirect(self, error, url, hops, { replace, transition }) in createNavigationBridge, which validates the origin, stops at MAX_REDIRECT_HOPS = 20 (comment: mirrors browsers) by logging once and calling emitNavigationError with an Error naming the URL and hop count, and otherwise calls self.navigate(target, { state, replace, transition, _skipCache: true, _redirectHops: hops + 1 }). A user-initiated navigation has no _redirectHops and starts at 0; Back/Forward passes 0 and its re-navigation carries 1. types.ts: _redirectHops?: number on NavigateOptionsInternal (internal flag next to _skipCache, no public option). self is this so test spies on bridge.navigate still see the re-navigation. Net source change: +29 lines (61 added, 32 removed, most of it the moved branch).

Test plan

• Unit src/__tests__/navigation-bridge-traversal-location.test.ts (5 new, 12 total in file):
  • "a superseded push whose response is a redirect does not follow the redirect over the newer navigation": A held, B started, A rejects ServerRedirect; navigate called only for A and B. Red on origin/main: navigate.mock.calls is [a, b, "http://localhost/login"] (:352, expected [ 'http://localhost/a', …(2) ] to deeply equal [ 'http://localhost/a', …(1) ]).
  • "a push stops at the hop limit and renders the error boundary": two URLs redirecting to each other; 21 navigate calls, one error-boundary update, one redirect loop log naming 20 redirects. Red on origin/main: expected "navigate" to be called 21 times, but got 51 times (the test stops the loop itself past 50 calls so it cannot hang).
  • "a traversal stops at the hop limit the same way": red on origin/main: expected "navigate" to be called 20 times, but got 51 times.
  • "a new user navigation starts again at zero hops" (red on origin/main, 51 calls).
  • "a single redirect is followed with one hop counted".
  • Existing single-redirect coverage, still green: "follows the redirect as a replace ..." (traversal; expectation now includes _redirectHops: 1) the cross-origin block on the push path in navigation-bridge.test.ts:468, and e2e expectLinkClickToRedirectingPageFollowsRedirect (push path, dev and production, both apps). No unit test covered a followed push redirect before; the new "single redirect" test does.
• E2e body expectRedirectLoopEndsInErrorBoundary in tests/shared-e2e/src/index.ts, used by new redirect-loop.test.ts in both apps (packages/rangojs-router/e2e, tests/cloudflare-basic/e2e), dev describe and (production) describe: link click into the loop, error boundary visible within 30s, exactly one [rango] Server redirect loop ... 20 redirects console line. New fixture routes /redirect-loop, /redirect-loop/ping, /redirect-loop/pong in both apps; router.named-routes.gen.ts regenerated in both (the only *.gen.ts that changed).
• The first item is unit-only (a superseded response is a timing case).
• typecheck, test:unit:all, lint, format, check:e2e-bucketing, check:e2e-parity --strict, check:docs-api, check:bundle-guards: green. Client router chunk (cloudflare-basic) 47472 B gzip, limit 48128 B (47283 B on fix(router): follow a server redirect on Back/Forward to a fetched entry (#1047) #1052, +189 B).
• E2e on the final tree, --no-deps, dev and production: router app popstate-redirect, navigation-url-commit, hook-render-stability, auth-boundary, redirect-loop (51 passed); cloudflare-basic the same minus auth-boundary (26 passed). The known gap #1046 cases in navigation-url-commit print as expected failures in both modes and both apps.

Notes

• The e2e asserts the boundary with /Server redirect loop|Internal Server Error/: the default boundary shows the error message in dev and a generic title in production.

• The stop emits the boundary for the URL at hop 20, and the address bar stays on the page the chain started from (nothing committed).

• Not changed: the redirect target equal to the entry before the redirecting one leaves two identical adjacent entries (fix(router): follow a server redirect on Back/Forward to a fetched entry (#1047) #1052 note 3).

• Refs fix(router): follow a server redirect on Back/Forward to a fetched entry (#1047) #1052 (notes 1 and 2), Back/Forward to an entry the server now redirects is not followed: "Unprocessable popstate response" and an error boundary #1047.

• Review: read by the reviewer directly (reuse, simplification, efficiency, depth) given the size (about 30 net lines of source). The shared followServerRedirect replaces two copies of the same branch, which the fix(router): follow a server redirect on Back/Forward to a fetched entry (#1047) #1052 review had left as two sites; with the hop count to carry, one helper is now the simpler form. Second run on the final commit: gate green (router 7571 unit, 512 RSC); redirect-loop, popstate-redirect (and auth-boundary in the router app), --no-deps: router app 12 passed in dev and 11 in production, cloudflare-basic 3 and 3.