Skip to content

Commit 413968e

Browse files
authored
Merge pull request #14974 from artsy/joeyAghion/lcpHint
chore(perf): include a high-priority image in Link response headers, if available
2 parents ce82e30 + 14b11e7 commit 413968e

File tree

6 files changed

+87
-111
lines changed

6 files changed

+87
-111
lines changed
Lines changed: 13 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
import fs from "fs"
22
import path from "path"
3-
import { getWebpackEarlyHints } from "Server/getWebpackEarlyHints"
3+
import { getEarlyHints } from "Server/getEarlyHints"
44

55
jest.mock("fs")
66
jest.mock("Server/config", () => ({ CDN_URL: "https://cdn.example.com" }))
77

88
const HINTS_PATH = path.join(process.cwd(), "dist", "early-hints.json")
99

10-
describe("getWebpackEarlyHints", () => {
10+
describe("getEarlyHints", () => {
1111
const mockReadFileSync = fs.readFileSync as jest.Mock
1212

1313
afterEach(() => {
@@ -20,10 +20,20 @@ describe("getWebpackEarlyHints", () => {
2020

2121
mockReadFileSync.mockReturnValueOnce(JSON.stringify(mockChunkFiles))
2222

23-
const result = getWebpackEarlyHints()
23+
const result = getEarlyHints([
24+
{
25+
type: "link",
26+
props: {
27+
rel: "preload",
28+
as: "image",
29+
href: "https://example.com/image.jpg",
30+
},
31+
},
32+
])
2433

2534
expect(fs.readFileSync).toHaveBeenCalledWith(HINTS_PATH, "utf-8")
2635
expect(result.linkHeaders).toEqual([
36+
`<https://example.com/image.jpg>; rel=preload; as=image`,
2737
`<https://cdn.example.com/chunk1.js>; rel=preload; as=script`,
2838
`<https://cdn.example.com/chunk2.js>; rel=preload; as=script`,
2939
])
@@ -32,23 +42,4 @@ describe("getWebpackEarlyHints", () => {
3242
`<link rel="preload" as="script" href="https://cdn.example.com/chunk2.js">`,
3343
])
3444
})
35-
36-
it("should return link headers and preload tags without CDN URL in development", () => {
37-
process.env.NODE_ENV = "development"
38-
const mockChunkFiles = ["/chunk1.js", "/chunk2.js"]
39-
40-
mockReadFileSync.mockReturnValueOnce(JSON.stringify(mockChunkFiles))
41-
42-
const result = getWebpackEarlyHints()
43-
44-
expect(fs.readFileSync).toHaveBeenCalledWith(HINTS_PATH, "utf-8")
45-
expect(result.linkHeaders).toEqual([
46-
`</chunk1.js>; rel=preload; as=script`,
47-
`</chunk2.js>; rel=preload; as=script`,
48-
])
49-
expect(result.linkPreloadTags).toEqual([
50-
`<link rel="preload" as="script" href="/chunk1.js">`,
51-
`<link rel="preload" as="script" href="/chunk2.js">`,
52-
])
53-
})
5445
})

src/Server/getEarlyHints.ts

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import { CDN_URL } from "Server/config"
2+
import path from "path"
3+
import fs from "fs"
4+
5+
const HINTS_PATH = path.join(process.cwd(), "dist", "early-hints.json")
6+
7+
const cdnUrl = (() => {
8+
if (process.env.NODE_ENV === "development") {
9+
return ""
10+
}
11+
12+
return CDN_URL
13+
})()
14+
15+
export const getEarlyHints = (
16+
headTags: any[]
17+
): {
18+
linkHeaders: string[]
19+
linkPreloadTags: string[]
20+
} => {
21+
const results = {
22+
linkHeaders: [] as string[],
23+
linkPreloadTags: [] as string[],
24+
}
25+
26+
// Set link headers based on <link...> tags rendered into the head
27+
const linkTags = headTags.filter(
28+
tag =>
29+
tag.type === "link" &&
30+
tag.props?.rel === "preload" &&
31+
tag.props?.as === "image" &&
32+
!!tag.props?.href
33+
)
34+
for (const linkTag of linkTags) {
35+
results.linkHeaders.push(
36+
`<${linkTag.props?.href}>; rel=preload; as=${linkTag.props?.as}`
37+
)
38+
}
39+
40+
// Also add initial js chunks to link headers and inject preload tags
41+
try {
42+
const chunkFiles = JSON.parse(fs.readFileSync(HINTS_PATH, "utf-8"))
43+
for (const file of chunkFiles) {
44+
results.linkHeaders.push(`<${cdnUrl}${file}>; rel=preload; as=script`)
45+
results.linkPreloadTags.push(
46+
`<link rel="preload" as="script" href="${cdnUrl}${file}">`
47+
)
48+
}
49+
} catch (error) {
50+
console.error("[getEarlyHints] Could not load early-hints.json:", error)
51+
}
52+
53+
return results
54+
}

src/Server/getWebpackEarlyHints.ts

Lines changed: 0 additions & 50 deletions
This file was deleted.

src/Server/middleware/linkHeadersMiddleware.ts

Lines changed: 0 additions & 32 deletions
This file was deleted.

src/System/Router/renderServerApp.tsx

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,13 @@ import { loadAssetManifest } from "Server/manifest"
55
import { ENABLE_SSR_STREAMING } from "Server/config"
66
import { getENV } from "Utils/getENV"
77
import { ServerAppResults } from "System/Router/serverRouter"
8-
import { getWebpackEarlyHints } from "Server/getWebpackEarlyHints"
8+
import { getEarlyHints } from "Server/getEarlyHints"
99
import { RenderToStreamResult } from "System/Router/Utils/renderToStream"
1010
import { buildHtmlTemplate, HTMLProps } from "html"
1111

1212
// TODO: Use the same variables as the asset middleware. Both config and sharify
1313
// have a default CDN_URL while this does not.
14-
const { CDN_URL, NODE_ENV, GEMINI_CLOUDFRONT_URL } = process.env
14+
const { CDN_URL, NODE_ENV, GEMINI_CLOUDFRONT_URL, WEBFONT_URL } = process.env
1515

1616
const MANIFEST = loadAssetManifest("dist/manifest.json")
1717

@@ -48,7 +48,7 @@ export const renderServerApp = ({
4848

4949
const scripts = extractScriptTags?.()
5050

51-
const { linkPreloadTags } = getWebpackEarlyHints()
51+
const { linkHeaders, linkPreloadTags } = getEarlyHints(headTags ?? [])
5252

5353
const options: HTMLProps = {
5454
cdnUrl: NODE_ENV === "production" ? (CDN_URL as string) : "",
@@ -84,6 +84,8 @@ export const renderServerApp = ({
8484
},
8585
}
8686

87+
setLinkHeaders(linkHeaders, res)
88+
8789
const statusCode = getENV("statusCode") ?? code
8890

8991
const htmlShell = buildHtmlTemplate(options)
@@ -107,3 +109,18 @@ export const renderServerApp = ({
107109
res.status(statusCode).send(htmlShell)
108110
}
109111
}
112+
113+
const setLinkHeaders = (linkHeaders: string[], res: ArtsyResponse) => {
114+
if (!res.headersSent) {
115+
res.header("Link", [
116+
`<${CDN_URL}>; rel=preconnect; crossorigin`,
117+
`<${GEMINI_CLOUDFRONT_URL}>; rel=preconnect;`,
118+
`<${WEBFONT_URL}>; rel=preconnect; crossorigin`,
119+
`<${WEBFONT_URL}/all-webfonts.css>; rel=preload; as=style`,
120+
`<${WEBFONT_URL}/ll-unica77_regular.woff2>; rel=preload; as=font; crossorigin`,
121+
`<${WEBFONT_URL}/ll-unica77_medium.woff2>; rel=preload; as=font; crossorigin`,
122+
`<${WEBFONT_URL}/ll-unica77_italic.woff2>; rel=preload; as=font; crossorigin`,
123+
...linkHeaders,
124+
])
125+
}
126+
}

src/middleware.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ import {
3333
import { morganMiddleware } from "./Server/middleware/morgan"
3434
import { ensureSslMiddleware } from "./Server/middleware/ensureSsl"
3535
import { hstsMiddleware } from "./Server/middleware/hsts"
36-
import { linkHeadersMiddleware } from "./Server/middleware/linkHeadersMiddleware"
3736
import { ipFilter } from "./Server/middleware/ipFilter"
3837
import { sessionMiddleware } from "./Server/middleware/session"
3938
import { assetMiddleware } from "./Server/middleware/assetMiddleware"
@@ -175,9 +174,6 @@ export function initializeMiddleware(app) {
175174
registerFeatureFlagService(UnleashService, UnleashFeatureFlagService)
176175
app.use(featureFlagMiddleware(UnleashService))
177176

178-
// Preconnect or preload associated links
179-
app.use(linkHeadersMiddleware)
180-
181177
/**
182178
* Routes for pinging system time and up
183179
*/

0 commit comments

Comments
 (0)