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

Font optimizations #14746

Merged
merged 74 commits into from
Jul 28, 2020
Merged
Show file tree
Hide file tree
Changes from 72 commits
Commits
Show all changes
74 commits
Select commit Hold shift + click to select a range
eff6343
adding a font gatherer webpack plugin
prateekbh Jun 15, 2020
9ed7962
fixing dep
prateekbh Jun 15, 2020
3b85140
Initial commit with framework for post processing middleware
atcastle Jun 18, 2020
3934f53
Merge branch 'post-process-optimization' of https://github.com/azukar…
prateekbh Jun 18, 2020
d5ed404
generating the style tags
prateekbh Jun 23, 2020
39fcfcd
WIP font optimizations
prateekbh Jun 24, 2020
8a932d1
working static pages font replacement
prateekbh Jun 25, 2020
126f904
minifying stylesheets
prateekbh Jun 25, 2020
069cbfa
Merge branch 'canary' of https://github.com/zeit/next.js into font-op…
prateekbh Jun 25, 2020
27bdbfe
Merge branch 'canary' of https://github.com/zeit/next.js into post-pr…
prateekbh Jun 25, 2020
6a14652
Merge branch 'post-process-optimization' into font-optims
prateekbh Jun 25, 2020
d5137c3
WIP font-optimization changes
prateekbh Jun 30, 2020
7a79523
Merge branch 'canary' of https://github.com/zeit/next.js into font-op…
prateekbh Jun 30, 2020
0f89a6c
adding comments
prateekbh Jun 30, 2020
2f45a7b
switching from filter to map
prateekbh Jun 30, 2020
6f9e1e7
fixing server path
prateekbh Jun 30, 2020
210a5cf
Fixing serverless builds
prateekbh Jun 30, 2020
ca93eba
reverting unwated change
prateekbh Jun 30, 2020
0bad5cb
removing commented code
prateekbh Jun 30, 2020
463ca85
adding experimental flag
prateekbh Jun 30, 2020
004ebcb
code resuability
prateekbh Jun 30, 2020
6acdce5
renaming flags
prateekbh Jun 30, 2020
2289980
renaming flags
prateekbh Jun 30, 2020
0d0c96d
guarding behind a flag
prateekbh Jul 1, 2020
45e7929
removing Promise.allSettled
prateekbh Jul 1, 2020
2602b31
Bug fixes and adding tests
prateekbh Jul 2, 2020
5cde0b2
sending experimental flags
prateekbh Jul 3, 2020
fcf567c
bug fixes and adding tests
prateekbh Jul 3, 2020
f70f936
adding serverless tests
prateekbh Jul 3, 2020
3ef9d55
minor improvement
prateekbh Jul 3, 2020
98e41c7
fixing experimental flag
prateekbh Jul 3, 2020
b8917c5
adding tests for serverless apps
prateekbh Jul 4, 2020
0dabf09
bug fix and adding tests for serverless
prateekbh Jul 4, 2020
9bfd563
Merge branch 'canary' of https://github.com/zeit/next.js into font-op…
prateekbh Jul 4, 2020
e17d425
uniform logic for inert font stylesheets
prateekbh Jul 5, 2020
c397f6c
sending process.env value to SSR case
prateekbh Jul 5, 2020
5f7ddd1
updating lock file
prateekbh Jul 5, 2020
a0a9c14
Merge branch 'canary' into font-optims
prateekbh Jul 5, 2020
c7fe9e9
Merge branch 'canary' of https://github.com/zeit/next.js into font-op…
prateekbh Jul 5, 2020
a21bba1
Merge branch 'font-optims' of https://github.com/azukaru/next.js into…
prateekbh Jul 5, 2020
d2be443
type fixes
prateekbh Jul 5, 2020
b7cd379
bug fix in test
prateekbh Jul 5, 2020
05cc713
bug fixes
prateekbh Jul 6, 2020
623a965
Merge branch 'canary' of https://github.com/zeit/next.js into font-op…
prateekbh Jul 6, 2020
dc83f9d
fixing serializable value bug
prateekbh Jul 6, 2020
1e23ca3
vanity changes
prateekbh Jul 6, 2020
97f89fe
query bug fixes
prateekbh Jul 6, 2020
cb3ff87
Merge branch 'canary' of https://github.com/zeit/next.js into font-op…
prateekbh Jul 6, 2020
3eaade2
moving ast types to dep
prateekbh Jul 6, 2020
6000fae
shifting font urls to a constant
prateekbh Jul 7, 2020
20ab602
removing dependency from recast
prateekbh Jul 7, 2020
91b5bb4
Merge branch 'canary' of https://github.com/zeit/next.js into font-op…
prateekbh Jul 7, 2020
cb5e320
fixing flag name and adding tests
prateekbh Jul 8, 2020
cf4d9d1
Merge branch 'canary' into font-optims
prateekbh Jul 8, 2020
bf09fcb
addressing comments
prateekbh Jul 9, 2020
bd2f156
Merge branch 'canary' of https://github.com/zeit/next.js into font-op…
prateekbh Jul 9, 2020
815d279
adding tests for emulated serverless builds
prateekbh Jul 15, 2020
58ad186
removes the need to fetch font from network in render and test fixes
prateekbh Jul 15, 2020
8de7630
Merge branch 'canary' of https://github.com/zeit/next.js into font-op…
prateekbh Jul 15, 2020
6d600ab
fixing type issues
prateekbh Jul 15, 2020
477937b
Merge branch 'canary' into font-optims
prateekbh Jul 15, 2020
a462650
fix for ie 11
prateekbh Jul 16, 2020
3762863
Merge branch 'font-optims' of https://github.com/azukaru/next.js into…
prateekbh Jul 16, 2020
af2c9d6
Merge branch 'canary' of https://github.com/zeit/next.js into font-op…
prateekbh Jul 16, 2020
45a90ed
removed the const due to tree shaking problem
prateekbh Jul 17, 2020
b3f7aea
Merge branch 'canary' of https://github.com/zeit/next.js into font-op…
prateekbh Jul 17, 2020
03eb098
increasing the error.js size as it now has filter logic for font opti…
prateekbh Jul 17, 2020
7f5680a
Merge branch 'canary' into font-optims
prateekbh Jul 17, 2020
2d77d04
Merge branch 'canary' of https://github.com/zeit/next.js into font-op…
prateekbh Jul 18, 2020
90f41fb
Update index.test.js
prateekbh Jul 20, 2020
0b6ee6b
Merge branch 'canary' of https://github.com/zeit/next.js into font-op…
prateekbh Jul 20, 2020
eeb5375
Merge branch 'font-optims' of https://github.com/azukaru/next.js into…
prateekbh Jul 20, 2020
9f7f904
Merge branch 'canary' of https://github.com/zeit/next.js into font-op…
prateekbh Jul 28, 2020
eb7f10e
Merge branch 'canary' into font-optims
prateekbh Jul 28, 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
9 changes: 8 additions & 1 deletion packages/next/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ import WebpackConformancePlugin, {
ReactSyncScriptsConformanceCheck,
} from './webpack/plugins/webpack-conformance-plugin'
import { WellKnownErrorsPlugin } from './webpack/plugins/wellknown-errors-plugin'

