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

fix: support function components in _document in no-page-custom-font #31560

Merged
merged 9 commits into from
Nov 26, 2021
30 changes: 28 additions & 2 deletions errors/no-page-custom-font.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

### Why This Error Occurred

A custom font was added to a page and not with a custom `Document`. This only adds the font to the specific page and not to the entire application.
- The custom font you're adding was added to a page - this only adds the font to the specific page and not the entire application.
- The custom font you're adding was added to a separate component within `Document` - this disables automatic font optimiztion.

### Possible Ways to Fix It

Expand Down Expand Up @@ -35,9 +36,34 @@ class MyDocument extends Document {
export default MyDocument
```

Or as a function component:

```js
// pages/_document.js

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

export default function Document() {
return (
<Html>
<Head>
<link
href="https://fonts.googleapis.com/css2?family=Inter&display=optional"
rel="stylesheet"
/>
</Head>
<body>
<Main />
<NextScript />
</body>
</Html>
)
}
```

### When Not To Use It

If you have a reason to only load a font for a particular page, then you can disable this rule.
If you have a reason to only load a font for a particular page or don't care about font optimization, then you can disable this rule.

### Useful Links

Expand Down
116 changes: 106 additions & 10 deletions packages/eslint-plugin-next/lib/rules/no-page-custom-font.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const NodeAttributes = require('../utils/node-attributes.js')
const { sep, posix } = require('path')

module.exports = {
meta: {
Expand All @@ -10,7 +11,22 @@ module.exports = {
},
},
create: function (context) {
const paths = context.getFilename().split('pages')
const page = paths[paths.length - 1]

// outside of a file within `pages`, bail
if (!page) {
return {}
}

const is_Document =
page.startsWith(`${sep}_document`) ||
page.startsWith(`${posix.sep}_document`)

let documentImportName
let localDefaultExportName
let exportDeclarationType

return {
ImportDeclaration(node) {
if (node.source.value === 'next/document') {
Expand All @@ -22,17 +38,91 @@ module.exports = {
}
}
},

ExportDefaultDeclaration(node) {
exportDeclarationType = node.declaration.type

if (node.declaration.type === 'FunctionDeclaration') {
localDefaultExportName = node.declaration.id.name
return
}

if (
node.declaration.type === 'ClassDeclaration' &&
node.declaration.superClass &&
node.declaration.superClass.name === documentImportName
) {
localDefaultExportName = node.declaration.id.name
}
},

JSXOpeningElement(node) {
const documentClass = context
.getAncestors()
.find(
(ancestorNode) =>
ancestorNode.type === 'ClassDeclaration' &&
ancestorNode.superClass &&
ancestorNode.superClass.name === documentImportName
if (node.name.name !== 'link') {
return
}

const ancestors = context.getAncestors()

// if `export default <name>` is further down within the file after the
// currently traversed component, then `localDefaultExportName` will
// still be undefined
if (!localDefaultExportName) {
// find the top level of the module
const program = ancestors.find(
(ancestor) => ancestor.type === 'Program'
)

if (documentClass || node.name.name !== 'link') {
// go over each token to find the combination of `export default <name>`
for (let i = 0; i <= program.tokens.length - 1; i++) {
if (localDefaultExportName) {
break
}

const token = program.tokens[i]

if (token.type === 'Keyword' && token.value === 'export') {
const nextToken = program.tokens[i + 1]

if (
nextToken &&
nextToken.type === 'Keyword' &&
nextToken.value === 'default'
) {
const maybeIdentifier = program.tokens[i + 2]

if (maybeIdentifier && maybeIdentifier.type === 'Identifier') {
localDefaultExportName = maybeIdentifier.value
}
}
}
}
}

const parentComponent = ancestors.find((ancestor) => {
// export default class ... extends ...
if (exportDeclarationType === 'ClassDeclaration') {
return (
ancestor.type === exportDeclarationType &&
ancestor.superClass &&
ancestor.superClass.name === documentImportName
)
}

// export default function ...
if (exportDeclarationType === 'FunctionDeclaration') {
return (
ancestor.type === exportDeclarationType &&
ancestor.id.name === localDefaultExportName
)
}

// function ...() {} export default ...
// class ... extends ...; export default ...
return ancestor.id && ancestor.id.name === localDefaultExportName
})

// file starts with _document and this <link /> is within the default export
if (is_Document && parentComponent) {
ljosberinn marked this conversation as resolved.
Show resolved Hide resolved
return
}

Expand All @@ -47,10 +137,16 @@ module.exports = {
hrefValue.startsWith('https://fonts.googleapis.com/css')

if (isGoogleFont) {
const end =
'This is discouraged. See https://nextjs.org/docs/messages/no-page-custom-font.'

const message = is_Document
? `Rendering this <link /> not inline within <Head> of Document disables font optimization. ${end}`
: `Custom fonts not added at the document level will only load for a single page. ${end}`

context.report({
node,
message:
'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.',
message,
})
}
},
Expand Down
126 changes: 99 additions & 27 deletions test/unit/eslint-plugin-next/no-page-custom-font.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@ import { RuleTester } from 'eslint'
})
const ruleTester = new RuleTester()

const filename = 'pages/_document.js'

ruleTester.run('no-page-custom-font', rule, {
valid: [
`import Document, { Html, Head } from "next/document";

{
code: `import Document, { Html, Head } from "next/document";
class MyDocument extends Document {
render() {
return (
Expand All @@ -30,11 +32,11 @@ ruleTester.run('no-page-custom-font', rule, {
);
}
}

export default MyDocument;
`,
`import NextDocument, { Html, Head } from "next/document";

export default MyDocument;`,
filename,
},
{
code: `import NextDocument, { Html, Head } from "next/document";
class Document extends NextDocument {
render() {
return (
Expand All @@ -49,16 +51,70 @@ ruleTester.run('no-page-custom-font', rule, {
);
}
}

export default Document;
`,
filename,
},
{
code: `export default function CustomDocument() {
return (
<Html>
<Head>
<link
href="https://fonts.googleapis.com/css2?family=Krona+One&display=swap"
rel="stylesheet"
/>
</Head>
</Html>
)
}`,
filename,
},
{
code: `function CustomDocument() {
return (
<Html>
<Head>
<link
href="https://fonts.googleapis.com/css2?family=Krona+One&display=swap"
rel="stylesheet"
/>
</Head>
</Html>
)
}

export default CustomDocument;
`,
filename,
},
{
code: `
import Document, { Html, Head } from "next/document";
class MyDocument {
render() {
return (
<Html>
<Head>
<link
href="https://fonts.googleapis.com/css2?family=Krona+One&display=swap"
rel="stylesheet"
/>
</Head>
</Html>
);
}
}

export default MyDocument;`,
filename,
},
],

