diff --git a/src/run/augment-next-response.ts b/src/run/augment-next-response.ts new file mode 100644 index 0000000000..02e01db89a --- /dev/null +++ b/src/run/augment-next-response.ts @@ -0,0 +1,66 @@ +import type { ServerResponse } from 'node:http' +import { isPromise } from 'node:util/types' + +import type { NextApiResponse } from 'next' + +import type { RequestContext } from './handlers/request-context.cjs' + +type ResRevalidateMethod = NextApiResponse['revalidate'] + +function isRevalidateMethod( + key: string, + nextResponseField: unknown, +): nextResponseField is ResRevalidateMethod { + return key === 'revalidate' && typeof nextResponseField === 'function' +} + +function isAppendHeaderMethod( + key: string, + nextResponseField: unknown, +): nextResponseField is ServerResponse['appendHeader'] { + return key === 'appendHeader' && typeof nextResponseField === 'function' +} + +// Needing to proxy the response object to intercept: +// - the revalidate call for on-demand revalidation on page routes +// - prevent .appendHeader calls for location header to add duplicate values +export const augmentNextResponse = (response: ServerResponse, requestContext: RequestContext) => { + return new Proxy(response, { + get(target: ServerResponse, key: string) { + const originalValue = Reflect.get(target, key) + if (isRevalidateMethod(key, originalValue)) { + return function newRevalidate(...args: Parameters) { + requestContext.didPagesRouterOnDemandRevalidate = true + + const result = originalValue.apply(target, args) + if (result && isPromise(result)) { + requestContext.trackBackgroundWork(result) + } + + return result + } + } + + if (isAppendHeaderMethod(key, originalValue)) { + return function patchedAppendHeader(...args: Parameters) { + if (typeof args[0] === 'string' && (args[0] === 'location' || args[0] === 'Location')) { + let existing = target.getHeader('location') + if (typeof existing !== 'undefined') { + if (!Array.isArray(existing)) { + existing = [String(existing)] + } + if (existing.includes(String(args[1]))) { + // if we already have that location header - bail early + // appendHeader should return the target for chaining + return target + } + } + } + + return originalValue.apply(target, args) + } + } + return originalValue + }, + }) +} diff --git a/src/run/handlers/server.ts b/src/run/handlers/server.ts index 0a51163bb4..af509e8344 100644 --- a/src/run/handlers/server.ts +++ b/src/run/handlers/server.ts @@ -5,6 +5,7 @@ import type { Context } from '@netlify/functions' import type { Span } from '@netlify/otel/opentelemetry' import type { WorkerRequestHandler } from 'next/dist/server/lib/types.js' +import { augmentNextResponse } from '../augment-next-response.js' import { getRunConfig, setRunConfig } from '../config.js' import { adjustDateHeader, @@ -13,7 +14,6 @@ import { setCacheTagsHeaders, setVaryHeaders, } from '../headers.js' -import { nextResponseProxy } from '../revalidate.js' import { setFetchBeforeNextPatchedIt } from '../storage/storage.cjs' import { getLogger, type RequestContext } from './request-context.cjs' @@ -97,7 +97,7 @@ export default async ( disableFaultyTransferEncodingHandling(res as unknown as ComputeJsOutgoingMessage) - const resProxy = nextResponseProxy(res, requestContext) + const resProxy = augmentNextResponse(res, requestContext) // We don't await this here, because it won't resolve until the response is finished. const nextHandlerPromise = nextHandler(req, resProxy).catch((error) => { diff --git a/src/run/revalidate.ts b/src/run/revalidate.ts deleted file mode 100644 index 6789072209..0000000000 --- a/src/run/revalidate.ts +++ /dev/null @@ -1,37 +0,0 @@ -import type { ServerResponse } from 'node:http' -import { isPromise } from 'node:util/types' - -import type { NextApiResponse } from 'next' - -import type { RequestContext } from './handlers/request-context.cjs' - -type ResRevalidateMethod = NextApiResponse['revalidate'] - -function isRevalidateMethod( - key: string, - nextResponseField: unknown, -): nextResponseField is ResRevalidateMethod { - return key === 'revalidate' && typeof nextResponseField === 'function' -} - -// Needing to proxy the response object to intercept the revalidate call for on-demand revalidation on page routes -export const nextResponseProxy = (response: ServerResponse, requestContext: RequestContext) => { - return new Proxy(response, { - get(target: ServerResponse, key: string) { - const originalValue = Reflect.get(target, key) - if (isRevalidateMethod(key, originalValue)) { - return function newRevalidate(...args: Parameters) { - requestContext.didPagesRouterOnDemandRevalidate = true - - const result = originalValue.apply(target, args) - if (result && isPromise(result)) { - requestContext.trackBackgroundWork(result) - } - - return result - } - } - return originalValue - }, - }) -} diff --git a/tests/fixtures/simple/app/app-redirect/[slug]/page.js b/tests/fixtures/simple/app/app-redirect/[slug]/page.js new file mode 100644 index 0000000000..bcf594ec7d --- /dev/null +++ b/tests/fixtures/simple/app/app-redirect/[slug]/page.js @@ -0,0 +1,11 @@ +import { redirect } from 'next/navigation' + +const Page = async () => { + return redirect(`/app-redirect/dest`) +} + +export const generateStaticParams = async () => { + return [{ slug: 'prerendered' }] +} + +export default Page diff --git a/tests/fixtures/simple/app/app-redirect/dest/page.js b/tests/fixtures/simple/app/app-redirect/dest/page.js new file mode 100644 index 0000000000..243ce2c44d --- /dev/null +++ b/tests/fixtures/simple/app/app-redirect/dest/page.js @@ -0,0 +1,9 @@ +export default function Page() { + return ( +
+