import FontStylesheetGatheringPlugin from './webpack/plugins/font-stylesheet-gathering-plugin'
type ExcludesFalse = <T>(x: T | false) => x is T

const isWebpack5 = parseInt(webpack.version!) === 5
Expand Down Expand Up @@ -862,6 +862,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
),
'process.env.__NEXT_SCROLL_RESTORATION': JSON.stringify(
config.experimental.scrollRestoration
),
Expand Down Expand Up @@ -967,6 +970,10 @@ export default async function getBaseWebpackConfig(
inputChunkName.replace(/\.js$/, '.module.js'),
})
})(),
config.experimental.optimizeFonts &&
!dev &&
isServer &&
new FontStylesheetGatheringPlugin(),
config.experimental.conformance &&
!isWebpack5 &&
!dev &&
Expand Down
13 changes: 12 additions & 1 deletion packages/next/build/webpack/loaders/next-serverless-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { loader } from 'webpack'
import { API_ROUTE } from '../../../lib/constants'
import {
BUILD_MANIFEST,
FONT_MANIFEST,
REACT_LOADABLE_MANIFEST,
ROUTES_MANIFEST,
} from '../../../next-server/lib/constants'
Expand Down Expand Up @@ -56,6 +57,10 @@ const nextServerlessLoader: loader.Loader = function () {
'/'
)
const routesManifest = join(distDir, ROUTES_MANIFEST).replace(/\\/g, '/')
const fontManifest = join(distDir, 'serverless', FONT_MANIFEST).replace(
/\\/g,
'/'
)

