Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/build/functions/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ const getHandlerFile = async (ctx: PluginContext): Promise<string> => {

const templateVariables: Record<string, string> = {
'{{useRegionalBlobs}}': ctx.useRegionalBlobs.toString(),
'{{excludeStaticPath}}': posixJoin(ctx.buildConfig.basePath || '', '/_next/static/*'),
}
// In this case it is a monorepo and we need to use a own template for it
// as we have to change the process working directory
Expand Down
5 changes: 5 additions & 0 deletions src/build/templates/handler-monorepo.tmpl.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,9 @@ export default async function (req, context) {
export const config = {
path: '/*',
preferStatic: true,
excludedPath: [
// We use `preferStatic: true` so we already won't run this on *existing* static assets,
// but by excluding this entire path we also avoid invoking the function just to 404.
'{{excludeStaticPath}}',
],
}
5 changes: 5 additions & 0 deletions src/build/templates/handler.tmpl.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,9 @@ export default async function handler(req, context) {
export const config = {
path: '/*',
preferStatic: true,
excludedPath: [
// We use `preferStatic: true` so we already won't run this on *existing* static assets,
// but by excluding this entire path we also avoid invoking the function just to 404.
'{{excludeStaticPath}}',
],
}
78 changes: 72 additions & 6 deletions tests/e2e/page-router.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { expect } from '@playwright/test'
import { nextVersionSatisfies } from '../utils/next-version-helpers.mjs'
import { test } from '../utils/playwright-helpers.js'
import { join } from 'node:path'
import { readdir } from 'node:fs/promises'

export function waitFor(millis: number) {
return new Promise((resolve) => setTimeout(resolve, millis))
Expand Down Expand Up @@ -614,6 +616,34 @@ test.describe('Simple Page Router (no basePath, no i18n)', () => {
/(s-maxage|max-age)/,
)
})

test.describe('static assets and function invocations', () => {
test('should return 200 for an existing static asset without invoking a function', async ({
page,
pageRouter,
}) => {
// Since assets are hashed, we can't hardcode anything here. Find something to fetch.
const [staticAsset] = await readdir(
join(pageRouter.isolatedFixtureRoot, '.next', 'static', 'chunks'),
)
expect(staticAsset).toBeDefined()

const response = await page.goto(`${pageRouter.url}/_next/static/chunks/${staticAsset}`)

expect(response?.status()).toBe(200)
expect(response?.headers()).not.toHaveProperty('x-nf-function-type')
})

test('should return 404 for a nonexistent static asset without invoking a function', async ({
page,
pageRouter,
}) => {
const response = await page.goto(`${pageRouter.url}/_next/static/stale123abcdef.js`)

expect(response?.status()).toBe(404)
expect(response?.headers()).not.toHaveProperty('x-nf-function-type')
})
})
})

test.describe('Page Router with basePath and i18n', () => {
Expand Down Expand Up @@ -1352,13 +1382,15 @@ test.describe('Page Router with basePath and i18n', () => {

test('requesting a non existing page route that needs to be fetched from the blob store like 404.html', async ({
page,
pageRouter,
pageRouterBasePathI18n,
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice catch!

}) => {
const response = await page.goto(new URL('non-existing', pageRouter.url).href)
const response = await page.goto(
new URL('base/path/non-existing', pageRouterBasePathI18n.url).href,
)
const headers = response?.headers() || {}
expect(response?.status()).toBe(404)

expect(await page.textContent('p')).toBe('Custom 404 page')
expect(await page.textContent('p')).toBe('Custom 404 page for locale: en')

// https://github.com/vercel/next.js/pull/69802 made changes to returned cache-control header,
// after that 404 pages would have `private` directive, before that it would not
Expand All @@ -1375,13 +1407,15 @@ test.describe('Page Router with basePath and i18n', () => {

test('requesting a non existing page route that needs to be fetched from the blob store like 404.html (notFound: true)', async ({
page,
pageRouter,
pageRouterBasePathI18n,
}) => {
const response = await page.goto(new URL('static/not-found', pageRouter.url).href)
const response = await page.goto(
new URL('base/path/static/not-found', pageRouterBasePathI18n.url).href,
)
const headers = response?.headers() || {}
expect(response?.status()).toBe(404)

expect(await page.textContent('p')).toBe('Custom 404 page')
expect(await page.textContent('p')).toBe('Custom 404 page for locale: en')

expect(headers['debug-netlify-cdn-cache-control']).toBe(
nextVersionSatisfies('>=15.0.0-canary.187')
Expand All @@ -1390,4 +1424,36 @@ test.describe('Page Router with basePath and i18n', () => {
)
expect(headers['cache-control']).toBe('public,max-age=0,must-revalidate')
})

test.describe('static assets and function invocations', () => {
test('should return 200 for an existing static asset without invoking a function', async ({
page,
pageRouterBasePathI18n,
}) => {
// Since assets are hashed, we can't hardcode anything here. Find something to fetch.
const [staticAsset] = await readdir(
join(pageRouterBasePathI18n.isolatedFixtureRoot, '.next', 'static', 'chunks'),
)
expect(staticAsset).toBeDefined()

const response = await page.goto(
`${pageRouterBasePathI18n.url}/base/path/_next/static/chunks/${staticAsset}`,
)

expect(response?.status()).toBe(200)
expect(response?.headers()).not.toHaveProperty('x-nf-function-type')
})

test('should return 404 for a nonexistent static asset without invoking a function', async ({
page,
pageRouterBasePathI18n,
}) => {
const response = await page.goto(
`${pageRouterBasePathI18n.url}/base/path/_next/static/stale123abcdef.js`,
)

expect(response?.status()).toBe(404)
expect(response?.headers()).not.toHaveProperty('x-nf-function-type')
})
})
})
3 changes: 3 additions & 0 deletions tests/utils/create-e2e-fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export interface DeployResult {
deployID: string
url: string
logs: string
isolatedFixtureRoot: string
}

type PackageManager = 'npm' | 'pnpm' | 'yarn' | 'bun' | 'berry'
Expand Down Expand Up @@ -92,6 +93,7 @@ export const createE2EFixture = async (fixture: string, config: E2EConfig = {})
cleanup: _cleanup,
deployID: result.deployID,
url: result.url,
isolatedFixtureRoot: result.isolatedFixtureRoot,
}
} catch (error) {
await _cleanup(true)
Expand Down Expand Up @@ -292,6 +294,7 @@ async function deploySite(
url: `https://${deployID}--${siteName}.netlify.app`,
deployID,
logs: output,
isolatedFixtureRoot,
}
}

Expand Down
2 changes: 1 addition & 1 deletion tests/utils/playwright-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ export const test = base.extend<
if (response.url().includes('/_next/static/')) {
expect(
response.headers()['cache-control'],
'_next/static assets should have immutable cache control',
`_next/static asset (${response.url()}) should have immutable cache control`,
).toContain('public,max-age=31536000,immutable')
}
})
Expand Down
Loading