Skip to content

Commit

Permalink
preload fonts using ReactDOM.preload (#48931)
Browse files Browse the repository at this point in the history
This PR updates the way we preload fonts. Previously we tracked which
fonts we needed to preload for each layer and rendered a `<link
rel="preload" href="..." as="font" />` tag for each preloadable font.
This unfortunately gets blocked by data fetching and we want to be able
to hint these preloads as soon as possible. Now that React support Float
methods in RSC we can use `ReactDOM.preload(..., { as: "font" })` to
implement this functionality

This PR makes the following changes
1. expose a `preloadFont` method through the RSC graph
2. expose a `preconnect` metho through the RSC graph
3. refactor the preloads generation to use `preloadFont` instead of
rendering a preload link
4. If there are no fonts to preload but fonts are being used in CSS then
a `preconnect` asset origin is called instead of rendering a preconnect
link
5. instead of emitting a data attribute per font preload indicating
whether the project is using size-adjust we now emit a single global
meta tag. In the future we may get more granular about which fonts are
being size adjusted. In the meantime the current hueristic is to add
`-s` to the filename so it can still be inferred.

In the process of completing this work I discovered there were some bugs
in how the preconnect logic was originally implemented. Previously it
was possible to get multiple preconnects per render. Additionally the
preconnect href was always `"/"` which is not correct if you are hosting
your fonts at a CDN. The refactor fixed both of these issues

I want to do a larger refactor of the asset loading logic in App-Render
but I'll save that for a couple weeks from now

Additionally, the serialized output of preloads now omits the word
anonymous when using crossorigin so tests were updated to reflect
`crossorigin=""`

Additionally, tests were updated to no longer look for the size-adjust
data attribute on preloads

Additionally, There is a note about leaving a `{null}` render in place
to avoid a conflict with how the router models lazy trees. I'll follow
up with a PR addressing this

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
  • Loading branch information
gnoff and kodiakhq[bot] committed Apr 28, 2023
1 parent 799a05c commit 4f5f476
Show file tree
Hide file tree
Showing 7 changed files with 84 additions and 85 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,8 @@ export {
renderToReadableStream,
decodeReply,
} from 'next/dist/compiled/react-server-dom-webpack/server.edge'
export { preloadStyle } from 'next/dist/server/app-render/rsc/preloads'
export {
preloadStyle,
preloadFont,
preconnect,
} from 'next/dist/server/app-render/rsc/preloads'
2 changes: 1 addition & 1 deletion packages/next/src/build/webpack/loaders/next-app-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,7 @@ const nextAppLoader: AppLoader = async function nextAppLoader() {
export { renderToReadableStream, decodeReply } from 'next/dist/compiled/react-server-dom-webpack/server.edge'
export const __next_app_webpack_require__ = __webpack_require__
export { preloadStyle } from 'next/dist/server/app-render/rsc/preloads'
export { preloadStyle, preloadFont, preconnect } from 'next/dist/server/app-render/rsc/preloads'
`

return result
Expand Down
92 changes: 43 additions & 49 deletions packages/next/src/server/app-render/app-render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import type {
RenderOpts,
Segment,
} from './types'

import type { StaticGenerationAsyncStorage } from '../../client/components/static-generation-async-storage'
import type { StaticGenerationBailout } from '../../client/components/static-generation-bailout'
import type { RequestAsyncStorage } from '../../client/components/request-async-storage'
Expand Down Expand Up @@ -60,7 +61,7 @@ import { getShortDynamicParamType } from './get-short-dynamic-param-type'
import { getSegmentParam } from './get-segment-param'
import { getCssInlinedLinkTags } from './get-css-inlined-link-tags'
import { getServerCSSForEntries } from './get-server-css-for-entries'
import { getPreloadedFontFilesInlineLinkTags } from './get-preloaded-font-files-inline-link-tags'
import { getPreloadableFonts } from './get-preloadable-fonts'
import { getScriptNonceFromHeader } from './get-script-nonce-from-header'
import { renderToString } from './render-to-string'
import { parseAndValidateFlightRouterState } from './parse-and-validate-flight-router-state'
Expand Down Expand Up @@ -155,6 +156,8 @@ export async function renderToHTMLOrFlight(
nextConfigOutput,
} = renderOpts

const appUsingSizeAdjust = nextFontManifest?.appUsingSizeAdjust

const clientReferenceManifest = renderOpts.clientReferenceManifest!
const serverCSSManifest = renderOpts.serverCSSManifest!

Expand Down Expand Up @@ -406,10 +409,7 @@ export async function renderToHTMLOrFlight(
layoutOrPagePath: string | undefined
injectedCSS: Set<string>
injectedFontPreloadTags: Set<string>
}): {
styles: React.ReactNode
preloads: React.ReactNode
} => {
}): React.ReactNode => {
const stylesheets: string[] = layoutOrPagePath
? getCssInlinedLinkTags(
clientReferenceManifest,
Expand All @@ -422,49 +422,35 @@ export async function renderToHTMLOrFlight(
: []

const preloadedFontFiles = layoutOrPagePath
? getPreloadedFontFilesInlineLinkTags(
? getPreloadableFonts(
serverCSSManifest!,
nextFontManifest,
serverCSSForEntries,
layoutOrPagePath,
injectedFontPreloadTagsWithCurrentLayout
)
: []
: null

const preloads = (
<>
{preloadedFontFiles?.length === 0 ? (
<>
<link
data-next-font={
nextFontManifest?.appUsingSizeAdjust ? 'size-adjust' : ''
}
rel="preconnect"
href="/"
crossOrigin="anonymous"
/>
</>
) : null}
{preloadedFontFiles
? preloadedFontFiles.map((fontFile) => {
const ext = /\.(woff|woff2|eot|ttf|otf)$/.exec(fontFile)![1]
return (
<link
key={fontFile}
rel="preload"
href={`${assetPrefix}/_next/${fontFile}`}
as="font"
type={`font/${ext}`}
crossOrigin="anonymous"
data-next-font={
fontFile.includes('-s') ? 'size-adjust' : ''
}
/>
)
})
: null}
</>
)
if (preloadedFontFiles) {
if (preloadedFontFiles.length) {
for (let i = 0; i < preloadedFontFiles.length; i++) {
const fontFilename = preloadedFontFiles[i]
const ext = /\.(woff|woff2|eot|ttf|otf)$/.exec(fontFilename)![1]
const type = `font/${ext}`
const href = `${assetPrefix}/_next/${fontFilename}`
ComponentMod.preloadFont(href, type)
}
} else {
try {
let url = new URL(assetPrefix)
ComponentMod.preconnect(url.origin, 'anonymous')
} catch (error) {
// assetPrefix must not be a fully qualified domain name. We assume
// we should preconnect to same origin instead
ComponentMod.preconnect('/', 'anonymous')
}
}
}

const styles = stylesheets
? stylesheets.map((href, index) => {
Expand Down Expand Up @@ -493,10 +479,7 @@ export async function renderToHTMLOrFlight(
})
: null

return {
styles,
preloads,
}
return styles
}

const parseLoaderTree = (tree: LoaderTree) => {
Expand Down Expand Up @@ -560,7 +543,7 @@ export async function renderToHTMLOrFlight(
injectedFontPreloadTags
)

const { styles, preloads } = getLayerAssets({
const styles = getLayerAssets({
layoutOrPagePath,
injectedCSS: injectedCSSWithCurrentLayout,
injectedFontPreloadTags: injectedFontPreloadTagsWithCurrentLayout,
Expand Down Expand Up @@ -911,7 +894,15 @@ export async function renderToHTMLOrFlight(
) : (
<Component {...props} />
)}
{preloads}
{/* This null is currently critical. The wrapped Component can render null and if there was not fragment
surrounding it this would look like a pending tree data state on the client which will cause an errror
and break the app. Long-term we need to move away from using null as a partial tree identifier since it
is a valid return type for the components we wrap. Once we make this change we can safely remove the
fragment. The reason the extra null here is required is that fragments which only have 1 child are elided.
If the Component above renders null the actual treedata will look like `[null, null]`. If we remove the extra
null it will look like `null` (the array is elided) and this is what confuses the client router.
TODO-APP update router to use a Symbol for partial tree detection */}
{null}
</>
)
},
Expand Down Expand Up @@ -1040,7 +1031,7 @@ export async function renderToHTMLOrFlight(
const { layoutOrPagePath } =
parseLoaderTree(loaderTreeToFilter)

const { styles } = getLayerAssets({
const styles = getLayerAssets({
layoutOrPagePath,
injectedCSS: new Set(injectedCSS),
injectedFontPreloadTags: new Set(injectedFontPreloadTags),
Expand Down Expand Up @@ -1074,7 +1065,7 @@ export async function renderToHTMLOrFlight(
injectedCSSWithCurrentLayout,
true
)
getPreloadedFontFilesInlineLinkTags(
getPreloadableFonts(
serverCSSManifest!,
nextFontManifest,
serverCSSForEntries,
Expand Down Expand Up @@ -1155,6 +1146,7 @@ export async function renderToHTMLOrFlight(
searchParams={providedSearchParams}
getDynamicParamFromSegment={getDynamicParamFromSegment}
/>
{appUsingSizeAdjust ? <meta name="next-size-adjust" /> : null}
</>
),
injectedCSS: new Set(),
Expand Down Expand Up @@ -1292,6 +1284,7 @@ export async function renderToHTMLOrFlight(
searchParams={providedSearchParams}
getDynamicParamFromSegment={getDynamicParamFromSegment}
/>
{appUsingSizeAdjust ? <meta name="next-size-adjust" /> : null}
</>
}
globalErrorComponent={GlobalError}
Expand Down Expand Up @@ -1499,6 +1492,7 @@ export async function renderToHTMLOrFlight(
searchParams={providedSearchParams}
getDynamicParamFromSegment={getDynamicParamFromSegment}
/>
{appUsingSizeAdjust ? <meta name="next-size-adjust" /> : null}
</head>
<body></body>
</html>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
import { getClientReferenceModuleKey } from '../../lib/client-reference'

/**
* Get inline <link> tags based on server CSS manifest. Only used when rendering to HTML.
* Get external stylesheet link hrefs based on server CSS manifest.
*/
export function getCssInlinedLinkTags(
clientReferenceManifest: ClientReferenceManifest,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,13 @@ import { NextFontManifest } from '../../build/webpack/plugins/next-font-manifest
import { ClientCSSReferenceManifest } from '../../build/webpack/plugins/flight-manifest-plugin'

/**
* Get inline <link rel="preload" as="font"> tags based on server CSS manifest and next/font manifest. Only used when rendering to HTML.
* Get hrefs for fonts to preload
* Returns null if there are no fonts at all.
* Returns string[] if there are fonts to preload (font paths)
* Returns empty string[] if there are fonts but none to preload and no other fonts have been preloaded
* Returns null if there are fonts but none to preload and at least some were previously preloaded
*/
export function getPreloadedFontFilesInlineLinkTags(
export function getPreloadableFonts(
serverCSSManifest: ClientCSSReferenceManifest,
nextFontManifest: NextFontManifest | undefined,
serverCSSForEntries: string[],
Expand Down Expand Up @@ -39,15 +43,11 @@ export function getPreloadedFontFilesInlineLinkTags(
}
}

// If we find an entry in the manifest but it's empty, add a preconnect tag by returning null.
// Only render a preconnect tag if we previously didn't preload any fonts.
if (
!foundFontUsage ||
(fontFiles.size === 0 && injectedFontPreloadTags.size > 0)
) {
if (fontFiles.size) {
return [...fontFiles].sort()
} else if (foundFontUsage && injectedFontPreloadTags.size === 0) {
return []
} else {
return null
}

// Sorting to make order deterministic
return [...fontFiles].sort()
}
12 changes: 12 additions & 0 deletions packages/next/src/server/app-render/rsc/preloads.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,15 @@ const stylePreloadOptions = { as: 'style' }
export function preloadStyle(href: string) {
;(ReactDOM as any).preload(href, stylePreloadOptions)
}

export function preloadFont(href: string, type: string) {
;(ReactDOM as any).preload(href, { as: 'font', type })
}

export function preconnect(href: string, crossOrigin?: string) {
if (typeof crossOrigin === 'string') {
;(ReactDOM as any).preconnect(href, { crossOrigin })
} else {
;(ReactDOM as any).preconnect(href)
}
}
33 changes: 11 additions & 22 deletions test/e2e/app-dir/next-font/next-font.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,27 +235,24 @@ describe.each([['app'], ['app-old']])('%s', (fixture: string) => {
expect(getAttrs($('link[as="font"]'))).toEqual([
{
as: 'font',
crossorigin: 'anonymous',
crossorigin: '',
href: '/_next/static/media/b2104791981359ae-s.p.woff2',
rel: 'preload',
type: 'font/woff2',
'data-next-font': 'size-adjust',
},
{
as: 'font',
crossorigin: 'anonymous',
crossorigin: '',
href: '/_next/static/media/b61859a50be14c53-s.p.woff2',
rel: 'preload',
type: 'font/woff2',
'data-next-font': 'size-adjust',
},
{
as: 'font',
crossorigin: 'anonymous',
crossorigin: '',
href: '/_next/static/media/e9b9dc0d8ba35f48-s.p.woff2',
rel: 'preload',
type: 'font/woff2',
'data-next-font': 'size-adjust',
},
])
})
Expand All @@ -270,27 +267,24 @@ describe.each([['app'], ['app-old']])('%s', (fixture: string) => {
expect(getAttrs($('link[as="font"]'))).toEqual([
{
as: 'font',
crossorigin: 'anonymous',
crossorigin: '',
href: '/_next/static/media/e1053f04babc7571-s.p.woff2',
rel: 'preload',
type: 'font/woff2',
'data-next-font': 'size-adjust',
},
{
as: 'font',
crossorigin: 'anonymous',
crossorigin: '',
href: '/_next/static/media/e9b9dc0d8ba35f48-s.p.woff2',
rel: 'preload',
type: 'font/woff2',
'data-next-font': 'size-adjust',
},
{
as: 'font',
crossorigin: 'anonymous',
crossorigin: '',
href: '/_next/static/media/feab2c68f2a8e9a4-s.p.woff2',
rel: 'preload',
type: 'font/woff2',
'data-next-font': 'size-adjust',
},
])
})
Expand All @@ -305,19 +299,17 @@ describe.each([['app'], ['app-old']])('%s', (fixture: string) => {
expect(getAttrs($('link[as="font"]'))).toEqual([
{
as: 'font',
crossorigin: 'anonymous',
crossorigin: '',
href: '/_next/static/media/75c5faeeb9c86969-s.p.woff2',
rel: 'preload',
type: 'font/woff2',
'data-next-font': 'size-adjust',
},
{
as: 'font',
crossorigin: 'anonymous',
crossorigin: '',
href: '/_next/static/media/e9b9dc0d8ba35f48-s.p.woff2',
rel: 'preload',
type: 'font/woff2',
'data-next-font': 'size-adjust',
},
])
})
Expand All @@ -332,19 +324,17 @@ describe.each([['app'], ['app-old']])('%s', (fixture: string) => {
expect(getAttrs($('link[as="font"]'))).toEqual([
{
as: 'font',
crossorigin: 'anonymous',
crossorigin: '',
href: '/_next/static/media/568e4c6d8123c4d6-s.p.woff2',
rel: 'preload',
type: 'font/woff2',
'data-next-font': 'size-adjust',
},
{
as: 'font',
crossorigin: 'anonymous',
crossorigin: '',
href: '/_next/static/media/e9b9dc0d8ba35f48-s.p.woff2',
rel: 'preload',
type: 'font/woff2',
'data-next-font': 'size-adjust',
},
])
})
Expand All @@ -359,10 +349,9 @@ describe.each([['app'], ['app-old']])('%s', (fixture: string) => {
// Preconnect
expect($('link[rel="preconnect"]').length).toBe(1)
expect($('link[rel="preconnect"]').get(0).attribs).toEqual({
crossorigin: 'anonymous',
crossorigin: '',
href: '/',
rel: 'preconnect',
'data-next-font': 'size-adjust',
})
// Preload
expect($('link[as="font"]').length).toBe(0)
Expand Down

0 comments on commit 4f5f476

Please sign in to comment.