Skip to content
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

feat: support after() #2717

Merged
merged 10 commits into from
Dec 9, 2024
4 changes: 2 additions & 2 deletions .github/workflows/pre-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ jobs:
- name: Install Deno
uses: denoland/setup-deno@v1
with:
# Should match the `DENO_VERSION_RANGE` from https://github.com/netlify/edge-bundler/blob/e55f825bd985d3c92e21d1b765d71e70d5628fba/node/bridge.ts#L17
deno-version: v1.37.0
# Should match the `DENO_VERSION_RANGE` from https://github.com/netlify/build/blob/main/packages/edge-bundler/node/bridge.ts#L20
deno-version: v1.44.4
- name: Extract tag and version
id: extract
run: |-
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/release-please.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ jobs:
- name: Install Deno
uses: denoland/setup-deno@v1
with:
# Should match the `DENO_VERSION_RANGE` from https://github.com/netlify/edge-bundler/blob/e55f825bd985d3c92e21d1b765d71e70d5628fba/node/bridge.ts#L17
deno-version: v1.37.0
# Should match the `DENO_VERSION_RANGE` from https://github.com/netlify/build/blob/main/packages/edge-bundler/node/bridge.ts#L20
deno-version: v1.44.4
- name: Build
run: npm run build
if: ${{ steps.release.outputs.release_created }}
Expand Down
10 changes: 5 additions & 5 deletions .github/workflows/run-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ jobs:
- name: Install Deno
uses: denoland/setup-deno@v1
with:
# Should match the `DENO_VERSION_RANGE` from https://github.com/netlify/edge-bundler/blob/main/node/bridge.ts#L17
deno-version: v1.37.0
# Should match the `DENO_VERSION_RANGE` from https://github.com/netlify/build/blob/main/packages/edge-bundler/node/bridge.ts#L20
deno-version: v1.44.4
- name: 'Install dependencies'
run: npm ci
- name: 'Prepare Netlify CLI'
Expand Down Expand Up @@ -134,7 +134,7 @@ jobs:
uses: denoland/setup-deno@v1
with:
# Should match the `DENO_VERSION_RANGE` from https://github.com/netlify/edge-bundler/blob/e55f825bd985d3c92e21d1b765d71e70d5628fba/node/bridge.ts#L17
deno-version: v1.37.0
deno-version: v1.44.4
- name: 'Install dependencies'
run: npm ci
- name: 'Build'
Expand Down Expand Up @@ -198,8 +198,8 @@ jobs:
- name: Install Deno
uses: denoland/setup-deno@v1
with:
# Should match the `DENO_VERSION_RANGE` from https://github.com/netlify/edge-bundler/blob/main/node/bridge.ts#L17
deno-version: v1.37.0
# Should match the `DENO_VERSION_RANGE` from https://github.com/netlify/build/blob/main/packages/edge-bundler/node/bridge.ts#L20
deno-version: v1.44.4
- name: 'Install dependencies'
run: npm ci
- name: 'Build'
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/size-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ jobs:
- name: Install Deno
uses: denoland/setup-deno@v1
with:
# Should match the `DENO_VERSION_RANGE` from https://github.com/netlify/edge-bundler/blob/main/node/bridge.ts#L17
deno-version: v1.37.0
# Should match the `DENO_VERSION_RANGE` from https://github.com/netlify/build/blob/main/packages/edge-bundler/node/bridge.ts#L20
deno-version: v1.44.4
- run: npm ci

- name: Package size report
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/test-e2e.yml
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,8 @@ jobs:
- name: Install Deno
uses: denoland/setup-deno@v1
with:
# Should match the `DENO_VERSION_RANGE` from https://github.com/netlify/edge-bundler/blob/main/node/bridge.ts#L17
deno-version: v1.37.0
# Should match the `DENO_VERSION_RANGE` from https://github.com/netlify/build/blob/main/packages/edge-bundler/node/bridge.ts#L20
deno-version: v1.44.4

- name: install runtime
run: npm install --ignore-scripts
Expand Down
2 changes: 1 addition & 1 deletion src/build/templates/handler-monorepo.tmpl.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export default async function (req, context) {
tracing.start()
}

const requestContext = createRequestContext(req)
const requestContext = createRequestContext(req, context)
const tracer = getTracer()