const escapedBuildId = escapeRegexp(buildId)
const pageIsDynamicRoute = isDynamicRoute(page)
Expand Down Expand Up @@ -264,7 +269,7 @@ const nextServerlessLoader: loader.Loader = function () {
}
const {parse} = require('url')
const {parse: parseQs} = require('querystring')
const {renderToHTML} = require('next/dist/next-server/server/render');
const { renderToHTML } = require('next/dist/next-server/server/render');
const { tryGetPreviewData } = require('next/dist/next-server/server/api-utils');
const {sendHTML} = require('next/dist/next-server/server/send-html');
const {sendPayload} = require('next/dist/next-server/server/send-payload');
Expand All @@ -273,6 +278,7 @@ const nextServerlessLoader: loader.Loader = function () {
const Document = require('${absoluteDocumentPath}').default;
const Error = require('${absoluteErrorPath}').default;
const App = require('${absoluteAppPath}').default;

${dynamicRouteImports}
${rewriteImports}

Expand Down Expand Up @@ -417,6 +423,11 @@ const nextServerlessLoader: loader.Loader = function () {
const previewData = tryGetPreviewData(req, res, options.previewProps)
const isPreviewMode = previewData !== false

if (process.env.__NEXT_OPTIMIZE_FONTS) {
renderOpts.optimizeFonts = true
renderOpts.fontManifest = require('${fontManifest}')
process.env['__NEXT_OPTIMIZE_FONT'+'S'] = true
}
let result = await renderToHTML(req, res, "${page}", Object.assign({}, getStaticProps ? { ...(parsedUrl.query.amp ? { amp: '1' } : {}) } : parsedUrl.query, nowParams ? nowParams : params, _params, isFallback ? { __nextFallback: 'true' } : {}), renderOpts)

if (!renderMode) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
// eslint-disable-next-line import/no-extraneous-dependencies
import { NodePath } from 'ast-types/lib/node-path'
import { compilation as CompilationType, Compiler } from 'webpack'
import { namedTypes } from 'ast-types'
import { RawSource } from 'webpack-sources'
import {
getFontDefinitionFromNetwork,
FontManifest,
} from '../../../next-server/server/font-utils'
// @ts-ignore
import BasicEvaluatedExpression from 'webpack/lib/BasicEvaluatedExpression'
import { OPTIMIZED_FONT_PROVIDERS } from '../../../next-server/lib/constants'

interface VisitorMap {
[key: string]: (path: NodePath) => void
}

export default class FontStylesheetGatheringPlugin {
compiler?: Compiler
gatheredStylesheets: Array<string> = []

private parserHandler = (
factory: CompilationType.NormalModuleFactory
): void => {
const JS_TYPES = ['auto', 'esm', 'dynamic']
// Do an extra walk per module and add interested visitors to the walk.
for (const type of JS_TYPES) {
factory.hooks.parser
.for('javascript/' + type)
.tap(this.constructor.name, (parser: any) => {
/**
* Webpack fun facts:
* `parser.hooks.call.for` cannot catch calls for user defined identifiers like `__jsx`
* it can only detect calls for native objects like `window`, `this`, `eval` etc.
* In order to be able to catch calls of variables like `__jsx`, first we need to catch them as
* Identifier and then return `BasicEvaluatedExpression` whose `id` and `type` webpack matches to
* invoke hook for call.
* See: https://github.com/webpack/webpack/blob/webpack-4/lib/Parser.js#L1931-L1932.
*/
parser.hooks.evaluate
.for('Identifier')
.tap(this.constructor.name, (node: namedTypes.Identifier) => {
// We will only optimize fonts from first party code.
if (parser?.state?.module?.resource.includes('node_modules')) {
return
}
return node.name === '__jsx'
? new BasicEvaluatedExpression()
//@ts-ignore
.setRange(node.range)
.setExpression(node)
.setIdentifier('__jsx')
: undefined
})

parser.hooks.call
.for('__jsx')
.tap(this.constructor.name, (node: namedTypes.CallExpression) => {
if (node.arguments.length !== 2) {
// A font link tag has only two arguments rel=stylesheet and href='...'
return
}
if (!isNodeCreatingLinkElement(node)) {
return
}

// node.arguments[0] is the name of the tag and [1] are the props.
const propsNode = node.arguments[1] as namedTypes.ObjectExpression
const props: { [key: string]: string } = {}
propsNode.properties.forEach((prop) => {
if (prop.type !== 'Property') {
return
}
if (
prop.key.type === 'Identifier' &&
prop.value.type === 'Literal'
) {
props[prop.key.name] = prop.value.value as string
}
})
if (
!props.rel ||
props.rel !== 'stylesheet' ||
!props.href ||
!OPTIMIZED_FONT_PROVIDERS.some((url) =>
props.href.startsWith(url)
)
) {
return false
}

this.gatheredStylesheets.push(props.href)
})
})
}
}

public apply(compiler: Compiler) {
this.compiler = compiler
compiler.hooks.normalModuleFactory.tap(
this.constructor.name,
this.parserHandler
)
compiler.hooks.make.tapAsync(this.constructor.name, (compilation, cb) => {
compilation.hooks.finishModules.tapAsync(
this.constructor.name,
async (_: any, modulesFinished: Function) => {
const fontDefinitionPromises = this.gatheredStylesheets.map((url) =>
getFontDefinitionFromNetwork(url)
)
let manifestContent: FontManifest = []

for (let promiseIndex in fontDefinitionPromises) {
manifestContent.push({
url: this.gatheredStylesheets[promiseIndex],
content: await fontDefinitionPromises[promiseIndex],
})
}
compilation.assets['font-manifest.json'] = new RawSource(
JSON.stringify(manifestContent, null, ' ')
)
modulesFinished()
}
)
cb()
})
}
}

function isNodeCreatingLinkElement(node: namedTypes.CallExpression) {
const callee = node.callee as namedTypes.Identifier
if (callee.type !== 'Identifier') {
return false
}
const componentNode = node.arguments[0] as namedTypes.Literal
if (componentNode.type !== 'Literal') {
return false
}
// Next has pragma: __jsx.
return callee.name === '__jsx' && componentNode.value === 'link'
}
1 change: 1 addition & 0 deletions packages/next/export/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,7 @@ export default async function exportApp(
subFolders,
buildExport: options.buildExport,
serverless: isTargetLikeServerless(nextConfig.target),
optimizeFonts: nextConfig.experimental.optimizeFonts,
})

for (const validation of result.ampValidations || []) {
Expand Down
35 changes: 33 additions & 2 deletions packages/next/export/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import 'next/dist/next-server/server/node-polyfill-fetch'
import { IncomingMessage, ServerResponse } from 'http'
import { ComponentType } from 'react'
import { GetStaticProps } from '../types'
import { requireFontManifest } from '../next-server/server/require'
import { FontManifest } from '../next-server/server/font-utils'

const envConfig = require('../next-server/lib/runtime-config')

Expand Down Expand Up @@ -44,6 +46,7 @@ interface ExportPageInput {
serverRuntimeConfig: string
subFolders: string
serverless: boolean
optimizeFonts: boolean
}

interface ExportPageResults {
Expand All @@ -60,6 +63,8 @@ interface RenderOpts {
ampSkipValidation?: boolean
hybridAmp?: boolean
inAmpMode?: boolean
optimizeFonts?: boolean
fontManifest?: FontManifest
}

type ComponentModule = ComponentType<{}> & {
Expand All @@ -78,6 +83,7 @@ export default async function exportPage({
serverRuntimeConfig,
subFolders,
serverless,
optimizeFonts,
}: ExportPageInput): Promise<ExportPageResults> {
let results: ExportPageResults = {
ampValidations: [],
Expand Down Expand Up @@ -211,7 +217,14 @@ export default async function exportPage({
req,
res,
'export',
{ ampPath },
{
ampPath,
/// @ts-ignore
optimizeFonts,
fontManifest: optimizeFonts
? requireFontManifest(distDir, serverless)
: null,
},
// @ts-ignore
params
)
Expand Down Expand Up @@ -246,7 +259,25 @@ export default async function exportPage({
html = components.Component
queryWithAutoExportWarn()
} else {
curRenderOpts = { ...components, ...renderOpts, ampPath, params }
/**
* This sets environment variable to be used at the time of static export 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.
*/
if (optimizeFonts) {
process.env.__NEXT_OPTIMIZE_FONTS = JSON.stringify(true)
}
curRenderOpts = {
...components,
...renderOpts,
ampPath,
params,
optimizeFonts,
fontManifest: optimizeFonts
? requireFontManifest(distDir, serverless)
: null,
}
// @ts-ignore
html = await renderMethod(req, res, page, query, curRenderOpts)
prateekbh marked this conversation as resolved.
Show resolved Hide resolved
}
Expand Down
2 changes: 2 additions & 0 deletions packages/next/next-server/lib/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export const EXPORT_DETAIL = 'export-detail.json'
export const PRERENDER_MANIFEST = 'prerender-manifest.json'
export const ROUTES_MANIFEST = 'routes-manifest.json'
export const REACT_LOADABLE_MANIFEST = 'react-loadable-manifest.json'
export const FONT_MANIFEST = 'font-manifest.json'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of adding a new manifest it'd easier to reuse the build-manifest I think 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be, however I am afraid that by writing build-manifest from multiple plugins they might overwrite each other?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't have to be multiple plugins though, we can share information into the build manifest 👍

The reason using build manifest is better is that it's automatically made available to rendering and _document, meaning you don't have to change the rendering code as much as you have to when introducing a new file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found that BuildManifestPlugin is built on client run of webpack. This is a problem because FontStylesheetGatheringPlugin runs on the server build of webpack.
The reason for keeping it on server side run is that otherwise it does not build the user defined _document.jsx which is a common place to define stylesheets and fonts and seems to be building only for server run of webpack.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timneutkens would there be a way to generate this on server run and consolidate during client bundle of webpack?

export const SERVER_DIRECTORY = 'server'
export const SERVERLESS_DIRECTORY = 'serverless'
export const CONFIG_FILE = 'next.config.js'
Expand All @@ -33,3 +34,4 @@ export const TEMPORARY_REDIRECT_STATUS = 307
export const PERMANENT_REDIRECT_STATUS = 308
export const STATIC_PROPS_ID = '__N_SSG'
export const SERVER_PROPS_ID = '__N_SSP'
export const OPTIMIZED_FONT_PROVIDERS = ['https://fonts.googleapis.com/css']
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we expect to add more optimized font providers?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, as in when requested

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Google Font has introduced a v2 API Update with supports for variable fonts

The v2 API has the new base URL: https://fonts.googleapis.com/css2

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! @wongmjane!
The usage of this const is with startswith so yeah the v2 API should already covered.
Please let us know if you find otherwise

15 changes: 15 additions & 0 deletions packages/next/next-server/lib/head.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,21 @@ function reduceComponents(
.reverse()
.map((c: React.ReactElement<any>, i: number) => {
const key = c.key || i
if (process.env.__NEXT_OPTIMIZE_FONTS) {
if (
c.type === 'link' &&
c.props['href'] &&
// TODO(prateekbh@): Replace this with const from `constants` when the tree shaking works.
['https://fonts.googleapis.com/css'].some((url) =>
c.props['href'].startsWith(url)
)
) {
const newProps = { ...(c.props || {}) }
newProps['data-href'] = newProps['href']
newProps['href'] = undefined
return React.cloneElement(c, newProps)
}
}
return React.cloneElement(c, { key })
})
}
Expand Down
Loading