[PR #340] [MERGED] fix: pass NextFetchEvent as second argument to middleware #493

New issue

Closed

opened 2026-05-06 13:08:22 +02:00 by BreizhHardware · 0 comments

BreizhHardware commented

2026-05-06 13:08:22 +02:00

Owner

📋 Pull Request Information

Original PR: https://github.com/cloudflare/vinext/pull/340
Author: @NathanDrake2406
Created: 3/8/2026
Status: ✅ Merged
Merged: 3/8/2026
Merged by: @james-elicx

Base: main ← Head: fix/middleware-next-fetch-event

📝 Commits (5)

5ddb6c5 fix: pass NextFetchEvent as second argument to middleware
eb7782b address review: add drainWaitUntil(), wire ctx.waitUntil() in Workers entry, fix async test
22a77cc fix deploy tests: update assertions for ctx-aware fetch and runMiddleware signatures
90f6013 address review: fix misleading comment, update JSDoc, use queueMicrotask in test
2797ccc pass undefined explicitly to runMiddleware in prod-server (no ctx on Node.js)

📊 Changes

8 files changed (+96 additions, -14 deletions)

View changed files

📝 packages/vinext/src/deploy.ts (+7 -2)
📝 packages/vinext/src/index.ts (+5 -3)
📝 packages/vinext/src/server/app-dev-server.ts (+4 -2)
📝 packages/vinext/src/server/middleware.ts (+7 -2)
📝 packages/vinext/src/server/prod-server.ts (+2 -2)
📝 packages/vinext/src/shims/server.ts (+5 -0)
📝 tests/deploy.test.ts (+3 -3)
📝 tests/shims.test.ts (+63 -0)

📄 Description

Summary

All 3 middleware invocation sites called middlewareFn(nextRequest) with only one argument, but NextMiddleware expects (request, event). Any middleware using event.waitUntil() threw at runtime because event was undefined.
Instantiate NextFetchEvent({ page }) at each call site, pass it as the second argument, and drain _waitUntilPromises via Promise.allSettled() (fire-and-forget) after middleware returns.
Made _waitUntilPromises non-private so it can be drained after middleware execution.

Call sites fixed:

packages/vinext/src/server/middleware.ts — shared runner (used by prod-server)
packages/vinext/src/index.ts — Pages Router dev codegen
packages/vinext/src/server/app-dev-server.ts — App Router dev codegen

Test plan

Added 2 new tests in tests/shims.test.ts (NextFetchEvent passed to middleware describe block):
- runMiddleware passes NextFetchEvent as second argument — verifies event is defined, waitUntil is callable, and sourcePage is set
- waitUntil promises are drained after middleware runs — verifies side effects from waitUntil promises actually execute
All 570 shims tests pass
All 206 app-router tests pass
All 127 pages-router tests pass
All 231 features tests pass
Typecheck clean
Lint clean

_{🔄 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/340 **Author:** [@NathanDrake2406](https://github.com/NathanDrake2406) **Created:** 3/8/2026 **Status:** ✅ Merged **Merged:** 3/8/2026 **Merged by:** [@james-elicx](https://github.com/james-elicx) **Base:** `main` ← **Head:** `fix/middleware-next-fetch-event` --- ### 📝 Commits (5) - [`5ddb6c5`](https://github.com/cloudflare/vinext/commit/5ddb6c521f58e554ddec08bee98bd25879ac930c) fix: pass NextFetchEvent as second argument to middleware - [`eb7782b`](https://github.com/cloudflare/vinext/commit/eb7782b20af51b8c8a1031c9ae0438fa24d9667b) address review: add drainWaitUntil(), wire ctx.waitUntil() in Workers entry, fix async test - [`22a77cc`](https://github.com/cloudflare/vinext/commit/22a77cc95fa9d1a3d11ea7fba4a0c40b70a0da22) fix deploy tests: update assertions for ctx-aware fetch and runMiddleware signatures - [`90f6013`](https://github.com/cloudflare/vinext/commit/90f601305ad8cc97a4c22abb2d9f36096bd11e62) address review: fix misleading comment, update JSDoc, use queueMicrotask in test - [`2797ccc`](https://github.com/cloudflare/vinext/commit/2797ccc0f9e21dd894996dfcdb350594f8c87c13) pass undefined explicitly to runMiddleware in prod-server (no ctx on Node.js) ### 📊 Changes **8 files changed** (+96 additions, -14 deletions) <details> <summary>View changed files</summary> 📝 `packages/vinext/src/deploy.ts` (+7 -2) 📝 `packages/vinext/src/index.ts` (+5 -3) 📝 `packages/vinext/src/server/app-dev-server.ts` (+4 -2) 📝 `packages/vinext/src/server/middleware.ts` (+7 -2) 📝 `packages/vinext/src/server/prod-server.ts` (+2 -2) 📝 `packages/vinext/src/shims/server.ts` (+5 -0) 📝 `tests/deploy.test.ts` (+3 -3) 📝 `tests/shims.test.ts` (+63 -0) </details> ### 📄 Description ## Summary - All 3 middleware invocation sites called `middlewareFn(nextRequest)` with only one argument, but `NextMiddleware` expects `(request, event)`. Any middleware using `event.waitUntil()` threw at runtime because `event` was `undefined`. - Instantiate `NextFetchEvent({ page })` at each call site, pass it as the second argument, and drain `_waitUntilPromises` via `Promise.allSettled()` (fire-and-forget) after middleware returns. - Made `_waitUntilPromises` non-private so it can be drained after middleware execution. **Call sites fixed:** - `packages/vinext/src/server/middleware.ts` — shared runner (used by prod-server) - `packages/vinext/src/index.ts` — Pages Router dev codegen - `packages/vinext/src/server/app-dev-server.ts` — App Router dev codegen ## Test plan - [x] Added 2 new tests in `tests/shims.test.ts` (`NextFetchEvent passed to middleware` describe block): - `runMiddleware passes NextFetchEvent as second argument` — verifies event is defined, `waitUntil` is callable, and `sourcePage` is set - `waitUntil promises are drained after middleware runs` — verifies side effects from `waitUntil` promises actually execute - [x] All 570 shims tests pass - [x] All 206 app-router tests pass - [x] All 127 pages-router tests pass - [x] All 231 features tests pass - [x] Typecheck clean - [x] Lint clean --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>