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

Automatic WebFont Optimization does not work #24781

Closed
Co9xs opened this issue May 4, 2021 · 47 comments
Closed

Automatic WebFont Optimization does not work #24781

Co9xs opened this issue May 4, 2021 · 47 comments
Assignees
Labels
Font (next/font) Related to Next.js Font Optimization.

Comments

@Co9xs
Copy link

Co9xs commented May 4, 2021

What version of Next.js are you using?

10.2.0

What version of Node.js are you using?

14.15.4

What browser are you using?

Chrome

What operating system are you using?

macOS BigSur

How are you deploying your application?

Vercel

Describe the Bug

The font style is not inlined even though I added a link to_document.tsx in the following format.
(I'm sure using the Head component exported from next/document)

<Html lang="ja">
  <Head>
    <meta charSet="utf-8" />
    <link  href="https://fonts.googleapis.com/css2?family=Noto+Sans+JP" rel="stylesheet" />
  </Head>
  <body>
    <Main />
    <NextScript />
  </body>
</Html>

I forked next.js to use a local next.js, and the same thing happened.

When I run 'next build', .next/server/font-manifest.json will be generated, but its contents will be empty.
This probably happens only with certain fonts (maybe Japanese font?).

Also, if you run 'yarn dev' after 'yarn build', regardless of whether the font is Japanese or not, the .next/server/font-manifest. json was disappeared, is this a problem?

I found this issue(#19159), but it was already closed and did not solve the problem.

Expected Behavior

The tag is transformed as expected

  • link href property is replaced with data-href of the same value
  • style tag is created with the same data-href key/value
  • style tag contains an inlined font definition

The .next/server/font-manifest.json contains font definitions as fetched from https://fonts.googleapis.com/css...

To Reproduce

  1. Build and start a server from my minimal repo:
git clone https://github.com/Co9xs/next-automatic-webfont-optimization-sample.git
cd next-automatic-webfont-optimization-sample
yarn 
yarn build
yarn dev
  1. Open http://localhost:3000/
  2. Inspect the of the document
  3. Inspect the built .next/server/font-manifest.json file
@Co9xs Co9xs added the bug Issue was opened via the bug report template. label May 4, 2021
@janicklas-ralph janicklas-ralph self-assigned this May 6, 2021
@janicklas-ralph
Copy link
Contributor

Thanks for reporting this. I will have a look into it.

@timneutkens timneutkens added kind: bug and removed bug Issue was opened via the bug report template. labels May 7, 2021
@timneutkens timneutkens added this to the Iteration 20 milestone May 7, 2021
@aresrioja10
Copy link

Same issue here. Any update? @janicklas-ralph

@marcelobaez
Copy link

The same happens to me. Neither using it on _app or _document works here.
Node version: 14.16.1
Nextjs version: 10.2.3

@pixelass
Copy link
Contributor

still an issue with: "next": "^11.0.0"

@pixelass
Copy link
Contributor

pixelass commented Jun 16, 2021

Note for OP: @Co9xs

The commands you mentioned don't optimize fonts: next dev

yarn 
yarn build
yarn dev # <-- this starts a dev server and ignores the next build command

However it is also an issue with next start

yarn 
yarn build
yarn start

This is the output we see:

<head>
<link rel="stylesheet" data-href="https://fonts.googleapis.com/css2?family=Poppins">
<!-- ... -->
<link rel="stylesheet" href="https://fonts.googleapis.com/css2?family=Poppins">
</head>

The link is transformed and prepended in the head but the fonts never get inlined and therefore no optimization has been done. The original link stays in place

Actually the DOM gets larger instead, since we now have an extra "useless" <link rel="stylesheet" data-href="https://fonts.googleapis.com/css2?family=Poppins">

@pixelass
Copy link
Contributor

pixelass commented Jun 16, 2021

For us this issue happened when loading additional babel presets

FAILS

// Result
// <head>
//   <link rel="stylesheet" data-href="https://fonts.googleapis.com/css2?family=Poppins">
//   <!-- ... -->
//   <link rel="stylesheet" href="https://fonts.googleapis.com/css2?family=Poppins">
// </head>
module.exports = {
	presets: ["next/babel", "@babel/preset-env", "@babel/preset-typescript", "@babel/preset-react"],
	plugins: [
		[
			"@emotion/babel-plugin",
			{
				sourceMap: true,
				autoLabel: "dev-only",
				labelFormat: "[local]",
				cssPropOptimization: true,
			},
		],
	],
};

WORKS

// Result
// <head>
//   <link rel="stylesheet" data-href="https://fonts.googleapis.com/css2?family=Poppins">
//   <!-- ... -->
//   <style data-href="https://fonts.googleapis.com/css2?family=Poppins:wght@400;600;700&amp;display=swap">/* ... */</style>
// </head>
module.exports = {
	presets: ["next/babel",  "@babel/preset-env", ],
	plugins: [
		[
			"@emotion/babel-plugin",
			{
				sourceMap: true,
				autoLabel: "dev-only",
				labelFormat: "[local]",
				cssPropOptimization: true,
			},
		],
	],
};

@timneutkens
Copy link
Member

For us this issue happened when loading additional babel presets

FAILS

// Result
// <head>
//   <link rel="stylesheet" data-href="https://fonts.googleapis.com/css2?family=Poppins">
//   <!-- ... -->
//   <link rel="stylesheet" href="https://fonts.googleapis.com/css2?family=Poppins">
// </head>
module.exports = {
	presets: ["next/babel", "@babel/preset-env", "@babel/preset-typescript", "@babel/preset-react"],
	plugins: [
		[
			"@emotion/babel-plugin",
			{
				sourceMap: true,
				autoLabel: "dev-only",
				labelFormat: "[local]",
				cssPropOptimization: true,
			},
		],
	],
};

WORKS

// Result
// <head>
//   <link rel="stylesheet" data-href="https://fonts.googleapis.com/css2?family=Poppins">
//   <!-- ... -->
//   <style data-href="https://fonts.googleapis.com/css2?family=Poppins:wght@400;600;700&amp;display=swap">/* ... */</style>
// </head>
module.exports = {
	presets: ["next/babel",  "@babel/preset-env", ],
	plugins: [
		[
			"@emotion/babel-plugin",
			{
				sourceMap: true,
				autoLabel: "dev-only",
				labelFormat: "[local]",
				cssPropOptimization: true,
			},
		],
	],
};

presets: ["next/babel"] is only needed for the example posted as all presets referenced are already used by next/babel. Adding them on top of next/babel will have a negative effect on performance and bundle size.

@pixelass
Copy link
Contributor

@timneutkens thanks for the explanation. We carried/copied this config through several repos and versions of nextjs. Bad habits.
We kept @babel/preset-env since we add a custom config to that plugin (no issues) but looking at the docs: (https://nextjs.org/docs/advanced-features/customizing-babel-config) we will now add it to next/babel

{
  "presets": [
    [
      "next/babel",
      {
        "preset-env": {},
      }
    ]
  ],
  "plugins": []
}

@RyKilleen
Copy link

RyKilleen commented Sep 9, 2021

This appears to still be an issue, with real impact on mobile lighthouse scores. Anything we can do to help support it being fixed?

The symptoms are as described above, regarding the extra <link> not being removed on next build. Our <link>s are declared in a component in _document.tsx

@RyKilleen
Copy link

RyKilleen commented Sep 9, 2021

It appears our issue is directly related to our .babelrc as well, so it's safe to say that if you're having font optimization issues: start there.

In our case, we were including the following plugin, which is already a part of babel/preset-env, which is already part of the next/babel preset.

[
    "@babel/plugin-transform-react-jsx",
    {
        "pragmaFrag": "React.Fragment"
    }
 ]

@dasblitz
Copy link

dasblitz commented Oct 5, 2021

Ran into this issue as well using "next": "11.0.1".

I cleaned up my .babelrc and .eslintrc files according to the documentation:

.babelrc: https://nextjs.org/docs/advanced-features/customizing-babel-config
.eslintrc: https://nextjs.org/docs/basic-features/eslint

Unfortunately the font still wasn't included correctly. Turned out that for me the culprit was a key prop which I added because when running yarn dev I got the following warning:

Warning: Each child in a list should have a unique "key" prop. See https://reactjs.org/link/warning-keys for more information.
    at script
    at Head (webpack-internal:///./node_modules/next/dist/pages/_document.js:252:5)
class MyDocument extends Document {
  render(): JSX.Element {
    return (
      <Html>
        <Head>
          {/* This does not work, notice the key prop. Removing it fixed the issue for me */}
          <link
            key="does-not-work"
            href="https://fonts.googleapis.com/css2?family=Poppins:wght@700&display=swap"
            rel="stylesheet"
          />
        </Head>
        <body>
          <Main />
          <NextScript />
        </body>
      </Html>
    );
  }
}

export default MyDocument;

Removing the key prop from the <link> element added the style element to the head of the document:

<head>
  <link href="https://fonts.googleapis.com/css2?family=Poppins:wght@700&amp;display=swap" rel="stylesheet">
  <!-- ... -->
  <style data-href="https://fonts.googleapis.com/css2?family=Poppins:wght@700&amp;display=swap">@font-face{font-family:'Poppins';font-style:normal;font-weight:700;font-display:swap;src:url(https://fonts.gstatic.com/s/poppins/v15/pxiByp8kv8JHgFVrLCz7V1g.woff) format('woff')}@font-face{font-family:'Poppins';font-style:normal;font-weight:700;font-display:swap;src:url(https://fonts.gstatic.com/s/poppins/v15/pxiByp8kv8JHgFVrLCz7Z11lFd2JQEl8qw.woff2) format('woff2');unicode-range:U+0900-097F,U+1CD0-1CF6,U+1CF8-1CF9,U+200C-200D,U+20A8,U+20B9,U+25CC,U+A830-A839,U+A8E0-A8FB}@font-face{font-family:'Poppins';font-style:normal;font-weight:700;font-display:swap;src:url(https://fonts.gstatic.com/s/poppins/v15/pxiByp8kv8JHgFVrLCz7Z1JlFd2JQEl8qw.woff2) format('woff2');unicode-range:U+0100-024F,U+0259,U+1E00-1EFF,U+2020,U+20A0-20AB,U+20AD-20CF,U+2113,U+2C60-2C7F,U+A720-A7FF}@font-face{font-family:'Poppins';font-style:normal;font-weight:700;font-display:swap;src:url(https://fonts.gstatic.com/s/poppins/v15/pxiByp8kv8JHgFVrLCz7Z1xlFd2JQEk.woff2) format('woff2');unicode-range:U+0000-00FF,U+0131,U+0152-0153,U+02BB-02BC,U+02C6,U+02DA,U+02DC,U+2000-206F,U+2074,U+20AC,U+2122,U+2191,U+2193,U+2212,U+2215,U+FEFF,U+FFFD}</style>
</head>

@styfle styfle modified the milestones: 11.1.x, 12.0.4 Nov 5, 2021
@timneutkens timneutkens added the Font (next/font) Related to Next.js Font Optimization. label Nov 16, 2021
@timneutkens timneutkens removed this from the 12.0.5 milestone Nov 17, 2021
@drpancake
Copy link

Unfortunately the font still wasn't included correctly. Turned out that for me the culprit was a key prop which I added because when running yarn dev I got the following warning:

Thank you. Removing the key prop fixed it for me too.

@janicklas-ralph
Copy link
Contributor

Does it work sometimes, or it doesn't inline always? Do you see any errors on the console during next build?
I'm having a hard time reproducing the bug with the info provided. Any additional info is appreciated.

@haldunanil
Copy link

Not sure if it was specifically addressed, but upgrading to Next.js 12.1.4 seemed to fix it!

@bouncycat
Copy link

Unfortunately, @twankruiswijk 's approach didn't work for me as my Document component is already a class. For reference, it doesn't work with this:

import React from "react";
import Document, { Html, Head, Main, NextScript } from "next/document";
import { ServerStyleSheet } from "styled-components";
import ServerStyleSheets from "@mui/styles/ServerStyleSheets";

class MyDocument extends Document {
  static async getInitialProps(
    // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
    ctx
  ): Promise<{
    styles: JSX.Element;
    html: string;
    head?: JSX.Element[];
  }> {
    const styledComponentsSheet = new ServerStyleSheet();
    const materialSheets = new ServerStyleSheets();
    const originalRenderPage = ctx.renderPage;

    try {
      ctx.renderPage = () =>
        originalRenderPage({
          enhanceApp: (App) => (props) =>
            styledComponentsSheet.collectStyles(
              materialSheets.collect(<App {...props} />)
            ),
        });

      const initialProps = await Document.getInitialProps(ctx);

      return {
        ...initialProps,
        styles: (
          <>
            {initialProps.styles}
            {materialSheets.getStyleElement()}
            {styledComponentsSheet.getStyleElement()}
          </>
        ),
      } as any;
    } finally {
      styledComponentsSheet.seal();
    }
  }

  render(): React.ReactElement {
    return (
      <Html lang="en">
        <Head>
          <link rel="preconnect" href="https://fonts.googleapis.com" />
          <link
            rel="preconnect"
            href="https://fonts.gstatic.com"
            crossOrigin=""
          />
          <link
            href="https://fonts.googleapis.com/css2?family=Libre+Franklin:wght@300;400;500;700&display=swap"
            rel="stylesheet"
          />
        </Head>
        <body>
          <Main />
          <NextScript />
        </body>
      </Html>
    );
  }
}

export default MyDocument;

I am encountering exactly the same problem with using Class and styled-component together inside getInitialProps, also unfortunately upgrading to next@12.1.4 does not fix the problem

@MakChan
Copy link

MakChan commented Apr 8, 2022

Does it work sometimes, or it doesn't inline always? Do you see any errors on the console during next build? I'm having a hard time reproducing the bug with the info provided. Any additional info is appreciated.

No, it never inlines, and there are no errors or warnings during next build.

I'm also using a Class component and next@12.1.4. The document page itself doesn't have a getInitialProps but individual pages do have getServerSideProps.

@janicklas-ralph
Copy link
Contributor

Thanks.
I tried with your version and I still haven't been able to recreate this issue. Can you check if there is a font-manifest.json in your build folder, and share the contents.

Also, what version of react are you using?

@iMohammadReza
Copy link

Still experiencing this with 12.1.6. Font optimization breaks in pages with getServerSideProps!

@alex-statsig
Copy link

This seemed to start happening to me after upgrading to react 18. Ended up disabling fontOptimization to fix for now

baptisteArno added a commit to baptisteArno/typebot.io that referenced this issue May 14, 2022
@baptisteArno
Copy link

I'm experiencing the same issue on Typebot after upgrading packages (React 18 and Next.js 12.1.6)

@zbauman3
Copy link
Contributor

I am also having this issue. Here's the setup:

// fonts.ts
export const fonts = [
  {
    key: "font1",
    rel: "stylesheet",
    href: "https://fonts.googleapis.com/css2?family=Montserrat:wght@600&family=Open+Sans:wght@400;600&display=swap"
  },
 // ... more fonts
];
export const fontComponents = fonts.map((font) => (React.createElement("link", { key: font.key, rel: font.rel, href: font.href, className: 'someclass' })))
// _document.tsx
...
import { fontComponents } from '../some/path/to/fonts.ts'
...
const Document = (): React.ReactElement => {
  return (
    <Html>
      <Head>{fontComponents}</Head>
      <body>
        <Main />
        <NextScript />
      </body>
    </Html>
  )
}

I've tried:

  • Disabling serverside/static/initial props
  • Returning a single hard-coded component from fonts.ts
  • Returning a Fragment from fonts.ts
  • Updating to v12.1.6
  • Using a class component for _document.tsx
  • I'm not using any extra Babel config

In all scenarios I'm seeing an output of:

<head>
  <link rel="stylesheet" data-href="https://fonts.googleapis.com/css2?family=...">
  ...
  <link rel="stylesheet" href="https://fonts.googleapis.com/css2?family=...">
</head>

The only solutions that does work is when I code the fonts directly into _document.tsx, which does not work for this setup.

@zbauman3
Copy link
Contributor

zbauman3 commented May 20, 2022

The most progress I've made is that it seems my elements are getting filtered out at/around here. It seems that isNodeCreatingLinkElement may be causing some of the issues people are having above, although I'm not sure that it is my issue exactly. I'll have more time to research this weekend.

I hope this helps some people track down their issues!

@aoshi321
Copy link

Any update in regards to this issue Im still experiencing this with React 18 and the latest version of next?

@benrychter
Copy link

Same issue: Next 12.1.6 and React 18.1.0. Custom _document is a React Class Component.

@ardokirsipuu
Copy link

ardokirsipuu commented May 31, 2022

I'm also experiencing this issue. Found a minimal setup that only requires:

Steps to reproduce:

  1. Run yarn create next-app
  2. In styles/global.css change font-family to Rubik
  3. In pages/index.js add this to the end:
export const getServerSideProps = async () => {
  return { props: {} };
};
  1. Add the following _document.js under pages/:
import Document, { Html, Head, Main, NextScript } from "next/document";

class MyDocument extends Document {
  render() {
    return (
      <Html>
        <Head>
          <link
            href="https://fonts.googleapis.com/css2?family=Rubik:wght@300;400;500;700&display=swap"
            rel="stylesheet"
          />
        </Head>
        <body>
          <Main />
          <NextScript />
        </body>
      </Html>
    );
  }
}

export default MyDocument;
  1. Run yarn build && yarn start
  2. Navigate to http://localhost:3000/
  3. Inspect and see how it uses the default font for everything instead of Rubik

@ghost
Copy link

ghost commented Jun 9, 2022

const Page = () => (
  <>
    <NextHead>
      <link // eslint-disable-line
        href="https://fonts.googleapis.com/css2?family=Inter:wght@400..800&display=swap"
        rel="stylesheet"
      />
    </NextHead>
  </>
)

export async function getServerSideProps(context) {
  return {
    props: {},
  };
}

export default Page;

^ Currently using this hotfix: adding the <link> tag to your Next.js page with getServerSideProps

@xereda
Copy link

xereda commented Jun 15, 2022

const Page = () => (
  <>
    <NextHead>
      <link // eslint-disable-line
        href="https://fonts.googleapis.com/css2?family=Inter:wght@400..800&display=swap"
        rel="stylesheet"
      />
    </NextHead>
  </>
)

export async function getServerSideProps(context) {
  return {
    props: {},
  };
}

export default Page;

^ Currently using this hotfix: adding the <link> tag to your Next.js page with getServerSideProps

Doesn't it make more sense to turn off the functionality?

@tomchentw
Copy link

tomchentw commented Jun 30, 2022

It seems broken for the pages with getServerSideProps. For ISR pages with getStaticProps or even static pages with no data, they work perfectly.

@JuniorTour
Copy link
Contributor

This issue seems to have been fixed by "next": "12.2.0".

I just test the case above with:

  • getServerSideProps
  • <Head><link></Head>

@ahmedB7r
Copy link

ahmedB7r commented Jul 9, 2022

i have same issue with writing the link in this dynamic form

<link href={`https://fonts.googleapis.com/css2?family=${fontFamily}:wght@500;700&display=optional`}
        rel="stylesheet"
           />

and just get this

<link rel="stylesheet" data-href="https://fonts.googleapis.com/css2?family=Cairo:wght@500;700&amp;display=optional">

work normally if write the href without any variables

maybe help

@xereda
Copy link

xereda commented Jul 22, 2022

NEWS! It was resolved with the Next.js update

@ahmedB7r
Copy link

i have same issue with writing the link in this dynamic form

<link href={`https://fonts.googleapis.com/css2?family=${fontFamily}:wght@500;700&display=optional`}
        rel="stylesheet"
           />

and just get this

<link rel="stylesheet" data-href="https://fonts.googleapis.com/css2?family=Cairo:wght@500;700&amp;display=optional">

work normally if write the href without any variables

maybe help

my case still not working

tourist added a commit to tourist/coinscan-front that referenced this issue Sep 21, 2022
@TrejoCode
Copy link

Same error, doesn't load on 12.3.1

@0xmax
Copy link

0xmax commented Oct 12, 2022

Same here, any updates on this?

@kfranqueiro
Copy link

kfranqueiro commented Mar 21, 2023

I also ran into this issue of Next 12 never properly inlining google font resources, even when it's nothing but a <link rel="stylesheet" href="..."> (i.e. no variables or spreads involved) inside of <Head>.

I dug around for a while and was able to get it working with this patch. I won't pretend to understand the details of the AST, but apparently the nodes of interest were neither _jsx nor __jsx, but just plain jsx for me. (Next 12.3.4, React 17.0.2)

diff --git a/node_modules/next/dist/build/webpack/plugins/font-stylesheet-gathering-plugin.js b/node_modules/next/dist/build/webpack/plugins/font-stylesheet-gathering-plugin.js
index 1ebc70a..d9050fe 100644
--- a/node_modules/next/dist/build/webpack/plugins/font-stylesheet-gathering-plugin.js
+++ b/node_modules/next/dist/build/webpack/plugins/font-stylesheet-gathering-plugin.js
@@ -76,7 +76,7 @@ function isNodeCreatingLinkElement(node) {
     }
     // React has pragma: _jsx.
     // Next has pragma: __jsx.
-    return (callee.name === "_jsx" || callee.name === "__jsx") && componentNode.value === "link";
+    return /^_*jsx$/.test(callee.name) && componentNode.value === "link";
 }
 class FontStylesheetGatheringPlugin {
     gatheredStylesheets = [];
@@ -109,7 +109,7 @@ class FontStylesheetGatheringPlugin {
                         return;
                     }
                     let result;
-                    if (node.name === "_jsx" || node.name === "__jsx") {
+                    if (/^_*jsx$/.test(node.name)) {
                         result = new _webpack.BasicEvaluatedExpression();
                         // @ts-ignore
                         result.setRange(node.range);
@@ -153,6 +153,7 @@ class FontStylesheetGatheringPlugin {
                     }
                 };
                 // React JSX transform:
+                parser.hooks.call.for("jsx").tap(this.constructor.name, jsxNodeHandler);
                 parser.hooks.call.for("_jsx").tap(this.constructor.name, jsxNodeHandler);
                 // Next.js JSX transform:
                 parser.hooks.call.for("__jsx").tap(this.constructor.name, jsxNodeHandler);

@timneutkens
Copy link
Member

Going to close this issue as automatic font optimization was superseded by next/font: https://nextjs.org/docs/basic-features/font-optimization

@github-actions
Copy link
Contributor

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Font (next/font) Related to Next.js Font Optimization.
Projects
None yet
Development

No branches or pull requests