[PR #743] [MERGED] refactor: post-PR #690 cleanup - shared test helper and ??= syntax #811

New issue

Closed

opened 2026-05-06 13:10:14 +02:00 by BreizhHardware · 0 comments

BreizhHardware commented

2026-05-06 13:10:14 +02:00

Owner

📋 Pull Request Information

Original PR: https://github.com/cloudflare/vinext/pull/743
Author: @Divkix
Created: 4/1/2026
Status: ✅ Merged
Merged: 4/2/2026
Merged by: @james-elicx

Base: main ← Head: cleanup/post-pr-690-cleanup-test-helper-an-syntax-nit

📝 Commits (1)

52b9afe refactor: extract shared waitForAppRouterHydration helper and apply ??= syntax

📊 Changes

21 files changed (+148 additions, -252 deletions)

View changed files

📝 packages/vinext/src/shims/navigation.ts (+20 -22)
📝 tests/e2e/app-router/form.spec.ts (+2 -5)
📝 tests/e2e/app-router/instrumentation-client.spec.ts (+10 -13)
📝 tests/e2e/app-router/layout-params.spec.ts (+3 -9)
📝 tests/e2e/app-router/navigation-flows.spec.ts (+7 -16)
📝 tests/e2e/app-router/navigation-regressions.spec.ts (+12 -18)
📝 tests/e2e/app-router/navigation.spec.ts (+8 -18)
📝 tests/e2e/app-router/nextjs-compat/actions-nav.spec.ts (+2 -8)
📝 tests/e2e/app-router/nextjs-compat/actions-revalidate.spec.ts (+3 -9)
📝 tests/e2e/app-router/nextjs-compat/dynamic.spec.ts (+6 -12)
📝 tests/e2e/app-router/nextjs-compat/error-nav.spec.ts (+3 -9)
📝 tests/e2e/app-router/nextjs-compat/external-redirect.spec.ts (+2 -8)
📝 tests/e2e/app-router/nextjs-compat/hooks.spec.ts (+14 -20)
📝 tests/e2e/app-router/nextjs-compat/metadata.spec.ts (+2 -8)
📝 tests/e2e/app-router/nextjs-compat/navigation.spec.ts (+8 -14)
📝 tests/e2e/app-router/nextjs-compat/prefetch.spec.ts (+7 -13)
📝 tests/e2e/app-router/nextjs-compat/rsc-basic.spec.ts (+4 -10)
📝 tests/e2e/app-router/nextjs-compat/search-params-key.spec.ts (+3 -9)
📝 tests/e2e/app-router/routes.spec.ts (+6 -15)
📝 tests/e2e/app-router/server-actions.spec.ts (+6 -15)

...and 1 more files

📄 Description

Addresses review comments from PR #690:

Extract waitForHydration helper (@james-elicx)
- Added `waitForAppRouterHydration()` to `tests/e2e/helpers.ts`
- Updated 19 App Router test files to use shared helper
- Removes ~100 lines of duplicated code across test suite
- Standardizes on `expect().toPass()` pattern for better error messages

Original comment here: https://github.com/cloudflare/vinext/pull/690#pullrequestreview-4047070113

Apply ??= syntax nit (@james-elicx)
- Replaced `if (!x) { x = {...} }` with `x ??= {...}` in navigation.ts
- Cleaner, idiomatic TypeScript for lazy initialization

Original comment here: https://github.com/cloudflare/vinext/pull/690#pullrequestreview-4047067498

Verification:

vp check: ✅ all pass (0 lint/type errors)
E2E subset (navigation-regressions.spec.ts): ✅ 11 passed

_{🔄 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/743 **Author:** [@Divkix](https://github.com/Divkix) **Created:** 4/1/2026 **Status:** ✅ Merged **Merged:** 4/2/2026 **Merged by:** [@james-elicx](https://github.com/james-elicx) **Base:** `main` ← **Head:** `cleanup/post-pr-690-cleanup-test-helper-an-syntax-nit` --- ### 📝 Commits (1) - [`52b9afe`](https://github.com/cloudflare/vinext/commit/52b9afe036d8d9f310642022f92de9f0559efdd0) refactor: extract shared waitForAppRouterHydration helper and apply ??= syntax ### 📊 Changes **21 files changed** (+148 additions, -252 deletions) <details> <summary>View changed files</summary> 📝 `packages/vinext/src/shims/navigation.ts` (+20 -22) 📝 `tests/e2e/app-router/form.spec.ts` (+2 -5) 📝 `tests/e2e/app-router/instrumentation-client.spec.ts` (+10 -13) 📝 `tests/e2e/app-router/layout-params.spec.ts` (+3 -9) 📝 `tests/e2e/app-router/navigation-flows.spec.ts` (+7 -16) 📝 `tests/e2e/app-router/navigation-regressions.spec.ts` (+12 -18) 📝 `tests/e2e/app-router/navigation.spec.ts` (+8 -18) 📝 `tests/e2e/app-router/nextjs-compat/actions-nav.spec.ts` (+2 -8) 📝 `tests/e2e/app-router/nextjs-compat/actions-revalidate.spec.ts` (+3 -9) 📝 `tests/e2e/app-router/nextjs-compat/dynamic.spec.ts` (+6 -12) 📝 `tests/e2e/app-router/nextjs-compat/error-nav.spec.ts` (+3 -9) 📝 `tests/e2e/app-router/nextjs-compat/external-redirect.spec.ts` (+2 -8) 📝 `tests/e2e/app-router/nextjs-compat/hooks.spec.ts` (+14 -20) 📝 `tests/e2e/app-router/nextjs-compat/metadata.spec.ts` (+2 -8) 📝 `tests/e2e/app-router/nextjs-compat/navigation.spec.ts` (+8 -14) 📝 `tests/e2e/app-router/nextjs-compat/prefetch.spec.ts` (+7 -13) 📝 `tests/e2e/app-router/nextjs-compat/rsc-basic.spec.ts` (+4 -10) 📝 `tests/e2e/app-router/nextjs-compat/search-params-key.spec.ts` (+3 -9) 📝 `tests/e2e/app-router/routes.spec.ts` (+6 -15) 📝 `tests/e2e/app-router/server-actions.spec.ts` (+6 -15) _...and 1 more files_ </details> ### 📄 Description Addresses review comments from PR #690: 1. **Extract waitForHydration helper** (@james-elicx) - Added \`waitForAppRouterHydration()\` to \`tests/e2e/helpers.ts\` - Updated 19 App Router test files to use shared helper - Removes ~100 lines of duplicated code across test suite - Standardizes on \`expect().toPass()\` pattern for better error messages Original comment here: https://github.com/cloudflare/vinext/pull/690#pullrequestreview-4047070113 2. **Apply ??= syntax nit** (@james-elicx) - Replaced \`if (!x) { x = {...} }\` with \`x ??= {...}\` in navigation.ts - Cleaner, idiomatic TypeScript for lazy initialization Original comment here: https://github.com/cloudflare/vinext/pull/690#pullrequestreview-4047067498 **Verification:** - vp check: ✅ all pass (0 lint/type errors) - E2E subset (navigation-regressions.spec.ts): ✅ 11 passed --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>