Skip to content

Commit 9c2f0bb

Browse files
piehpieh
authored
fix: apply permanent cdn-cache-control for fully static page (#274)
* fix: apply permanent cdn-cache-control for fully static page * test: update headers.test.ts * test: add integration and e2e test case * test: update assertion after adding new page
1 parent b2b006f commit 9c2f0bb

File tree

10 files changed

+88
-12
lines changed

10 files changed

+88
-12
lines changed

‎src/run/handlers/request-context.cts

+1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { AsyncLocalStorage } from 'node:async_hooks'
33
export type RequestContext = {
44
responseCacheGetLastModified?: number
55
responseCacheKey?: string
6+
usedFsRead?: boolean
67
}
78

89
type RequestContextAsyncLocalStorage = AsyncLocalStorage<RequestContext>

‎src/run/handlers/server.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ export default async (request: Request) => {
8282

8383
await adjustDateHeader({ headers: response.headers, request, span, tracer, requestContext })
8484

85-
setCacheControlHeaders(response.headers, request)
85+
setCacheControlHeaders(response.headers, request, requestContext)
8686
setCacheTagsHeaders(response.headers, request, tagsManifest)
8787
setVaryHeaders(response.headers, request, nextConfig)
8888
setCacheStatusHeader(response.headers)

‎src/run/headers.test.ts

+29-10
Original file line numberDiff line numberDiff line change
@@ -146,16 +146,35 @@ describe('headers', () => {
146146
describe('setCacheControlHeaders', () => {
147147
const defaultUrl = 'https://example.com'
148148

149-
test('should not set any headers if "cache-control" is not set', () => {
149+
test('should not set any headers if "cache-control" is not set and "requestContext.usedFsRead" is not truthy', () => {
150150
const headers = new Headers()
151151
const request = new Request(defaultUrl)
152152
vi.spyOn(headers, 'set')
153153

154-
setCacheControlHeaders(headers, request)
154+
setCacheControlHeaders(headers, request, {})
155155

156156
expect(headers.set).toHaveBeenCalledTimes(0)
157157
})
158158

159+
test('should set permanent "netlify-cdn-cache-control" if "cache-control" is not set and "requestContext.usedFsRead" is truthy', () => {
160+
const headers = new Headers()
161+
const request = new Request(defaultUrl)
162+
vi.spyOn(headers, 'set')
163+
164+
setCacheControlHeaders(headers, request, { usedFsRead: true })
165+
166+
expect(headers.set).toHaveBeenNthCalledWith(
167+
1,
168+
'cache-control',
169+
'public, max-age=0, must-revalidate',
170+
)
171+
expect(headers.set).toHaveBeenNthCalledWith(
172+
2,
173+
'netlify-cdn-cache-control',
174+
'max-age=31536000',
175+
)
176+
})
177+
159178
test('should not set any headers if "cache-control" is set and "cdn-cache-control" is present', () => {
160179
const givenHeaders = {
161180
'cache-control': 'public, max-age=0, must-revalidate',
@@ -165,7 +184,7 @@ describe('headers', () => {
165184
const request = new Request(defaultUrl)
166185
vi.spyOn(headers, 'set')
167186

168-
setCacheControlHeaders(headers, request)
187+
setCacheControlHeaders(headers, request, {})
169188

170189
expect(headers.set).toHaveBeenCalledTimes(0)
171190
})
@@ -179,7 +198,7 @@ describe('headers', () => {
179198
const request = new Request(defaultUrl)
180199
vi.spyOn(headers, 'set')
181200

182-
setCacheControlHeaders(headers, request)
201+
setCacheControlHeaders(headers, request, {})
183202

184203
expect(headers.set).toHaveBeenCalledTimes(0)
185204
})
@@ -192,7 +211,7 @@ describe('headers', () => {
192211
const request = new Request(defaultUrl)
193212
vi.spyOn(headers, 'set')
194213

195-
setCacheControlHeaders(headers, request)
214+
setCacheControlHeaders(headers, request, {})
196215

197216
expect(headers.set).toHaveBeenNthCalledWith(
198217
1,
@@ -214,7 +233,7 @@ describe('headers', () => {
214233
const request = new Request(defaultUrl, { method: 'HEAD' })
215234
vi.spyOn(headers, 'set')
216235

217-
setCacheControlHeaders(headers, request)
236+
setCacheControlHeaders(headers, request, {})
218237

219238
expect(headers.set).toHaveBeenNthCalledWith(
220239
1,
@@ -236,7 +255,7 @@ describe('headers', () => {
236255
const request = new Request(defaultUrl, { method: 'POST' })
237256
vi.spyOn(headers, 'set')
238257

239-
setCacheControlHeaders(headers, request)
258+
setCacheControlHeaders(headers, request, {})
240259

241260
expect(headers.set).toHaveBeenCalledTimes(0)
242261
})
@@ -249,7 +268,7 @@ describe('headers', () => {
249268
const request = new Request(defaultUrl)
250269
vi.spyOn(headers, 'set')
251270

252-
setCacheControlHeaders(headers, request)
271+
setCacheControlHeaders(headers, request, {})
253272

254273
expect(headers.set).toHaveBeenNthCalledWith(1, 'cache-control', 'public')
255274
expect(headers.set).toHaveBeenNthCalledWith(
@@ -267,7 +286,7 @@ describe('headers', () => {
267286
const request = new Request(defaultUrl)
268287
vi.spyOn(headers, 'set')
269288

270-
setCacheControlHeaders(headers, request)
289+
setCacheControlHeaders(headers, request, {})
271290

272291
expect(headers.set).toHaveBeenNthCalledWith(1, 'cache-control', 'max-age=604800')
273292
expect(headers.set).toHaveBeenNthCalledWith(
@@ -285,7 +304,7 @@ describe('headers', () => {
285304
const request = new Request(defaultUrl)
286305
vi.spyOn(headers, 'set')
287306

288-
setCacheControlHeaders(headers, request)
307+
setCacheControlHeaders(headers, request, {})
289308

290309
expect(headers.set).toHaveBeenNthCalledWith(
291310
1,

‎src/run/headers.ts

+15-1
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,11 @@ export const adjustDateHeader = async ({
174174
* Ensure stale-while-revalidate and s-maxage don't leak to the client, but
175175
* assume the user knows what they are doing if CDN cache controls are set
176176
*/
177-
export const setCacheControlHeaders = (headers: Headers, request: Request) => {
177+
export const setCacheControlHeaders = (
178+
headers: Headers,
179+
request: Request,
180+
requestContext: RequestContext,
181+
) => {
178182
const cacheControl = headers.get('cache-control')
179183
if (
180184
cacheControl !== null &&
@@ -197,6 +201,16 @@ export const setCacheControlHeaders = (headers: Headers, request: Request) => {
197201
headers.set('cache-control', browserCacheControl || 'public, max-age=0, must-revalidate')
198202
headers.set('netlify-cdn-cache-control', cdnCacheControl)
199203
}
204+
205+
if (
206+
cacheControl === null &&
207+
!headers.has('cdn-cache-control') &&
208+
!headers.has('netlify-cdn-cache-control') &&
209+
requestContext.usedFsRead
210+
) {
211+
headers.set('cache-control', 'public, max-age=0, must-revalidate')
212+
headers.set('netlify-cdn-cache-control', `max-age=31536000`)
213+
}
200214
}
201215

202216
export const setCacheTagsHeaders = (headers: Headers, request: Request, manifest: TagsManifest) => {

‎src/run/next.cts

+7
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import { trace } from '@opentelemetry/api'
77
import { patchFs } from 'fs-monkey'
88
import type { getRequestHandlers } from 'next/dist/server/lib/start-server.js'
99

10+
import { getRequestContext } from './handlers/request-context.cjs'
11+
1012
type FS = typeof import('fs')
1113

1214
const fetchBeforeNextPatchedIt = globalThis.fetch
@@ -31,6 +33,11 @@ export async function getMockedRequestHandlers(...args: Parameters<typeof getReq
3133
const relPath = relative(resolve('.next/server/pages'), path)
3234
const file = await store.get(await encodeBlobKey(relPath))
3335
if (file !== null) {
36+
const requestContext = getRequestContext()
37+
if (requestContext) {
38+
requestContext.usedFsRead = true
39+
}
40+
3441
return file
3542
}
3643
}

‎tests/e2e/page-router.test.ts

+14
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,15 @@ test('requesting a non existing page route that needs to be fetched from the blo
7979
pageRouter,
8080
}) => {
8181
const response = await page.goto(new URL('non-exisitng', pageRouter.url).href)
82+
const headers = response?.headers() || {}
8283
expect(response?.status()).toBe(404)
8384

8485
expect(await page.textContent('h1')).toBe('404')
86+
87+
expect(headers['netlify-cdn-cache-control']).toBe(
88+
'no-cache, no-store, max-age=0, must-revalidate',
89+
)
90+
expect(headers['cache-control']).toBe('no-cache,no-store,max-age=0,must-revalidate')
8591
})
8692

8793
test('requesting a page with a very long name works', async ({ page, pageRouter }) => {
@@ -165,3 +171,11 @@ test('unstable-cache should work', async ({ pageRouter }) => {
165171
return 'success'
166172
}, 'success')
167173
})
174+
175+
test('Fully static pages should be cached permanently', async ({ page, pageRouter }) => {
176+
const response = await page.goto(new URL('static/fully-static', pageRouter.url).href)
177+
const headers = response?.headers() || {}
178+
179+
expect(headers['netlify-cdn-cache-control']).toBe('max-age=31536000')
180+
expect(headers['cache-control']).toBe('public,max-age=0,must-revalidate')
181+
})

‎tests/fixtures/page-router/pages/static/fully-static.js

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
const FullyStatic = () => (
2+
<div>
3+
<p>This page is not using getStaticProps()</p>
4+
</div>
5+
)
6+
7+
export default FullyStatic

‎tests/integration/cache-handler.test.ts

+1
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ describe('page router', () => {
4949
'/static/revalidate-slow',
5050
'404.html',
5151
'500.html',
52+
'static/fully-static.html',
5253
])
5354

5455
// test the function call

‎tests/integration/page-router.test.ts

+12
Original file line numberDiff line numberDiff line change
@@ -113,3 +113,15 @@ test<FixtureTestContext>('Should return JSON for data req to page route', async
113113

114114
expect(data.pageProps.show).toBeDefined()
115115
})
116+
117+
test<FixtureTestContext>('Should set permanent "netlify-cdn-cache-control" header on fully static pages"', async (ctx) => {
118+
await createFixture('page-router', ctx)
119+
await runPlugin(ctx)
120+
121+
const response = await invokeFunction(ctx, {
122+
url: '/static/fully-static',
123+
})
124+
125+
expect(response.headers?.['netlify-cdn-cache-control']).toBe('max-age=31536000')
126+
expect(response.headers?.['cache-control']).toBe('public, max-age=0, must-revalidate')
127+
})

‎tests/integration/static.test.ts

+1
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ test<FixtureTestContext>('requesting a non existing page route that needs to be
4545
'/static/revalidate-slow',
4646
'404.html',
4747
'500.html',
48+
'static/fully-static.html',
4849
// the real key is much longer and ends in a hash, but we only assert on the first 50 chars to make it easier
4950
])
5051

0 commit comments

Comments
 (0)