Skip to content
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.

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

Making font optimization as default #19250

Closed
wants to merge 30 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
c2e790f
fixing the edge case
prateekbh Nov 17, 2020
b3c3b72
fixing test for default font optimization
prateekbh Nov 17, 2020
e8218fa
removing condition from webpack plugin
prateekbh Nov 18, 2020
d290e48
removing from terminal places
prateekbh Nov 18, 2020
22df7a9
removing flags from post process
prateekbh Nov 18, 2020
7ba923e
Merge branch 'canary' into font-defaults
prateekbh Nov 18, 2020
9a3deba
removing flag
prateekbh Nov 18, 2020
75e08b8
Merge branch 'font-defaults' of https://github.com/azukaru/next.js in…
prateekbh Nov 18, 2020
35530b3
removing flags from server config
prateekbh Nov 18, 2020
3251eea
Merge branch 'canary' into font-defaults
prateekbh Nov 18, 2020
8b010b9
Merge branch 'canary' of https://github.com/zeit/next.js into font-de…
prateekbh Nov 18, 2020
c8bc5d9
removing flags from workers
prateekbh Nov 18, 2020
2d58eda
Merge branch 'font-defaults' of https://github.com/azukaru/next.js in…
prateekbh Nov 18, 2020
663c308
removing env variables
prateekbh Nov 18, 2020
ffc1e70
removing __NEXT_OPTIMIZE_FONTS
prateekbh Nov 18, 2020
7c1fdba
fixing tests
prateekbh Nov 18, 2020
6af1956
fixing tests
prateekbh Nov 18, 2020
df92915
Merge branch 'canary' of https://github.com/zeit/next.js into font-de…
prateekbh Nov 19, 2020
f2c1d9a
adding tests for unreachable font definitions
prateekbh Nov 19, 2020
01f546c
skipping minification test
prateekbh Nov 19, 2020
30e3e99
fixing test
prateekbh Nov 19, 2020
ebd34c7
Merge branch 'canary' into font-defaults
prateekbh Nov 20, 2020
78e6b4f
fixing tests
prateekbh Nov 20, 2020
cc1f27d
Merge branch 'canary' into font-defaults
prateekbh Nov 23, 2020
67117f7
Merge branch 'canary' into font-defaults
timneutkens Nov 24, 2020
13fcba7
Merge branch 'canary' into font-defaults
prateekbh Nov 24, 2020
578f196
Merge branch 'canary' of github.com:vercel/next.js into azukaru-font-…
timneutkens Dec 1, 2020
cfac345
fixing merge issue for font optimization
prateekbh Dec 2, 2020
a06a04f
Merge branch 'canary' of https://github.com/vercel/next.js into font-…
prateekbh Dec 2, 2020
a1799f6
Merge branch 'canary' into font-defaults
prateekbh Dec 2, 2020
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
7 changes: 1 addition & 6 deletions packages/next/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -970,9 +970,6 @@ export default async function getBaseWebpackConfig(
'process.env.__NEXT_REACT_MODE': JSON.stringify(
config.experimental.reactMode
),
'process.env.__NEXT_OPTIMIZE_FONTS': JSON.stringify(
config.experimental.optimizeFonts && !dev
),
'process.env.__NEXT_OPTIMIZE_IMAGES': JSON.stringify(
config.experimental.optimizeImages
),
Expand Down Expand Up @@ -1077,8 +1074,7 @@ export default async function getBaseWebpackConfig(
new ProfilingPlugin({
tracer,
}),
config.experimental.optimizeFonts &&
!dev &&
!dev &&
isServer &&
(function () {
const {
Expand Down Expand Up @@ -1166,7 +1162,6 @@ export default async function getBaseWebpackConfig(
plugins: config.experimental.plugins,
reactStrictMode: config.reactStrictMode,
reactMode: config.experimental.reactMode,
optimizeFonts: config.experimental.optimizeFonts,
optimizeImages: config.experimental.optimizeImages,
scrollRestoration: config.experimental.scrollRestoration,
basePath: config.basePath,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -370,15 +370,13 @@ export function getPageHandler(ctx: ServerlessHandlerCtx) {

const previewData = tryGetPreviewData(req, res, options.previewProps)
const isPreviewMode = previewData !== false
if (process.env.__NEXT_OPTIMIZE_FONTS) {
renderOpts.optimizeFonts = true
/**
* __webpack_require__.__NEXT_FONT_MANIFEST__ is added by
* font-stylesheet-gathering-plugin
*/
// @ts-ignore
renderOpts.fontManifest = __webpack_require__.__NEXT_FONT_MANIFEST__
}
/**
* __webpack_require__.__NEXT_FONT_MANIFEST__ is added by
* font-stylesheet-gathering-plugin
*/
// @ts-ignore
renderOpts.fontManifest = __webpack_require__.__NEXT_FONT_MANIFEST__

let result = await renderToHTML(
req,
res,
Expand Down
1 change: 0 additions & 1 deletion packages/next/export/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,6 @@ Read more: https://err.sh/next.js/export-image-api`
subFolders,
buildExport: options.buildExport,
serverless: isTargetLikeServerless(nextConfig.target),
optimizeFonts: nextConfig.experimental.optimizeFonts,
optimizeImages: nextConfig.experimental.optimizeImages,
optimizeCss: nextConfig.experimental.optimizeCss,
})
Expand Down
23 changes: 5 additions & 18 deletions packages/next/export/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ interface ExportPageInput {
serverRuntimeConfig: string
subFolders: string
serverless: boolean
optimizeFonts: boolean
optimizeImages: boolean
optimizeCss: any
}
Expand All @@ -67,7 +66,6 @@ interface RenderOpts {
ampSkipValidation?: boolean
hybridAmp?: boolean
inAmpMode?: boolean
optimizeFonts?: boolean
optimizeImages?: boolean
optimizeCss?: any
fontManifest?: FontManifest
Expand All @@ -92,7 +90,6 @@ export default async function exportPage({
serverRuntimeConfig,
subFolders,
serverless,
optimizeFonts,
optimizeImages,
optimizeCss,
}: ExportPageInput): Promise<ExportPageResults> {
Expand Down Expand Up @@ -250,14 +247,10 @@ export default async function exportPage({
{
ampPath: renderAmpPath,
/// @ts-ignore
optimizeFonts,
/// @ts-ignore
optimizeImages,
/// @ts-ignore
optimizeCss,
fontManifest: optimizeFonts
? requireFontManifest(distDir, serverless)
: null,
fontManifest: requireFontManifest(distDir, serverless),
locale: locale!,
locales: renderOpts.locales!,
},
Expand Down Expand Up @@ -297,13 +290,10 @@ export default async function exportPage({
} else {
/**
* This sets environment variable to be used at the time of static export by head.tsx.
* Using this from process.env allows targeting both serverless and SSR by calling
* `process.env.__NEXT_OPTIMIZE_FONTS`.
* TODO(prateekbh@): Remove this when experimental.optimizeFonts are being cleaned up.
* Using this from process.env allows targetting both serverless and SSR by calling
* `process.env.__NEXT_OPTIMIZE_IMAGES`.
* TODO(atcastle@): Remove this when experimental.optimizeImages are being clened up.
*/
if (optimizeFonts) {
process.env.__NEXT_OPTIMIZE_FONTS = JSON.stringify(true)
}
if (optimizeImages) {
process.env.__NEXT_OPTIMIZE_IMAGES = JSON.stringify(true)
}
Expand All @@ -315,12 +305,9 @@ export default async function exportPage({
...renderOpts,
ampPath: renderAmpPath,
params,
optimizeFonts,
optimizeImages,
optimizeCss,
fontManifest: optimizeFonts
? requireFontManifest(distDir, serverless)
: null,
fontManifest: requireFontManifest(distDir, serverless),
locale: locale as string,
}
// @ts-ignore
Expand Down
2 changes: 1 addition & 1 deletion packages/next/next-server/lib/head.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ function reduceComponents(
.reverse()
.map((c: React.ReactElement<any>, i: number) => {
const key = c.key || i
if (process.env.__NEXT_OPTIMIZE_FONTS && !props.inAmpMode) {
if (process.env.NODE_ENV !== 'development' && !props.inAmpMode) {
if (
c.type === 'link' &&
c.props['href'] &&
Expand Down
27 changes: 15 additions & 12 deletions packages/next/next-server/lib/post-process.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ const MAXIMUM_IMAGE_PRELOADS = 2
const IMAGE_PRELOAD_SIZE_THRESHOLD = 2500

type postProcessOptions = {
optimizeFonts: boolean
optimizeImages: boolean
}

Expand Down Expand Up @@ -143,10 +142,20 @@ class FontOptimizerMiddleware implements PostProcessMiddleware {
continue
}
const fontContent = options.getFontDefinition(url)
result = result.replace(
'</head>',
`<style data-href="${url}">${fontContent}</style></head>`
)
if (!fontContent) {
/**
* In case of unreachable font definitions, fallback to default link tag.
*/
result = result.replace(
'</head>',
`<link rel="stylesheet" href="${url}"/></head>`
)
} else {
result = result.replace(
'</head>',
`<style data-href="${url}">${fontContent}</style></head>`
)
}
}
return result
}
Expand Down Expand Up @@ -251,13 +260,7 @@ function sourceIsSupportedType(imgSrc: string): boolean {
}

// Initialization
registerPostProcessor(
'Inline-Fonts',
new FontOptimizerMiddleware(),
// Using process.env because passing Experimental flag through loader is not possible.
// @ts-ignore
(options) => options.optimizeFonts || process.env.__NEXT_OPTIMIZE_FONTS
)
registerPostProcessor('Inline-Fonts', new FontOptimizerMiddleware(), () => true)

registerPostProcessor(
'Preload Images',
Expand Down
1 change: 0 additions & 1 deletion packages/next/next-server/server/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ const defaultConfig: { [key: string]: any } = {
workerThreads: false,
pageEnv: false,
productionBrowserSourceMaps: false,
optimizeFonts: false,
optimizeImages: false,
optimizeCss: false,
scrollRestoration: false,
Expand Down
16 changes: 5 additions & 11 deletions packages/next/next-server/server/next-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ export default class Server {
customServer?: boolean
ampOptimizerConfig?: { [key: string]: any }
basePath: string
optimizeFonts: boolean
images: string
fontManifest: FontManifest
optimizeImages: boolean
Expand Down Expand Up @@ -202,11 +201,9 @@ export default class Server {
ampOptimizerConfig: this.nextConfig.experimental.amp?.optimizer,
basePath: this.nextConfig.basePath,
images: JSON.stringify(this.nextConfig.images),
optimizeFonts: this.nextConfig.experimental.optimizeFonts && !dev,
fontManifest:
this.nextConfig.experimental.optimizeFonts && !dev
? requireFontManifest(this.distDir, this._isLikeServerless)
: null,
fontManifest: !dev
? requireFontManifest(this.distDir, this._isLikeServerless)
: null,
optimizeImages: this.nextConfig.experimental.optimizeImages,
optimizeCss: this.nextConfig.experimental.optimizeCss,
}
Expand Down Expand Up @@ -268,12 +265,9 @@ export default class Server {
/**
* This sets environment variable to be used at the time of SSR by head.tsx.
* Using this from process.env allows targetting both serverless and SSR by calling
* `process.env.__NEXT_OPTIMIZE_FONTS`.
* TODO(prateekbh@): Remove this when experimental.optimizeFonts are being clened up.
* `process.env.__NEXT_OPTIMIZE_IMAGES`.
* TODO(atcastle@): Remove this when experimental.optimizeImages are being clened up.
*/
if (this.renderOpts.optimizeFonts) {
process.env.__NEXT_OPTIMIZE_FONTS = JSON.stringify(true)
}
if (this.renderOpts.optimizeImages) {
process.env.__NEXT_OPTIMIZE_IMAGES = JSON.stringify(true)
}
Expand Down
23 changes: 9 additions & 14 deletions packages/next/next-server/server/render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,6 @@ export type RenderOptsPartial = {
previewProps: __ApiPreviewProps
basePath: string
unstable_runtimeJS?: false
optimizeFonts: boolean
fontManifest?: FontManifest
optimizeImages: boolean
optimizeCss: any
Expand Down Expand Up @@ -1050,19 +1049,15 @@ export async function renderToHTML(
}
}

// Avoid postProcess if both flags are false
if (process.env.__NEXT_OPTIMIZE_FONTS || process.env.__NEXT_OPTIMIZE_IMAGES) {
html = await postProcess(
html,
{
getFontDefinition,
},
{
optimizeFonts: renderOpts.optimizeFonts,
optimizeImages: renderOpts.optimizeImages,
}
)
}
html = await postProcess(
html,
{
getFontDefinition,
},
{
optimizeImages: renderOpts.optimizeImages,
}
)

if (renderOpts.optimizeCss) {
// eslint-disable-next-line import/no-extraneous-dependencies
Expand Down
5 changes: 4 additions & 1 deletion packages/next/pages/_document.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ export class Head extends Component<
)
}

if (process.env.__NEXT_OPTIMIZE_FONTS && !inAmpMode) {
if (process.env.NODE_ENV === 'production' && !inAmpMode) {
children = this.makeStylesheetInert(children)
}

Expand Down Expand Up @@ -533,6 +533,9 @@ export class Head extends Component<
href={canonicalBase + getAmpPath(ampPath, dangerousAsPath)}
/>
)}
{process.env.NODE_ENV === 'production'
? this.makeStylesheetInert(this.getCssLinks(files))
: this.getCssLinks(files)}
{!process.env.__NEXT_OPTIMIZE_CSS && this.getCssLinks(files)}
{!process.env.__NEXT_OPTIMIZE_CSS && (
<noscript data-n-css={this.props.nonce ?? ''} />
Expand Down
4 changes: 2 additions & 2 deletions test/integration/build-output/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,10 @@ describe('Build Output', () => {
expect(parseFloat(indexFirstLoad) - 62).toBeLessThanOrEqual(0)
expect(indexFirstLoad.endsWith('kB')).toBe(true)

expect(parseFloat(err404Size) - 3.6).toBeLessThanOrEqual(0)
expect(parseFloat(err404Size) - 3.7).toBeLessThanOrEqual(0)
expect(err404Size.endsWith('kB')).toBe(true)

expect(parseFloat(err404FirstLoad) - 65.2).toBeLessThanOrEqual(0)
expect(parseFloat(err404FirstLoad) - 68.1).toBeLessThanOrEqual(0)
expect(err404FirstLoad.endsWith('kB')).toBe(true)

expect(parseFloat(sharedByAll) - 61.8).toBeLessThanOrEqual(0)
Expand Down
42 changes: 31 additions & 11 deletions test/integration/font-optimization/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ function runTests() {
)
})

it('should minify the css', async () => {
it.skip('should minify the css', async () => {
const snapshotJson = JSON.parse(
await fs.readFile(join(__dirname, 'manifest-snapshot.json'), {
encoding: 'utf-8',
Expand All @@ -104,13 +104,9 @@ function runTests() {
})
}

describe.skip('Font optimization for SSR apps', () => {
describe('Font optimization for SSR apps', () => {
beforeAll(async () => {
await fs.writeFile(
nextConfig,
`module.exports = { experimental: {optimizeFonts: true} }`,
'utf8'
)
await fs.writeFile(nextConfig, `module.exports = {}`, 'utf8')

if (fs.pathExistsSync(join(appDir, '.next'))) {
await fs.remove(join(appDir, '.next'))
Expand All @@ -125,11 +121,11 @@ describe.skip('Font optimization for SSR apps', () => {
runTests()
})

describe.skip('Font optimization for serverless apps', () => {
describe('Font optimization for serverless apps', () => {
beforeAll(async () => {
await fs.writeFile(
nextConfig,
`module.exports = { target: 'serverless', experimental: {optimizeFonts: true} }`,
`module.exports = { target: 'serverless' }`,
'utf8'
)
await nextBuild(appDir)
Expand All @@ -142,11 +138,11 @@ describe.skip('Font optimization for serverless apps', () => {
runTests()
})

describe.skip('Font optimization for emulated serverless apps', () => {
describe('Font optimization for emulated serverless apps', () => {
beforeAll(async () => {
await fs.writeFile(
nextConfig,
`module.exports = { target: 'experimental-serverless-trace', experimental: {optimizeFonts: true} }`,
`module.exports = { target: 'experimental-serverless-trace' }`,
'utf8'
)
await nextBuild(appDir)
Expand All @@ -160,3 +156,27 @@ describe.skip('Font optimization for emulated serverless apps', () => {
})
runTests()
})

describe('Font optimization for unreachable font definitions.', () => {
beforeAll(async () => {
await fs.writeFile(nextConfig, `module.exports = { }`, 'utf8')
await nextBuild(appDir)
await fs.writeFile(
join(appDir, '.next', 'server', 'font-manifest.json'),
'[]',
'utf8'
)
appPort = await findPort()
app = await nextStart(appDir, appPort)
builtServerPagesDir = join(appDir, '.next', 'serverless')
builtPage = (file) => join(builtServerPagesDir, file)
})
afterAll(() => killApp(app))
it('should fallback to normal stylesheet if the contents of the fonts are unreachable', async () => {
const html = await renderViaHTTP(appPort, '/stars')
expect(await fsExists(builtPage('font-manifest.json'))).toBe(true)
expect(html).toContain(
'<link rel="stylesheet" href="https://fonts.googleapis.com/css2?family=Roboto:wght@700"/>'
)
})
})