const handlerResponse = await runWithRequestContext(requestContext, () => {
Expand Down
2 changes: 1 addition & 1 deletion src/build/templates/handler.tmpl.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export default async function handler(req, context) {
if (process.env.NETLIFY_OTLP_TRACE_EXPORTER_URL) {
tracing.start()
}
const requestContext = createRequestContext(req)
const requestContext = createRequestContext(req, context)
const tracer = getTracer()

const handlerResponse = await runWithRequestContext(requestContext, () => {
Expand Down
15 changes: 13 additions & 2 deletions src/run/handlers/request-context.cts
Original file line number Diff line number Diff line change
@@ -1,11 +1,18 @@
import { AsyncLocalStorage } from 'node:async_hooks'

import type { Context } from '@netlify/functions'
import { LogLevel, systemLogger } from '@netlify/functions/internal'

import type { NetlifyCachedRouteValue } from '../../shared/cache-types.cjs'

type SystemLogger = typeof systemLogger

// TODO: remove once https://github.com/netlify/serverless-functions-api/pull/219
// is released and public types are updated
export interface FutureContext extends Context {
waitUntil?: (promise: Promise<unknown>) => void
}

export type RequestContext = {
captureServerTiming: boolean
responseCacheGetLastModified?: number
Expand All @@ -28,13 +35,17 @@ export type RequestContext = {

type RequestContextAsyncLocalStorage = AsyncLocalStorage<RequestContext>

export function createRequestContext(request?: Request): RequestContext {
export function createRequestContext(request?: Request, context?: FutureContext): RequestContext {
const backgroundWorkPromises: Promise<unknown>[] = []

return {
captureServerTiming: request?.headers.has('x-next-debug-logging') ?? false,
trackBackgroundWork: (promise) => {
backgroundWorkPromises.push(promise)
if (context?.waitUntil) {
context.waitUntil(promise)
} else {
backgroundWorkPromises.push(promise)
}
},
get backgroundWorkPromise() {
return Promise.allSettled(backgroundWorkPromises)
Expand Down
30 changes: 13 additions & 17 deletions src/run/handlers/server.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import type { OutgoingHttpHeaders } from 'http'

import { ComputeJsOutgoingMessage, toComputeResponse, toReqRes } from '@fastly/http-compute-js'
import { Context } from '@netlify/functions'
import type { NextConfigComplete } from 'next/dist/server/config-shared.js'
import type { WorkerRequestHandler } from 'next/dist/server/lib/types.js'

Expand All @@ -16,9 +15,12 @@ import { nextResponseProxy } from '../revalidate.js'

import { createRequestContext, getLogger, getRequestContext } from './request-context.cjs'
import { getTracer } from './tracer.cjs'
import { setupWaitUntil } from './wait-until.cjs'

const nextImportPromise = import('../next.cjs')

setupWaitUntil()

let nextHandler: WorkerRequestHandler, nextConfig: NextConfigComplete

/**
Expand All @@ -44,13 +46,7 @@ const disableFaultyTransferEncodingHandling = (res: ComputeJsOutgoingMessage) =>
}
}

// TODO: remove once https://github.com/netlify/serverless-functions-api/pull/219
// is released and public types are updated
interface FutureContext extends Context {
waitUntil?: (promise: Promise<unknown>) => void
}

export default async (request: Request, context: FutureContext) => {
export default async (request: Request) => {
const tracer = getTracer()

if (!nextHandler) {
Expand Down Expand Up @@ -128,19 +124,19 @@ export default async (request: Request, context: FutureContext) => {
return new Response(body || null, response)
}

if (context.waitUntil) {
context.waitUntil(requestContext.backgroundWorkPromise)
}

const keepOpenUntilNextFullyRendered = new TransformStream({
async flush() {
// it's important to keep the stream open until the next handler has finished
await nextHandlerPromise
if (!context.waitUntil) {
// if waitUntil is not available, we have to keep response stream open until background promises are resolved
// to ensure that all background work executes
await requestContext.backgroundWorkPromise
}

// Next.js relies on `close` event emitted by response to trigger running callback variant of `next/after`
// however @fastly/http-compute-js never actually emits that event - so we have to emit it ourselves,
// otherwise Next would never run the callback variant of `next/after`
res.emit('close')
Comment on lines +132 to +135
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to support callback variant of after():

after(async () => {
  // this will not start executing immediately - instead this function will be stored to and start executing after response was ~sent
  // make sense to not compete for any resources while working on response and delay executing this function to after that
})

This is not needed to support promise variant like so:

after(new Promise(resolve => {
  // this starts executing immediately and will ensure it will finish as long 
  // as function timeout is not reached
})


// if waitUntil is not available, we have to keep response stream open until background promises are resolved
// to ensure that all background work executes
await requestContext.backgroundWorkPromise
},
})

Expand Down
26 changes: 26 additions & 0 deletions src/run/handlers/wait-until.cts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { getRequestContext } from './request-context.cjs'

/**
* @see https://github.com/vercel/next.js/blob/canary/packages/next/src/server/after/builtin-request-context.ts
*/
const NEXT_REQUEST_CONTEXT_SYMBOL = Symbol.for('@next/request-context')

export type NextJsRequestContext = {
get(): { waitUntil?: (promise: Promise<unknown>) => void } | undefined
}

type GlobalThisWithRequestContext = typeof globalThis & {
[NEXT_REQUEST_CONTEXT_SYMBOL]?: NextJsRequestContext
}

/**
* Registers a `waitUntil` to be used by Next.js for next/after
*/
export function setupWaitUntil() {
// eslint-disable-next-line @typescript-eslint/no-extra-semi
;(globalThis as GlobalThisWithRequestContext)[NEXT_REQUEST_CONTEXT_SYMBOL] = {
get() {
return { waitUntil: getRequestContext()?.trackBackgroundWork }
},
}
}
39 changes: 39 additions & 0 deletions tests/e2e/simple-app.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,3 +272,42 @@ test('can require CJS module that is not bundled', async ({ simple }) => {
expect(parsedBody.notBundledCJSModule.isBundled).toEqual(false)
expect(parsedBody.bundledCJSModule.isBundled).toEqual(true)
})

test('next/after callback is executed and finishes', async ({ page, simple }) => {
test.skip(!nextVersionSatisfies('>=15.0.0'), 'This test is only for Next.js 15+')

// trigger initial request to check page which might be stale and allow regenerating in background
await page.goto(`${simple.url}/after/check`)

await new Promise((resolve) => setTimeout(resolve, 5000))

// after it was possibly regenerated we can start checking actual content of the page
await page.goto(`${simple.url}/after/check`)
const pageInfoLocator1 = await page.locator('#page-info')
const pageInfo1 = JSON.parse((await pageInfoLocator1.textContent()) ?? '{}')

expect(typeof pageInfo1?.timestamp, 'Check page should have timestamp').toBe('number')

await page.goto(`${simple.url}/after/check`)
const pageInfoLocator2 = await page.locator('#page-info')
const pageInfo2 = JSON.parse((await pageInfoLocator2.textContent()) ?? '{}')

expect(typeof pageInfo2?.timestamp, 'Check page should have timestamp').toBe('number')

expect(pageInfo2.timestamp, 'Check page should be cached').toBe(pageInfo1.timestamp)

await page.goto(`${simple.url}/after/trigger`)

// wait for next/after to trigger revalidation of check page
await new Promise((resolve) => setTimeout(resolve, 5000))

await page.goto(`${simple.url}/after/check`)
const pageInfoLocator3 = await page.locator('#page-info')
const pageInfo3 = JSON.parse((await pageInfoLocator3.textContent()) ?? '{}')

expect(typeof pageInfo3?.timestamp, 'Check page should have timestamp').toBe('number')
expect(
pageInfo3.timestamp,
'Check page should be invalidated with newer timestamp',
).toBeGreaterThan(pageInfo1.timestamp)
})
10 changes: 10 additions & 0 deletions tests/fixtures/simple/app/after/check/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
export const revalidate = 3600 // arbitrarily long, just so that it doesn't happen during a test run

export default async function Page() {
const data = {
timestamp: Date.now(),
}
console.log('/timestamp/key/[key] rendered', data)

return <div id="page-info">{JSON.stringify(data)}</div>
}
17 changes: 17 additions & 0 deletions tests/fixtures/simple/app/after/trigger/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { revalidatePath } from 'next/cache'
import { unstable_after as after, connection } from 'next/server'

export default async function Page() {
await connection()
after(async () => {
// this will run after response was sent
console.log('after() triggered')
console.log('after() sleep 1s')
await new Promise((resolve) => setTimeout(resolve, 1000))

console.log('after() revalidatePath /after/check')
revalidatePath('/after/check')
})

return <div>Page with after()</div>
}
1 change: 1 addition & 0 deletions tests/fixtures/simple/next.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const nextConfig = {
outputFileTracingIncludes: {
'/': ['public/**'],
},
after: true,
},
images: {
remotePatterns: [
Expand Down
1 change: 1 addition & 0 deletions tests/integration/simple-app.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ test<FixtureTestContext>('Test that the simple next app is working', async (ctx)
const blobEntries = await getBlobEntries(ctx)
expect(blobEntries.map(({ key }) => decodeBlobKey(key)).sort()).toEqual([
'/404',
'/after/check',
'/api/cached-permanent',
'/api/cached-revalidate',
'/image/local',
Expand Down
10 changes: 7 additions & 3 deletions tests/smoke/deploy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,13 @@ describe('version check', () => {
)
},
)
test('yarn monorepo multiple next versions site is compatible', { retry: 0 }, async () => {
await smokeTest(selfCleaningFixtureFactories.yarnMonorepoMultipleNextVersionsSiteCompatible)
})
test(
'yarn monorepo multiple next versions site is compatible',
{ retry: 0, timeout: 1_000 * 60 * 5 },
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this change just bumps/sets timeout - we have 3 extensions being installed which we don't even use because site is in Netlify-testing account and that's pretty slow and this test case in particular was already slow before, but those 2 things combined meant that it was always timing out now

async () => {
await smokeTest(selfCleaningFixtureFactories.yarnMonorepoMultipleNextVersionsSiteCompatible)
},
)

test(
'yarn monorepo multiple next versions site is incompatible should not deploy',
Expand Down
Loading