-
Notifications
You must be signed in to change notification settings - Fork 221
feat: make vinext fully compatible with clerk middleware #126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -214,7 +214,7 @@ import { | |
| import { createElement, Suspense, Fragment } from "react"; | ||
| import { setNavigationContext as _setNavigationContextOrig, getNavigationContext as _getNavigationContext } from "next/navigation"; | ||
| import { setHeadersContext, headersContextFromRequest, getDraftModeCookieHeader, getAndClearPendingCookies, consumeDynamicUsage, markDynamicUsage, runWithHeadersContext, applyMiddlewareRequestHeaders } from "next/headers"; | ||
| import { NextRequest } from "next/server"; | ||
| import { NextRequest, NextFetchEvent } from "next/server"; | ||
| import { ErrorBoundary, NotFoundBoundary } from "vinext/error-boundary"; | ||
| import { LayoutSegmentProvider } from "vinext/layout-segment-context"; | ||
| import { MetadataHead, mergeMetadata, resolveModuleMetadata, ViewportHead, mergeViewport, resolveModuleViewport } from "vinext/metadata"; | ||
|
|
@@ -1236,6 +1236,8 @@ function __applyConfigHeaders(pathname, ctx) { | |
| } | ||
|
|
||
| export default async function handler(request) { | ||
| const _waitUntilPromises = []; | ||
|
|
||
| // Wrap the entire request in nested AsyncLocalStorage.run() scopes to ensure | ||
| // per-request isolation for all state modules. Each runWith*() creates an | ||
| // ALS scope that propagates through all async continuations (including RSC | ||
|
|
@@ -1248,7 +1250,7 @@ export default async function handler(request) { | |
| _runWithPrivateCache(() => | ||
| runWithFetchCache(async () => { | ||
| const __reqCtx = __buildRequestContext(request); | ||
| const response = await _handleRequest(request, __reqCtx); | ||
| const response = await _handleRequest(request, __reqCtx, _waitUntilPromises); | ||
| // Apply custom headers from next.config.js to non-redirect responses. | ||
| // Skip redirects (3xx) because Response.redirect() creates immutable headers, | ||
| // and Next.js doesn't apply custom headers to redirects anyway. | ||
|
|
@@ -1262,6 +1264,12 @@ export default async function handler(request) { | |
| response.headers.set(h.key, h.value); | ||
| } | ||
| } | ||
|
|
||
| // Expose waitUntil promises to the runtime (like Cloudflare Workers) | ||
| if (response && _waitUntilPromises && _waitUntilPromises.length > 0) { | ||
| Object.defineProperty(response, "__vinextWaitUntil", { value: _waitUntilPromises, enumerable: false }); | ||
|
Comment on lines
1264
to
+1270
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This only runs for responses that flow through the normal |
||
| } | ||
|
|
||
| return response; | ||
| }) | ||
| ) | ||
|
|
@@ -1270,7 +1278,7 @@ export default async function handler(request) { | |
| ); | ||
| } | ||
|
|
||
| async function _handleRequest(request, __reqCtx) { | ||
| async function _handleRequest(request, __reqCtx, _outerWaitUntilPromises) { | ||
| const __reqStart = process.env.NODE_ENV !== "production" ? performance.now() : 0; | ||
| let __compileEnd; | ||
| let __renderEnd; | ||
|
|
@@ -1387,7 +1395,15 @@ async function _handleRequest(request, __reqCtx) { | |
| mwUrl.pathname = cleanPathname; | ||
| const mwRequest = new Request(mwUrl, request); | ||
| const nextRequest = mwRequest instanceof NextRequest ? mwRequest : new NextRequest(mwRequest); | ||
| const mwResponse = await middlewareFn(nextRequest); | ||
| const mwEvent = new NextFetchEvent({ page: cleanPathname }); | ||
| const mwResponse = await middlewareFn(nextRequest, mwEvent); | ||
|
|
||
| // If the middleware registered any waitUntil promises, we need to bubble them up | ||
| // to the runtime (like Cloudflare Workers) so they aren't cancelled. | ||
| if (mwEvent.waitUntilPromises && mwEvent.waitUntilPromises.length > 0) { | ||
| _outerWaitUntilPromises.push(...mwEvent.waitUntilPromises); | ||
| } | ||
|
Comment on lines
+1401
to
+1405
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: the |
||
|
|
||
| if (mwResponse) { | ||
| // Check for x-middleware-next (continue) | ||
| if (mwResponse.headers.get("x-middleware-next") === "1") { | ||
|
|
@@ -1405,6 +1421,11 @@ async function _handleRequest(request, __reqCtx) { | |
| } else { | ||
| // Check for redirect | ||
| if (mwResponse.status >= 300 && mwResponse.status < 400) { | ||
| // Attach waitUntil promises before early return so they reach ctx.waitUntil(). | ||
| // This is the most common Clerk auth pattern: redirect unauthenticated users. | ||
| if (_outerWaitUntilPromises.length > 0) { | ||
| Object.defineProperty(mwResponse, "__vinextWaitUntil", { value: _outerWaitUntilPromises, enumerable: false }); | ||
| } | ||
| return mwResponse; | ||
| } | ||
| // Check for rewrite | ||
|
Comment on lines
1421
to
1431
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good — this covers the most critical early-return path (Clerk auth redirect). The |
||
|
|
@@ -1424,7 +1445,10 @@ async function _handleRequest(request, __reqCtx) { | |
| } | ||
| } | ||
| } else { | ||
| // Middleware returned a custom response | ||
| // Middleware returned a custom response — attach waitUntil promises before early return | ||
| if (_outerWaitUntilPromises.length > 0) { | ||
| Object.defineProperty(mwResponse, "__vinextWaitUntil", { value: _outerWaitUntilPromises, enumerable: false }); | ||
| } | ||
| return mwResponse; | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,8 +15,13 @@ | |
| // @ts-expect-error — virtual module resolved by vinext | ||
| import rscHandler from "virtual:vinext-rsc-entry"; | ||
|
|
||
| interface ExecutionContext { | ||
| waitUntil(promise: Promise<any>): void; | ||
| passThroughOnException(): void; | ||
| } | ||
|
|
||
| export default { | ||
| async fetch(request: Request): Promise<Response> { | ||
| async fetch(request: Request, env: any, ctx: ExecutionContext): Promise<Response> { | ||
| const url = new URL(request.url); | ||
|
|
||
| // Normalize backslashes (browsers treat /\ as //) before any other checks. | ||
|
|
@@ -47,6 +52,14 @@ export default { | |
| // Delegate to RSC handler (which decodes + normalizes the pathname itself) | ||
| const result = await rscHandler(request); | ||
|
|
||
| // Extract waitUntil promises BEFORE the instanceof check — the property is | ||
| // non-enumerable on the Response and we need to hand it off to ctx before returning. | ||
| if (result && typeof result === "object" && "__vinextWaitUntil" in result && Array.isArray(result.__vinextWaitUntil)) { | ||
| for (const p of result.__vinextWaitUntil) { | ||
| ctx.waitUntil(p); | ||
| } | ||
| } | ||
|
Comment on lines
+55
to
+61
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The guard is thorough and correctly placed before the |
||
|
|
||
| if (result instanceof Response) { | ||
| return result; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -830,6 +830,13 @@ async function startPagesRouterServer(options: PagesRouterServerOptions) { | |
| // Apply custom status code from middleware rewrite | ||
| // (e.g. NextResponse.rewrite(url, { status: 403 })) | ||
| middlewareRewriteStatus = result.rewriteStatus; | ||
|
|
||
| // Await any background promises registered via event.waitUntil() during middleware. | ||
| // In prod-server (Node.js) there's no Workers ctx.waitUntil(), so we settle them | ||
| // here to avoid silently dropping work (e.g. Clerk session sync). | ||
| if (result.waitUntilPromises && result.waitUntilPromises.length > 0) { | ||
| Promise.allSettled(result.waitUntilPromises).catch(() => {}); | ||
| } | ||
|
Comment on lines
+834
to
+839
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: this This is the most common Clerk auth pattern — redirect unauthenticated users. Compare with Fix: move this block above the const result = await runMiddleware(webRequest);
// Settle waitUntil promises immediately — in Node.js there's no ctx.waitUntil().
if (result.waitUntilPromises?.length) {
Promise.allSettled(result.waitUntilPromises).catch(() => {});
}
if (!result.continue) {
// ... existing redirect/response handling |
||
| } | ||
|
|
||
| // Unpack x-middleware-request-* headers into the actual request so that | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -394,6 +394,10 @@ export class NextFetchEvent { | |
| waitUntil(promise: Promise<unknown>): void { | ||
| this._waitUntilPromises.push(promise); | ||
| } | ||
|
|
||
| get waitUntilPromises(): Promise<unknown>[] { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Exposing the private array directly means callers can mutate it (push, splice, etc.). This is fine for the current internal-only usage, but worth noting the tradeoff. If this ever becomes part of a public API surface, consider returning a copy ( |
||
| return this._waitUntilPromises; | ||
| } | ||
| } | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2308,7 +2308,7 @@ describe("App Router next.config.js features (generateRscEntry)", () => { | |
| expect(code).toContain("__safeDevHosts"); | ||
| // Should call dev origin validation inside _handleRequest | ||
| const callSite = code.indexOf("const __originBlock = __validateDevRequestOrigin(request)"); | ||
| const handleRequestIdx = code.indexOf("async function _handleRequest(request, __reqCtx)"); | ||
| const handleRequestIdx = code.indexOf("async function _handleRequest(request, __reqCtx, _outerWaitUntilPromises)"); | ||
| expect(callSite).toBeGreaterThan(-1); | ||
| expect(handleRequestIdx).toBeGreaterThan(-1); | ||
| // The call should be inside the function body (after the function declaration) | ||
|
|
@@ -2530,6 +2530,13 @@ describe("App Router middleware with NextRequest", () => { | |
| expect(key.startsWith("x-middleware-")).toBe(false); | ||
| } | ||
| }); | ||
|
|
||
| it("middleware receives event with waitUntil (for Clerk compat)", async () => { | ||
| const res = await fetch(`${baseUrl}/middleware-event`); | ||
| expect(res.status).toBe(200); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test verifies that Consider adding a unit test that calls |
||
| const text = await res.text(); | ||
| expect(text).toBe("Event OK"); | ||
| }); | ||
| }); | ||
|
|
||
| describe("SSR entry CSS preload fix", () => { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct placement — extracting
waitUntilPromisesbefore the!result.continuecheck ensures promises survive all middleware result paths (redirect, custom response, continue). This is the pattern thatprod-server.tsshould follow too.