[PR #1053] [MERGED] test(app-router): lock navigation lifecycle settlement and root-layout hard navigation behaviour #1052

New issue

Closed

opened 2026-05-06 13:11:45 +02:00 by BreizhHardware · 0 comments

BreizhHardware commented

2026-05-06 13:11:45 +02:00

Owner

📋 Pull Request Information

Original PR: https://github.com/cloudflare/vinext/pull/1053
Author: @NathanDrake2406
Created: 5/4/2026
Status: ✅ Merged
Merged: 5/5/2026
Merged by: @james-elicx

Base: main ← Head: test/lock-lifecycle-settlement

📝 Commits (6)

10a9406 test(app-router): lock navigation lifecycle settlement and root-layout hard navigation behaviour
b7a516e test: remove hallucinatory comments from app-browser-entry tests
982ccde test: remove duplicate shouldHardNavigate null-root assertion
f37ec9e test: remove low-signal helper truth-table tests, fix hard-navigate ordering assertion
a8c3703 Merge remote-tracking branch 'origin/main' into pr-1053
986f5f9 fix(test): add missing operationLane field to new lifecycle settlement tests

📊 Changes

1 file changed (+336 additions, -0 deletions)

View changed files

📝 tests/app-browser-entry.test.ts (+336 -0)

📄 Description

Summary

Adds 6 unit tests locking two Wave 0 baseline areas from the #726 router kernel architecture — in preparation for the Layer 1 lifecycle spine migration.

Lifecycle settlement (4 tests)

The App Router uses a completely different lifecycle mechanism from the Pages Router (activeNavigationId counter + promise-based commit gate vs AbortController-based cancellation). The Pages Router has 10 proven race-condition tests in shims.test.ts. The App Router had zero equivalent unit tests for these controller-level concurrency semantics.

Test	Hostile timeline verified
Three navigations, payloads resolve in reverse order	A→B→C, C wins; B and A skipped before commit-effect creation
Stale cross-root is skipped, not hard-navigated	Cross-root A superseded by same-root B; no `window.location.assign`
`resolveAndClassifyNavigationCommit` skip classification	startedNavId ≠ activeNavId → skip through the await-payload helper path
Failed payload cleanly settles pending router state	Rejected promise → settled pending, error propagated

Test	Behaviour locked
`renderNavigationPayload` calls `window.location.assign` on cross-root	Full integrated dispatch→hard-navigate path
Hard-navigate settles pending router state before navigating away	Pending promise resolved before `window.location.assign` fires (asserted via `assign.mockImplementation`)

Why this matters for #726

The architecture's Implementation Plan says Layer 0 is "freeze today's flat-wire behavior with compatibility tests." These tests are the lifecycle-settlement and root-layout halves of that layer. Without them, the "delete the old writer" steps in Layers 4–5 have no regression safety net for the concurrency semantics that are hardest to verify by code review alone.

Design decisions

No real timers. Every test uses deferred Promises for payload resolution control. No setTimeout, no vi.advanceTimersByTime. Zero flake surface.
No awaiting renderNavigationPayload returns. The returned promise resolves only when NavigationCommitSignal mounts in a React tree, which never happens in unit tests. Tests verify state through stateRef.current after yielding microticks — same pattern as the existing "uses render ids independent from navigation ids" test.
Behaviour contracts, not implementation internals. Tests assert on stateRef.current.routeId (user-visible state), window.location.assign calls (browser effect), and createNavigationCommitEffect invocation (side-effect gating). They don't assert on internal Map sizes or reducer shape.
Follows existing conventions. Same createControllerHarness, createState, createResolvedElements, stubWindow helpers. Same vi.spyOn/mockRestore/detach() cleanup pattern. Sits in the same describe/file as the existing controller tests.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/cloudflare/vinext/pull/1053 **Author:** [@NathanDrake2406](https://github.com/NathanDrake2406) **Created:** 5/4/2026 **Status:** ✅ Merged **Merged:** 5/5/2026 **Merged by:** [@james-elicx](https://github.com/james-elicx) **Base:** `main` ← **Head:** `test/lock-lifecycle-settlement` --- ### 📝 Commits (6) - [`10a9406`](https://github.com/cloudflare/vinext/commit/10a9406ded4692c34ac043c656e1d5e5365f9042) test(app-router): lock navigation lifecycle settlement and root-layout hard navigation behaviour - [`b7a516e`](https://github.com/cloudflare/vinext/commit/b7a516eb60f905d03e9d048e7fc3133b39a9deb4) test: remove hallucinatory comments from app-browser-entry tests - [`982ccde`](https://github.com/cloudflare/vinext/commit/982ccde3b359da7787536409abc65e5fe8e589a3) test: remove duplicate shouldHardNavigate null-root assertion - [`f37ec9e`](https://github.com/cloudflare/vinext/commit/f37ec9eea0426c4fcc1087909b1be232ee8b9990) test: remove low-signal helper truth-table tests, fix hard-navigate ordering assertion - [`a8c3703`](https://github.com/cloudflare/vinext/commit/a8c370396eb7e7e4629f9d265fab97798c419b00) Merge remote-tracking branch 'origin/main' into pr-1053 - [`986f5f9`](https://github.com/cloudflare/vinext/commit/986f5f9e0e854e9e4d1c2bd6a5192eddd6fffc57) fix(test): add missing operationLane field to new lifecycle settlement tests ### 📊 Changes **1 file changed** (+336 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `tests/app-browser-entry.test.ts` (+336 -0) </details> ### 📄 Description ## Summary Adds 6 unit tests locking two Wave 0 baseline areas from the [#726 router kernel architecture](https://github.com/cloudflare/vinext/issues/726) — in preparation for the Layer 1 lifecycle spine migration. ### Lifecycle settlement (4 tests) The App Router uses a completely different lifecycle mechanism from the Pages Router (`activeNavigationId` counter + promise-based commit gate vs `AbortController`-based cancellation). The Pages Router has 10 proven race-condition tests in `shims.test.ts`. The App Router had zero equivalent unit tests for these controller-level concurrency semantics. | Test | Hostile timeline verified | |------|--------------------------| | Three navigations, payloads resolve in reverse order | A→B→C, C wins; B and A skipped before commit-effect creation | | Stale cross-root is skipped, not hard-navigated | Cross-root A superseded by same-root B; no `window.location.assign` | | `resolveAndClassifyNavigationCommit` skip classification | startedNavId ≠ activeNavId → skip through the await-payload helper path | | Failed payload cleanly settles pending router state | Rejected promise → settled pending, error propagated | ### Root-layout hard navigation (2 tests) | Test | Behaviour locked | |------|-----------------| | `renderNavigationPayload` calls `window.location.assign` on cross-root | Full integrated dispatch→hard-navigate path | | Hard-navigate settles pending router state before navigating away | Pending promise resolved before `window.location.assign` fires (asserted via `assign.mockImplementation`) | ### Why this matters for #726 The architecture's Implementation Plan says Layer 0 is "freeze today's flat-wire behavior with compatibility tests." These tests are the lifecycle-settlement and root-layout halves of that layer. Without them, the "delete the old writer" steps in Layers 4–5 have no regression safety net for the concurrency semantics that are hardest to verify by code review alone. ### Design decisions - **No real timers.** Every test uses deferred Promises for payload resolution control. No `setTimeout`, no `vi.advanceTimersByTime`. Zero flake surface. - **No awaiting `renderNavigationPayload` returns.** The returned promise resolves only when `NavigationCommitSignal` mounts in a React tree, which never happens in unit tests. Tests verify state through `stateRef.current` after yielding microticks — same pattern as the existing "uses render ids independent from navigation ids" test. - **Behaviour contracts, not implementation internals.** Tests assert on `stateRef.current.routeId` (user-visible state), `window.location.assign` calls (browser effect), and `createNavigationCommitEffect` invocation (side-effect gating). They don't assert on internal Map sizes or reducer shape. - **Follows existing conventions.** Same `createControllerHarness`, `createState`, `createResolvedElements`, `stubWindow` helpers. Same `vi.spyOn`/`mockRestore`/`detach()` cleanup pattern. Sits in the same describe/file as the existing controller tests. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>