Skip to content

Commit

Permalink
temporary: re-flag font optimization (vercel#20372)
Browse files Browse the repository at this point in the history
There's currently two bugs with the font optimization, but we'd really like to ship a stable version.

To unblock the stable release, we're **temporarily** reflagging this. It'll be unflagged on canary again!
  • Loading branch information
Timer authored Dec 21, 2020
1 parent a9f1975 commit 260ab51
Show file tree
Hide file tree
Showing 15 changed files with 112 additions and 40 deletions.
23 changes: 14 additions & 9 deletions packages/next/build/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { loadEnvConfig } from '@next/env'
import chalk from 'chalk'
import crypto from 'crypto'
import { promises, writeFileSync } from 'fs'
import Worker from 'jest-worker'
import chalk from 'chalk'
import devalue from 'next/dist/compiled/devalue'
import escapeStringRegexp from 'next/dist/compiled/escape-string-regexp'
import findUp from 'next/dist/compiled/find-up'
Expand All @@ -21,7 +22,7 @@ import loadCustomRoutes, {
Redirect,
RouteType,
} from '../lib/load-custom-routes'
import { loadEnvConfig } from '@next/env'
import { nonNullable } from '../lib/non-nullable'
import { recursiveDelete } from '../lib/recursive-delete'
import { verifyTypeScriptSetup } from '../lib/verifyTypeScriptSetup'
import {
Expand Down Expand Up @@ -66,7 +67,9 @@ import { CompilerResult, runCompiler } from './compiler'
import { createEntrypoints, createPagesMapping } from './entries'
import { generateBuildId } from './generate-build-id'
import { isWriteable } from './is-writeable'
import * as Log from './output/log'
import createSpinner from './spinner'
import { traceAsyncFn, tracer } from './tracer'
import {
collectPages,
getJsPageSizeInKb,
Expand All @@ -80,8 +83,6 @@ import {
import getBaseWebpackConfig from './webpack-config'
import { PagesManifest } from './webpack/plugins/pages-manifest-plugin'
import { writeBuildId } from './write-build-id'
import * as Log from './output/log'
import { tracer, traceAsyncFn } from './tracer'

const staticCheckWorker = require.resolve('./utils')

Expand Down Expand Up @@ -372,12 +373,16 @@ export default async function build(
BUILD_MANIFEST,
PRERENDER_MANIFEST,
REACT_LOADABLE_MANIFEST,
path.join(
isLikeServerless ? SERVERLESS_DIRECTORY : SERVER_DIRECTORY,
FONT_MANIFEST
),
config.experimental.optimizeFonts
? path.join(
isLikeServerless ? SERVERLESS_DIRECTORY : SERVER_DIRECTORY,
FONT_MANIFEST
)
: null,
BUILD_ID_FILE,
].map((file) => path.join(config.distDir, file)),
]
.filter(nonNullable)
.map((file) => path.join(config.distDir, file)),
ignore: [
path.relative(
dir,
Expand Down
7 changes: 6 additions & 1 deletion packages/next/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -962,6 +962,9 @@ 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 @@ -1066,7 +1069,8 @@ export default async function getBaseWebpackConfig(
new ProfilingPlugin({
tracer,
}),
!dev &&
config.experimental.optimizeFonts &&
!dev &&
isServer &&
(function () {
const {
Expand Down Expand Up @@ -1154,6 +1158,7 @@ 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 @@ -279,12 +279,17 @@ export function getPageHandler(ctx: ServerlessHandlerCtx) {

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

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__
}

let result = await renderToHTML(
req,
res,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,9 @@ export class FontStylesheetGatheringPlugin {
isWebpack5 ? '__webpack_require__' : mainTemplate.requireFn
}.__NEXT_FONT_MANIFEST__ = ${JSON.stringify(
this.manifestContent
)};`
)};
// Enable feature:
process.env.__NEXT_OPTIMIZE_FONTS = JSON.stringify(true);`
}
)
}
Expand Down
1 change: 1 addition & 0 deletions packages/next/export/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,7 @@ 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: 21 additions & 2 deletions packages/next/export/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ interface ExportPageInput {
serverRuntimeConfig: string
subFolders: string
serverless: boolean
optimizeFonts: boolean
optimizeImages: boolean
optimizeCss: any
}
Expand All @@ -66,6 +67,7 @@ interface RenderOpts {
ampSkipValidation?: boolean
hybridAmp?: boolean
inAmpMode?: boolean
optimizeFonts?: boolean
optimizeImages?: boolean
optimizeCss?: any
fontManifest?: FontManifest
Expand All @@ -90,6 +92,7 @@ export default async function exportPage({
serverRuntimeConfig,
subFolders,
serverless,
optimizeFonts,
optimizeImages,
optimizeCss,
}: ExportPageInput): Promise<ExportPageResults> {
Expand Down Expand Up @@ -247,10 +250,14 @@ export default async function exportPage({
{
ampPath: renderAmpPath,
/// @ts-ignore
optimizeFonts,
/// @ts-ignore
optimizeImages,
/// @ts-ignore
optimizeCss,
fontManifest: requireFontManifest(distDir, serverless),
fontManifest: optimizeFonts
? requireFontManifest(distDir, serverless)
: null,
locale: locale!,
locales: renderOpts.locales!,
},
Expand Down Expand Up @@ -288,6 +295,15 @@ export default async function exportPage({
html = components.Component
queryWithAutoExportWarn()
} 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.
*/
if (optimizeFonts) {
process.env.__NEXT_OPTIMIZE_FONTS = JSON.stringify(true)
}
if (optimizeImages) {
process.env.__NEXT_OPTIMIZE_IMAGES = JSON.stringify(true)
}
Expand All @@ -299,9 +315,12 @@ export default async function exportPage({
...renderOpts,
ampPath: renderAmpPath,
params,
optimizeFonts,
optimizeImages,
optimizeCss,
fontManifest: requireFontManifest(distDir, serverless),
fontManifest: optimizeFonts
? requireFontManifest(distDir, serverless)
: null,
locale: locale as string,
}
// @ts-ignore
Expand Down
3 changes: 3 additions & 0 deletions packages/next/lib/non-nullable.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export function nonNullable<T>(value: T): value is NonNullable<T> {
return value !== null && value !== undefined
}
6 changes: 5 additions & 1 deletion packages/next/next-server/lib/head.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,11 @@ function reduceComponents(
.reverse()
.map((c: React.ReactElement<any>, i: number) => {
const key = c.key || i
if (process.env.NODE_ENV !== 'development' && !props.inAmpMode) {
if (
process.env.NODE_ENV !== 'development' &&
process.env.__NEXT_OPTIMIZE_FONTS &&
!props.inAmpMode
) {
if (
c.type === 'link' &&
c.props['href'] &&
Expand Down
9 changes: 8 additions & 1 deletion packages/next/next-server/lib/post-process.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const MAXIMUM_IMAGE_PRELOADS = 2
const IMAGE_PRELOAD_SIZE_THRESHOLD = 2500

type postProcessOptions = {
optimizeFonts: boolean
optimizeImages: boolean
}

Expand Down Expand Up @@ -261,7 +262,13 @@ function sourceIsSupportedType(imgSrc: string): boolean {
}

// Initialization
registerPostProcessor('Inline-Fonts', new FontOptimizerMiddleware(), () => true)
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(
'Preload Images',
Expand Down
1 change: 1 addition & 0 deletions packages/next/next-server/server/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ const defaultConfig: NextConfig = {
reactMode: 'legacy',
workerThreads: false,
pageEnv: false,
optimizeFonts: false,
optimizeImages: false,
optimizeCss: false,
scrollRestoration: false,
Expand Down
12 changes: 9 additions & 3 deletions packages/next/next-server/server/next-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ 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,9 +203,11 @@ export default class Server {
ampOptimizerConfig: this.nextConfig.experimental.amp?.optimizer,
basePath: this.nextConfig.basePath,
images: JSON.stringify(this.nextConfig.images),
fontManifest: !dev
? requireFontManifest(this.distDir, this._isLikeServerless)
: null,
optimizeFonts: this.nextConfig.experimental.optimizeFonts && !dev,
fontManifest:
this.nextConfig.experimental.optimizeFonts && !dev
? requireFontManifest(this.distDir, this._isLikeServerless)
: null,
optimizeImages: this.nextConfig.experimental.optimizeImages,
optimizeCss: this.nextConfig.experimental.optimizeCss,
}
Expand Down Expand Up @@ -269,6 +272,9 @@ export default class Server {
* `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
20 changes: 11 additions & 9 deletions packages/next/next-server/server/render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ export type RenderOptsPartial = {
previewProps: __ApiPreviewProps
basePath: string
unstable_runtimeJS?: false
optimizeFonts: boolean
fontManifest?: FontManifest
optimizeImages: boolean
optimizeCss: any
Expand Down Expand Up @@ -1050,15 +1051,16 @@ export async function renderToHTML(
}

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

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

if (process.env.NODE_ENV !== 'development') {
if (
process.env.NODE_ENV !== 'development' &&
process.env.__NEXT_OPTIMIZE_FONTS
) {
cssLinkElements = this.makeStylesheetInert(
cssLinkElements
) as ReactElement[]
Expand Down Expand Up @@ -369,7 +372,11 @@ export class Head extends Component<
)
}

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

Expand Down
2 changes: 1 addition & 1 deletion test/integration/build-output/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ describe('Build Output', () => {
expect(parseFloat(err404Size) - 3.7).toBeLessThanOrEqual(0)
expect(err404Size.endsWith('kB')).toBe(true)

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

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

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

if (fs.pathExistsSync(join(appDir, '.next'))) {
await fs.remove(join(appDir, '.next'))
Expand All @@ -125,7 +129,7 @@ describe('Font optimization for serverless apps', () => {
beforeAll(async () => {
await fs.writeFile(
nextConfig,
`module.exports = { target: 'serverless' }`,
`module.exports = { target: 'serverless', experimental: {optimizeFonts: true} }`,
'utf8'
)
await nextBuild(appDir)
Expand All @@ -142,16 +146,17 @@ describe('Font optimization for emulated serverless apps', () => {
beforeAll(async () => {
await fs.writeFile(
nextConfig,
`module.exports = { target: 'experimental-serverless-trace' }`,
`module.exports = { target: 'experimental-serverless-trace', experimental: {optimizeFonts: true} }`,
'utf8'
)
await nextBuild(appDir)
appPort = await findPort()
await startServerlessEmulator(appDir, appPort)
app = await startServerlessEmulator(appDir, appPort)
builtServerPagesDir = join(appDir, '.next', 'serverless')
builtPage = (file) => join(builtServerPagesDir, file)
})
afterAll(async () => {
await killApp(app)
await fs.remove(nextConfig)
})
runTests()
Expand Down

0 comments on commit 260ab51

Please sign in to comment.