invalid: [
{
code: `
import Head from 'next/head'

export default function IndexPage() {
return (
<div>
Expand All @@ -73,6 +129,7 @@ ruleTester.run('no-page-custom-font', rule, {
)
}
`,
filename: 'pages/index.tsx',
errors: [
{
message:
Expand All @@ -83,29 +140,44 @@ ruleTester.run('no-page-custom-font', rule, {
},
{
code: `
import Document, { Html, Head } from "next/document";
import Head from 'next/head'

class MyDocument {
render() {
return (
<Html>
<Head>
<link
href="https://fonts.googleapis.com/css2?family=Krona+One&display=swap"
rel="stylesheet"
/>
</Head>
</Html>
);
}

function Links() {
return (
<>
<link
href="https://fonts.googleapis.com/css2?family=Inter"
rel="stylesheet"
/>
<link
href="https://fonts.googleapis.com/css2?family=Open+Sans"
rel="stylesheet"
/>
</>
)
}

export default MyDocument;`,

export default function IndexPage() {
return (
<div>
<Head>
<Links />
</Head>
<p>Hello world!</p>
</div>
)
}
`,
filename,
errors: [
{
message:
'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.',
type: 'JSXOpeningElement',
'Rendering this <link /> not inline within <Head> of Document disables font optimization. This is discouraged. See https://nextjs.org/docs/messages/no-page-custom-font.',
},
{
message:
'Rendering this <link /> not inline within <Head> of Document disables font optimization. This is discouraged. See https://nextjs.org/docs/messages/no-page-custom-font.',
},
],
},
Expand Down