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.

Sign up for GitHub

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

Jump to bottom

Current eslint rule no-page-custom-font can't handle functional _document #29970

Closed
frencojobs opened this issue Oct 16, 2021 · 2 comments · Fixed by #31560
Closed

Current eslint rule no-page-custom-font can't handle functional _document #29970

frencojobs opened this issue Oct 16, 2021 · 2 comments · Fixed by #31560
Labels
Linting Related to `next lint` or ESLint with Next.js.

Comments

@frencojobs
Copy link

frencojobs commented Oct 16, 2021 •
edited
Loading

What version of Next.js are you using?

11.1.1

What version of Node.js are you using?

14.18.0

What browser are you using?

Chrome

What operating system are you using?

macOS

How are you deploying your application?

next start

Describe the Bug

Assuming #28515 will warrant us the ability to write custom _document in functional way, the current implementation of eslint rule no-page-custom-font needs to be refactored. It seems like it is currently done via checking whether the stylesheet <link> is in a Document class or not.

next.js/packages/eslint-plugin-next/lib/rules/no-page-custom-font.js

Lines 30 to 32 in 701fa99

ancestorNode.type === 'ClassDeclaration' &&
ancestorNode.superClass &&
ancestorNode.superClass.name === documentImportName

So, for a document like this

import {Head, Html, Main, NextScript} from 'next/document'

const Document = () => (
  <Html>
    <Head>
      <link
        href='https://fonts.googleapis.com/css2?family=Inter&display=optional'
        rel='stylesheet'
      />
    </Head>
    <body>
      <Main />
      <NextScript />
    </body>
  </Html>
)

export default Document

next lint will wrongly yield a warning like this

./pages/_document.tsx
6:7  Warning: Custom fonts not added at the document level will only load for a single page. This is discouraged. See https://nextjs.org/docs/messages/no-page-custom-font.  @next/next/no-page-custom-font

Expected Behavior

next lint to not wrongly yield the warning for the functional _document.

To Reproduce

  1. Use a typical class Document with a google font <link> embedded.
  2. Run next lint and witness there's no warning.
  3. Switch the document to a functional one.
  4. Run next lint again and there should be a warning as aforementioned.
@frencojobs frencojobs added the bug Issue was opened via the bug report template. label Oct 16, 2021
@housseindjirdeh housseindjirdeh self-assigned this Oct 18, 2021
@tvarwig
Copy link

tvarwig commented Nov 9, 2021

This is happening on 12.0.3 as well

// _document.tsx
import { Head, Html, Main, NextScript } from 'next/document'

export default function Document() {
    return (
        <Html className="h-full bg-gray-100">
            <Head>
                <link rel="preconnect" href="https://fonts.googleapis.com" />
                <link rel="preconnect" href="https://fonts.gstatic.com" crossOrigin="true" />
                <link href="https://fonts.googleapis.com/css2?family=IBM+Plex+Sans+Condensed:wght@300;400;500;600;700&display=swap" rel="stylesheet" />
            </Head>
            <body className="h-full">
                <Main />
                <NextScript />
            </body>
        </Html>
    )
}
./pages/_document.tsx
9:17  Warning: Custom fonts not added at the document level will only load for a single page. This is discouraged. See https://nextjs.org/docs/messages/no-page-custom-font.  @next/next/no-page-custom-font

@timneutkens timneutkens added Linting Related to `next lint` or ESLint with Next.js. kind: bug and removed bug Issue was opened via the bug report template. labels Nov 15, 2021
@ljosberinn ljosberinn mentioned this issue Nov 18, 2021
Merged
@housseindjirdeh housseindjirdeh removed their assignment Nov 18, 2021
@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Linting Related to `next lint` or ESLint with Next.js.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: support function components in _document in no-page-custom-font
5 participants
@timneutkens @tvarwig @housseindjirdeh @frencojobs @balazsorban44