Hello next/navigation#redirect target

+
+ ) +} + +export const dynamic = 'force-static' diff --git a/tests/integration/simple-app.test.ts b/tests/integration/simple-app.test.ts index 8e77b3d2a8..3b37d3b6cc 100644 --- a/tests/integration/simple-app.test.ts +++ b/tests/integration/simple-app.test.ts @@ -108,6 +108,8 @@ test('Test that the simple next app is working', async (ctx) shouldHaveAppRouterNotFoundInPrerenderManifest() ? '/_not-found' : undefined, '/api/cached-permanent', '/api/cached-revalidate', + '/app-redirect/dest', + '/app-redirect/prerendered', '/config-redirect', '/config-redirect/dest', '/config-rewrite', @@ -445,6 +447,31 @@ test.skipIf(hasDefaultTurbopackBuilds())( }, ) +// below version check is not exact (not located exactly when, but Next.js had a bug for prerender pages that should redirect would not work correctly) +// currently only know that 13.5.1 and 14.2.35 doesn't have correct response (either not a 307 or completely missing 'location' header), while 15.5.9 has 307 response with location header +test.skipIf(nextVersionSatisfies('<15.0.0'))( + `app router page that uses next/navigation#redirect works when page is prerendered`, + async (ctx) => { + await createFixture('simple', ctx) + await runPlugin(ctx) + + const response = await invokeFunction(ctx, { url: `/app-redirect/prerendered` }) + + expect(response.statusCode).toBe(307) + expect(response.headers['location']).toBe('/app-redirect/dest') + }, +) + +test(`app router page that uses next/navigation#redirect works when page is NOT prerendered`, async (ctx) => { + await createFixture('simple', ctx) + await runPlugin(ctx) + + const response = await invokeFunction(ctx, { url: `/app-redirect/non-prerendered` }) + + expect(response.statusCode).toBe(307) + expect(response.headers['location']).toBe('/app-redirect/dest') +}) + describe('next patching', async () => { const { cp: originalCp, appendFile } = (await vi.importActual( 'node:fs/promises',