Skip to content

fix: adjust cache-control handling for [email protected] #2666

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Oct 16, 2024
Merged

fix: adjust cache-control handling for [email protected] #2666

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4f5f563
fix: drop empty directives to not produce invalid header values
pieh Oct 14, 2024
93dd2b6
test: adjust tests for [email protected]
pieh Oct 14, 2024
de41cb3
Merge branch 'main' into michalpiechowiak/frb-1385-next1500-canary187…
pieh Oct 15, 2024
9a98fe0
Merge branch 'main' into michalpiechowiak/frb-1385-next1500-canary187…
pieh Oct 16, 2024
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
5 changes: 4 additions & 1 deletion src/run/headers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,10 @@ const generateNetlifyVaryValues = ({
}

const getHeaderValueArray = (header: string): string[] => {
return header.split(',').map((value) => value.trim())
return header
.split(',')
.map((value) => value.trim())
.filter(Boolean)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/vercel/next.js/pull/71159/files#diff-ae41ee59d88093c9c0551e12f654b1b99e4211b1c9fa6dc1471febb0361004e7R21 this can now produce invalid value like s-maxage=31536000, so we remove ~falsy/empty directives, so at least on our end we produce valid value.

Hopefully they fix their stuff in which this added .filter would do nothing (so it also won't hurt to have it always)

}

const omitHeaderValues = (header: string, values: string[]): string => {
Expand Down
17 changes: 13 additions & 4 deletions tests/e2e/on-demand-app.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { expect } from '@playwright/test'
import { test } from '../utils/playwright-helpers.js'
import { nextVersionSatisfies } from '../utils/next-version-helpers.mjs'

test.describe('app router on-demand revalidation', () => {
for (const { label, prerendered, pagePath, revalidateApiPath, expectedH1Content } of [
Expand Down Expand Up @@ -90,7 +91,9 @@ test.describe('app router on-demand revalidation', () => {
expect(response1?.status()).toBe(200)
expect(headers1['x-nextjs-cache']).toBeUndefined()
expect(headers1['netlify-cdn-cache-control']).toBe(
's-maxage=31536000, stale-while-revalidate=31536000, durable',
nextVersionSatisfies('>=15.0.0-canary.187')
? 's-maxage=31536000, durable'
: 's-maxage=31536000, stale-while-revalidate=31536000, durable',
)

const date1 = await page.textContent('[data-testid="date-now"]')
Expand Down Expand Up @@ -118,7 +121,9 @@ test.describe('app router on-demand revalidation', () => {
expect(headers2['cache-status']).toMatch(/"Next.js"; hit/m)
}
expect(headers2['netlify-cdn-cache-control']).toBe(
's-maxage=31536000, stale-while-revalidate=31536000, durable',
nextVersionSatisfies('>=15.0.0-canary.187')
? 's-maxage=31536000, durable'
: 's-maxage=31536000, stale-while-revalidate=31536000, durable',
)

// the page is cached
Expand Down Expand Up @@ -148,7 +153,9 @@ test.describe('app router on-demand revalidation', () => {
expect(response3?.status()).toBe(200)
expect(headers3?.['x-nextjs-cache']).toBeUndefined()
expect(headers3['netlify-cdn-cache-control']).toBe(
's-maxage=31536000, stale-while-revalidate=31536000, durable',
nextVersionSatisfies('>=15.0.0-canary.187')
? 's-maxage=31536000, durable'
: 's-maxage=31536000, stale-while-revalidate=31536000, durable',
)

// the page has now an updated date
Expand All @@ -175,7 +182,9 @@ test.describe('app router on-demand revalidation', () => {
expect(headers4['cache-status']).toMatch(/"Next.js"; hit/m)
}
expect(headers4['netlify-cdn-cache-control']).toBe(
's-maxage=31536000, stale-while-revalidate=31536000, durable',
nextVersionSatisfies('>=15.0.0-canary.187')
? 's-maxage=31536000, durable'
: 's-maxage=31536000, stale-while-revalidate=31536000, durable',
)

// the page is cached
Expand Down
80 changes: 60 additions & 20 deletions tests/e2e/page-router.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,9 @@ test.describe('Simple Page Router (no basePath, no i18n)', () => {
expect(headers1['x-nextjs-cache']).toBeUndefined()
expect(headers1['netlify-cache-tag']).toBe(`_n_t_${encodeURI(pagePath).toLowerCase()}`)
expect(headers1['netlify-cdn-cache-control']).toBe(
's-maxage=31536000, stale-while-revalidate=31536000, durable',
nextVersionSatisfies('>=15.0.0-canary.187')
? 's-maxage=31536000, durable'
: 's-maxage=31536000, stale-while-revalidate=31536000, durable',
)

const date1 = await page.textContent('[data-testid="date-now"]')
Expand Down Expand Up @@ -156,7 +158,9 @@ test.describe('Simple Page Router (no basePath, no i18n)', () => {
expect(headers1Json['x-nextjs-cache']).toBeUndefined()
expect(headers1Json['netlify-cache-tag']).toBe(`_n_t_${encodeURI(pagePath).toLowerCase()}`)
expect(headers1Json['netlify-cdn-cache-control']).toBe(
's-maxage=31536000, stale-while-revalidate=31536000, durable',
nextVersionSatisfies('>=15.0.0-canary.187')
? 's-maxage=31536000, durable'
: 's-maxage=31536000, stale-while-revalidate=31536000, durable',
)
const data1 = (await response1Json?.json()) || {}
expect(data1?.pageProps?.time).toBe(date1)
Expand All @@ -181,7 +185,9 @@ test.describe('Simple Page Router (no basePath, no i18n)', () => {
expect(headers2['cache-status']).toMatch(/"Next.js"; hit/m)
}
expect(headers2['netlify-cdn-cache-control']).toBe(
's-maxage=31536000, stale-while-revalidate=31536000, durable',
nextVersionSatisfies('>=15.0.0-canary.187')
? 's-maxage=31536000, durable'
: 's-maxage=31536000, stale-while-revalidate=31536000, durable',
)

// the page is cached
Expand Down Expand Up @@ -212,7 +218,9 @@ test.describe('Simple Page Router (no basePath, no i18n)', () => {
expect(headers2Json['cache-status']).toMatch(/"Next.js"; hit/m)
}
expect(headers2Json['netlify-cdn-cache-control']).toBe(
's-maxage=31536000, stale-while-revalidate=31536000, durable',
nextVersionSatisfies('>=15.0.0-canary.187')
? 's-maxage=31536000, durable'
: 's-maxage=31536000, stale-while-revalidate=31536000, durable',
)

const data2 = (await response2Json?.json()) || {}
Expand Down Expand Up @@ -267,7 +275,9 @@ test.describe('Simple Page Router (no basePath, no i18n)', () => {
expect(response3Json?.status()).toBe(200)
expect(headers3Json['x-nextjs-cache']).toBeUndefined()
expect(headers3Json['netlify-cdn-cache-control']).toBe(
's-maxage=31536000, stale-while-revalidate=31536000, durable',
nextVersionSatisfies('>=15.0.0-canary.187')
? 's-maxage=31536000, durable'
: 's-maxage=31536000, stale-while-revalidate=31536000, durable',
)

const data3 = (await response3Json?.json()) || {}
Expand Down Expand Up @@ -382,7 +392,9 @@ test.describe('Simple Page Router (no basePath, no i18n)', () => {
expect(await page.textContent('h1')).toBe('404')

expect(headers['netlify-cdn-cache-control']).toBe(
's-maxage=31536000, stale-while-revalidate=31536000, durable',
nextVersionSatisfies('>=15.0.0-canary.187')
? 's-maxage=31536000, durable'
: 's-maxage=31536000, stale-while-revalidate=31536000, durable',
)
expect(headers['cache-control']).toBe('public,max-age=0,must-revalidate')
})
Expand Down Expand Up @@ -532,7 +544,9 @@ test.describe('Page Router with basePath and i18n', () => {
`_n_t_/en${encodeURI(pagePath).toLowerCase()}`,
)
expect(headers1ImplicitLocale['netlify-cdn-cache-control']).toBe(
's-maxage=31536000, stale-while-revalidate=31536000, durable',
nextVersionSatisfies('>=15.0.0-canary.187')
? 's-maxage=31536000, durable'
: 's-maxage=31536000, stale-while-revalidate=31536000, durable',
)

const date1ImplicitLocale = await page.textContent('[data-testid="date-now"]')
Expand Down Expand Up @@ -560,7 +574,9 @@ test.describe('Page Router with basePath and i18n', () => {
`_n_t_/en${encodeURI(pagePath).toLowerCase()}`,
)
expect(headers1ExplicitLocale['netlify-cdn-cache-control']).toBe(
's-maxage=31536000, stale-while-revalidate=31536000, durable',
nextVersionSatisfies('>=15.0.0-canary.187')
? 's-maxage=31536000, durable'
: 's-maxage=31536000, stale-while-revalidate=31536000, durable',
)

const date1ExplicitLocale = await page.textContent('[data-testid="date-now"]')
Expand Down Expand Up @@ -594,7 +610,9 @@ test.describe('Page Router with basePath and i18n', () => {
`_n_t_/en${encodeURI(pagePath).toLowerCase()}`,
)
expect(headers1Json['netlify-cdn-cache-control']).toBe(
's-maxage=31536000, stale-while-revalidate=31536000, durable',
nextVersionSatisfies('>=15.0.0-canary.187')
? 's-maxage=31536000, durable'
: 's-maxage=31536000, stale-while-revalidate=31536000, durable',
)
const data1 = (await response1Json?.json()) || {}
expect(data1?.pageProps?.time).toBe(date1ImplicitLocale)
Expand Down Expand Up @@ -622,7 +640,9 @@ test.describe('Page Router with basePath and i18n', () => {
expect(headers2ImplicitLocale['cache-status']).toMatch(/"Next.js"; hit/m)
}
expect(headers2ImplicitLocale['netlify-cdn-cache-control']).toBe(
's-maxage=31536000, stale-while-revalidate=31536000, durable',
nextVersionSatisfies('>=15.0.0-canary.187')
? 's-maxage=31536000, durable'
: 's-maxage=31536000, stale-while-revalidate=31536000, durable',
)

// the page is cached
Expand Down Expand Up @@ -652,7 +672,9 @@ test.describe('Page Router with basePath and i18n', () => {
expect(headers2ExplicitLocale['cache-status']).toMatch(/"Next.js"; hit/m)
}
expect(headers2ExplicitLocale['netlify-cdn-cache-control']).toBe(
's-maxage=31536000, stale-while-revalidate=31536000, durable',
nextVersionSatisfies('>=15.0.0-canary.187')
? 's-maxage=31536000, durable'
: 's-maxage=31536000, stale-while-revalidate=31536000, durable',
)

// the page is cached
Expand Down Expand Up @@ -683,7 +705,9 @@ test.describe('Page Router with basePath and i18n', () => {
expect(headers2Json['cache-status']).toMatch(/"Next.js"; hit/m)
}
expect(headers2Json['netlify-cdn-cache-control']).toBe(
's-maxage=31536000, stale-while-revalidate=31536000, durable',
nextVersionSatisfies('>=15.0.0-canary.187')
? 's-maxage=31536000, durable'
: 's-maxage=31536000, stale-while-revalidate=31536000, durable',
)

const data2 = (await response2Json?.json()) || {}
Expand Down Expand Up @@ -777,7 +801,9 @@ test.describe('Page Router with basePath and i18n', () => {
expect(headers3Json['cache-status']).toMatch(/"Next.js"; hit/m)
}
expect(headers3Json['netlify-cdn-cache-control']).toBe(
's-maxage=31536000, stale-while-revalidate=31536000, durable',
nextVersionSatisfies('>=15.0.0-canary.187')
? 's-maxage=31536000, durable'
: 's-maxage=31536000, stale-while-revalidate=31536000, durable',
)

const data3 = (await response3Json?.json()) || {}
Expand Down Expand Up @@ -866,7 +892,9 @@ test.describe('Page Router with basePath and i18n', () => {
expect(response4Json?.status()).toBe(200)
expect(headers4Json['x-nextjs-cache']).toBeUndefined()
expect(headers4Json['netlify-cdn-cache-control']).toBe(
's-maxage=31536000, stale-while-revalidate=31536000, durable',
nextVersionSatisfies('>=15.0.0-canary.187')
? 's-maxage=31536000, durable'
: 's-maxage=31536000, stale-while-revalidate=31536000, durable',
)

const data4 = (await response4Json?.json()) || {}
Expand Down Expand Up @@ -912,7 +940,9 @@ test.describe('Page Router with basePath and i18n', () => {
expect(headers1['x-nextjs-cache']).toBeUndefined()
expect(headers1['netlify-cache-tag']).toBe(`_n_t_/de${encodeURI(pagePath).toLowerCase()}`)
expect(headers1['netlify-cdn-cache-control']).toBe(
's-maxage=31536000, stale-while-revalidate=31536000, durable',
nextVersionSatisfies('>=15.0.0-canary.187')
? 's-maxage=31536000, durable'
: 's-maxage=31536000, stale-while-revalidate=31536000, durable',
)

const date1 = await page.textContent('[data-testid="date-now"]')
Expand Down Expand Up @@ -943,7 +973,9 @@ test.describe('Page Router with basePath and i18n', () => {
`_n_t_/de${encodeURI(pagePath).toLowerCase()}`,
)
expect(headers1Json['netlify-cdn-cache-control']).toBe(
's-maxage=31536000, stale-while-revalidate=31536000, durable',
nextVersionSatisfies('>=15.0.0-canary.187')
? 's-maxage=31536000, durable'
: 's-maxage=31536000, stale-while-revalidate=31536000, durable',
)
const data1 = (await response1Json?.json()) || {}
expect(data1?.pageProps?.time).toBe(date1)
Expand Down Expand Up @@ -971,7 +1003,9 @@ test.describe('Page Router with basePath and i18n', () => {
expect(headers2['cache-status']).toMatch(/"Next.js"; hit/m)
}
expect(headers2['netlify-cdn-cache-control']).toBe(
's-maxage=31536000, stale-while-revalidate=31536000, durable',
nextVersionSatisfies('>=15.0.0-canary.187')
? 's-maxage=31536000, durable'
: 's-maxage=31536000, stale-while-revalidate=31536000, durable',
)

// the page is cached
Expand Down Expand Up @@ -1003,7 +1037,9 @@ test.describe('Page Router with basePath and i18n', () => {
expect(headers2Json['cache-status']).toMatch(/"Next.js"; hit/m)
}
expect(headers2Json['netlify-cdn-cache-control']).toBe(
's-maxage=31536000, stale-while-revalidate=31536000, durable',
nextVersionSatisfies('>=15.0.0-canary.187')
? 's-maxage=31536000, durable'
: 's-maxage=31536000, stale-while-revalidate=31536000, durable',
)

const data2 = (await response2Json?.json()) || {}
Expand Down Expand Up @@ -1070,7 +1106,9 @@ test.describe('Page Router with basePath and i18n', () => {
expect(headers3Json['cache-status']).toMatch(/"Next.js"; hit/m)
}
expect(headers3Json['netlify-cdn-cache-control']).toBe(
's-maxage=31536000, stale-while-revalidate=31536000, durable',
nextVersionSatisfies('>=15.0.0-canary.187')
? 's-maxage=31536000, durable'
: 's-maxage=31536000, stale-while-revalidate=31536000, durable',
)

const data3 = (await response3Json?.json()) || {}
Expand Down Expand Up @@ -1114,7 +1152,9 @@ test.describe('Page Router with basePath and i18n', () => {
expect(await page.textContent('h1')).toBe('404')

expect(headers['netlify-cdn-cache-control']).toBe(
's-maxage=31536000, stale-while-revalidate=31536000, durable',
nextVersionSatisfies('>=15.0.0-canary.187')
? 's-maxage=31536000, durable'
: 's-maxage=31536000, stale-while-revalidate=31536000, durable',
)
expect(headers['cache-control']).toBe('public,max-age=0,must-revalidate')
})
Expand Down
4 changes: 3 additions & 1 deletion tests/e2e/simple-app.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,9 @@ test('requesting a non existing page route that needs to be fetched from the blo
expect(await page.textContent('h1')).toBe('404 Not Found')

expect(headers['netlify-cdn-cache-control']).toBe(
's-maxage=31536000, stale-while-revalidate=31536000, durable',
nextVersionSatisfies('>=15.0.0-canary.187')
? 's-maxage=31536000, durable'
: 's-maxage=31536000, stale-while-revalidate=31536000, durable',
)
expect(headers['cache-control']).toBe('public,max-age=0,must-revalidate')
})
Expand Down
17 changes: 13 additions & 4 deletions tests/e2e/turborepo.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { expect } from '@playwright/test'
import { test } from '../utils/playwright-helpers.js'
import { nextVersionSatisfies } from '../utils/next-version-helpers.mjs'

// those tests have different fixtures and can run in parallel
test.describe.configure({ mode: 'parallel' })
Expand Down Expand Up @@ -35,7 +36,9 @@ test.describe('[PNPM] Package manager', () => {
expect(response1?.status()).toBe(200)
expect(headers1['x-nextjs-cache']).toBeUndefined()
expect(headers1['netlify-cdn-cache-control']).toBe(
's-maxage=31536000, stale-while-revalidate=31536000, durable',
nextVersionSatisfies('>=15.0.0-canary.187')
? 's-maxage=31536000, durable'
: 's-maxage=31536000, stale-while-revalidate=31536000, durable',
)

const date1 = await page.textContent('[data-testid="date-now"]')
Expand Down Expand Up @@ -65,7 +68,9 @@ test.describe('[PNPM] Package manager', () => {
expect(headers2['cache-status']).toMatch(/"Next.js"; hit/m)
}
expect(headers2['netlify-cdn-cache-control']).toBe(
's-maxage=31536000, stale-while-revalidate=31536000, durable',
nextVersionSatisfies('>=15.0.0-canary.187')
? 's-maxage=31536000, durable'
: 's-maxage=31536000, stale-while-revalidate=31536000, durable',
)

// the page is cached
Expand Down Expand Up @@ -139,7 +144,9 @@ test.describe('[NPM] Package manager', () => {
expect(response1?.status()).toBe(200)
expect(headers1['x-nextjs-cache']).toBeUndefined()
expect(headers1['netlify-cdn-cache-control']).toBe(
's-maxage=31536000, stale-while-revalidate=31536000, durable',
nextVersionSatisfies('>=15.0.0-canary.187')
? 's-maxage=31536000, durable'
: 's-maxage=31536000, stale-while-revalidate=31536000, durable',
)

const date1 = await page.textContent('[data-testid="date-now"]')
Expand Down Expand Up @@ -169,7 +176,9 @@ test.describe('[NPM] Package manager', () => {
expect(headers2['cache-status']).toMatch(/"Next.js"; hit/m)
}
expect(headers2['netlify-cdn-cache-control']).toBe(
's-maxage=31536000, stale-while-revalidate=31536000, durable',
nextVersionSatisfies('>=15.0.0-canary.187')
? 's-maxage=31536000, durable'
: 's-maxage=31536000, stale-while-revalidate=31536000, durable',
)

// the page is cached
Expand Down
9 changes: 7 additions & 2 deletions tests/integration/cache-handler.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
getBlobEntries,
startMockBlobStore,
} from '../utils/helpers.js'
import { nextVersionSatisfies } from '../utils/next-version-helpers.mjs'

// Disable the verbose logging of the lambda-local runtime
getLogger().level = 'alert'
Expand Down Expand Up @@ -89,7 +90,9 @@ describe('page router', () => {
).toEqual(
expect.objectContaining({
'cache-status': '"Next.js"; hit',
'netlify-cdn-cache-control': 's-maxage=5, stale-while-revalidate=31536000, durable',
'netlify-cdn-cache-control': nextVersionSatisfies('>=15.0.0-canary.187')
? `s-maxage=5, stale-while-revalidate=${31536000 - 5}, durable`
: 's-maxage=5, stale-while-revalidate=31536000, durable',
}),
)
expect(
Expand Down Expand Up @@ -240,7 +243,9 @@ describe('app router', () => {
// It will be hit instead of stale
expect.objectContaining({
'cache-status': '"Next.js"; hit',
'netlify-cdn-cache-control': 's-maxage=31536000, stale-while-revalidate=31536000, durable',
'netlify-cdn-cache-control': nextVersionSatisfies('>=15.0.0-canary.187')
? 's-maxage=31536000, durable'
: 's-maxage=31536000, stale-while-revalidate=31536000, durable',
}),
)
expect(
Expand Down
Loading
Loading