From 57e156bc49024fda19ffdffb1ed4befc4a07c2c3 Mon Sep 17 00:00:00 2001 From: Prateek Bhatnagar Date: Fri, 4 Dec 2020 01:52:54 -0800 Subject: [PATCH] Making font optimization as default (#19758) - Making font optimizations as default - Re-enabling tests - Fixes #19159 --- packages/next/build/webpack-config.ts | 7 +--- .../next-serverless-loader/page-handler.ts | 15 +++---- packages/next/export/index.ts | 1 - packages/next/export/worker.ts | 23 +--------- packages/next/next-server/lib/head.tsx | 2 +- packages/next/next-server/lib/post-process.ts | 27 ++++++------ packages/next/next-server/server/config.ts | 1 - .../next/next-server/server/next-server.ts | 16 +++---- packages/next/next-server/server/render.tsx | 22 ++++------ packages/next/pages/_document.tsx | 4 +- .../build-output/test/index.test.js | 4 +- .../font-optimization/test/index.test.js | 42 ++++++++++++++----- .../script-loader/test/index.test.js | 5 --- 13 files changed, 74 insertions(+), 95 deletions(-) diff --git a/packages/next/build/webpack-config.ts b/packages/next/build/webpack-config.ts index 7d32a355b3..a6369a54be 100644 --- a/packages/next/build/webpack-config.ts +++ b/packages/next/build/webpack-config.ts @@ -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 ), @@ -1077,8 +1074,7 @@ export default async function getBaseWebpackConfig( new ProfilingPlugin({ tracer, }), - config.experimental.optimizeFonts && - !dev && + !dev && isServer && (function () { const { @@ -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, diff --git a/packages/next/build/webpack/loaders/next-serverless-loader/page-handler.ts b/packages/next/build/webpack/loaders/next-serverless-loader/page-handler.ts index 4f7ce8b3cf..a991aea988 100644 --- a/packages/next/build/webpack/loaders/next-serverless-loader/page-handler.ts +++ b/packages/next/build/webpack/loaders/next-serverless-loader/page-handler.ts @@ -370,15 +370,12 @@ 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, diff --git a/packages/next/export/index.ts b/packages/next/export/index.ts index 26eeb6d3a8..7256aac7e5 100644 --- a/packages/next/export/index.ts +++ b/packages/next/export/index.ts @@ -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, }) diff --git a/packages/next/export/worker.ts b/packages/next/export/worker.ts index b9e0808ead..c4fdac7c7f 100644 --- a/packages/next/export/worker.ts +++ b/packages/next/export/worker.ts @@ -47,7 +47,6 @@ interface ExportPageInput { serverRuntimeConfig: string subFolders: string serverless: boolean - optimizeFonts: boolean optimizeImages: boolean optimizeCss: any } @@ -67,7 +66,6 @@ interface RenderOpts { ampSkipValidation?: boolean hybridAmp?: boolean inAmpMode?: boolean - optimizeFonts?: boolean optimizeImages?: boolean optimizeCss?: any fontManifest?: FontManifest @@ -92,7 +90,6 @@ export default async function exportPage({ serverRuntimeConfig, subFolders, serverless, - optimizeFonts, optimizeImages, optimizeCss, }: ExportPageInput): Promise { @@ -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!, }, @@ -295,15 +288,6 @@ 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) } @@ -315,12 +299,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 diff --git a/packages/next/next-server/lib/head.tsx b/packages/next/next-server/lib/head.tsx index c10e61b5e6..fd185a038b 100644 --- a/packages/next/next-server/lib/head.tsx +++ b/packages/next/next-server/lib/head.tsx @@ -136,7 +136,7 @@ function reduceComponents( .reverse() .map((c: React.ReactElement, 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'] && diff --git a/packages/next/next-server/lib/post-process.ts b/packages/next/next-server/lib/post-process.ts index fcbefe997a..d20b9ba42d 100644 --- a/packages/next/next-server/lib/post-process.ts +++ b/packages/next/next-server/lib/post-process.ts @@ -6,7 +6,6 @@ const MAXIMUM_IMAGE_PRELOADS = 2 const IMAGE_PRELOAD_SIZE_THRESHOLD = 2500 type postProcessOptions = { - optimizeFonts: boolean optimizeImages: boolean } @@ -143,10 +142,20 @@ class FontOptimizerMiddleware implements PostProcessMiddleware { continue } const fontContent = options.getFontDefinition(url) - result = result.replace( - '', - `` - ) + if (!fontContent) { + /** + * In case of unreachable font definitions, fallback to default link tag. + */ + result = result.replace( + '', + `` + ) + } else { + result = result.replace( + '', + `` + ) + } } return result } @@ -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', diff --git a/packages/next/next-server/server/config.ts b/packages/next/next-server/server/config.ts index cb155a9283..1e2c693360 100644 --- a/packages/next/next-server/server/config.ts +++ b/packages/next/next-server/server/config.ts @@ -53,7 +53,6 @@ const defaultConfig: { [key: string]: any } = { workerThreads: false, pageEnv: false, productionBrowserSourceMaps: false, - optimizeFonts: false, optimizeImages: false, optimizeCss: false, scrollRestoration: false, diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index 029f9d2db0..a456335901 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -147,7 +147,6 @@ export default class Server { customServer?: boolean ampOptimizerConfig?: { [key: string]: any } basePath: string - optimizeFonts: boolean images: string fontManifest: FontManifest optimizeImages: boolean @@ -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, } @@ -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) } diff --git a/packages/next/next-server/server/render.tsx b/packages/next/next-server/server/render.tsx index 3bccf448df..9f98f1741e 100644 --- a/packages/next/next-server/server/render.tsx +++ b/packages/next/next-server/server/render.tsx @@ -162,7 +162,6 @@ export type RenderOptsPartial = { previewProps: __ApiPreviewProps basePath: string unstable_runtimeJS?: false - optimizeFonts: boolean fontManifest?: FontManifest optimizeImages: boolean optimizeCss: any @@ -1051,18 +1050,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 diff --git a/packages/next/pages/_document.tsx b/packages/next/pages/_document.tsx index 52db9536db..28e5911241 100644 --- a/packages/next/pages/_document.tsx +++ b/packages/next/pages/_document.tsx @@ -212,7 +212,7 @@ export class Head extends Component< ) }) - if (process.env.__NEXT_OPTIMIZE_FONTS) { + if (process.env.NODE_ENV !== 'development') { cssLinkElements = this.makeStylesheetInert( cssLinkElements ) as ReactElement[] @@ -369,7 +369,7 @@ export class Head extends Component< ) } - if (process.env.__NEXT_OPTIMIZE_FONTS && !inAmpMode) { + if (process.env.NODE_ENV !== 'development' && !inAmpMode) { children = this.makeStylesheetInert(children) } diff --git a/test/integration/build-output/test/index.test.js b/test/integration/build-output/test/index.test.js index 3512b72b52..e214f2cb91 100644 --- a/test/integration/build-output/test/index.test.js +++ b/test/integration/build-output/test/index.test.js @@ -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) diff --git a/test/integration/font-optimization/test/index.test.js b/test/integration/font-optimization/test/index.test.js index c775f49787..fe08fa948f 100644 --- a/test/integration/font-optimization/test/index.test.js +++ b/test/integration/font-optimization/test/index.test.js @@ -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', @@ -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')) @@ -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) @@ -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) @@ -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( + '' + ) + }) +}) diff --git a/test/integration/script-loader/test/index.test.js b/test/integration/script-loader/test/index.test.js index a2a764baad..51b719f951 100644 --- a/test/integration/script-loader/test/index.test.js +++ b/test/integration/script-loader/test/index.test.js @@ -112,11 +112,6 @@ describe('Script Loader', () => { `link[rel=stylesheet][href^="/_next/static/css"] ~ link[rel=preload][href="${src}"]` ).length ).toBeGreaterThan(0) - expect( - $( - `link[rel=stylesheet][href="https://fonts.googleapis.com/css?family=Voces"] ~ link[rel=preload][href="${src}"]` - ).length - ).toBeGreaterThan(0) // Script is inserted before NextScripts expect( -- GitLab