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

ESLint Plugin: Custom Font at page-level rule #24789

Merged
merged 6 commits into from
May 10, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
4 changes: 4 additions & 0 deletions errors/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,10 @@
"title": "no-on-app-updated-hook",
"path": "/errors/no-on-app-updated-hook.md"
},
{
"title": "no-page-custom-font",
"path": "/errors/no-page-custom-font.md"
},
{
"title": "no-router-instance",
"path": "/errors/no-router-instance.md"
Expand Down
45 changes: 45 additions & 0 deletions errors/no-page-custom-font.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# No Page Custom Font

### 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.

### Possible Ways to Fix It

Create the file `./pages/document.js` and add the font to a custom Document:

```jsx
// pages/_document.js

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=Inter&display=optional"
rel="stylesheet"
/>
</Head>
<body>
<Main />
<NextScript />
</body>
</Html>
)
}
}

export default MyDocument
```

### 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.

### Useful Links

- [Custom Document](https://nextjs.org/docs/advanced-features/custom-document)
- [Font Optimization](https://nextjs.org/docs/basic-features/font-optimization)
2 changes: 2 additions & 0 deletions packages/eslint-plugin-next/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ module.exports = {
'no-sync-scripts': require('./rules/no-sync-scripts'),
'no-html-link-for-pages': require('./rules/no-html-link-for-pages'),
'no-unwanted-polyfillio': require('./rules/no-unwanted-polyfillio'),
'no-page-custom-font': require('./rules/no-page-custom-font'),
},
configs: {
recommended: {
Expand All @@ -13,6 +14,7 @@ module.exports = {
'@next/next/no-sync-scripts': 1,
'@next/next/no-html-link-for-pages': 1,
'@next/next/no-unwanted-polyfillio': 1,
'@next/next/no-page-custom-font': 1,
},
},
},
Expand Down
55 changes: 55 additions & 0 deletions packages/eslint-plugin-next/lib/rules/no-page-custom-font.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
const NodeAttributes = require('../utils/nodeAttributes.js')

module.exports = {
meta: {
docs: {
description:
'Recommend adding custom font in a custom document and not in a specific page',
recommended: true,
},
},
create: function (context) {
let documentImport = false
return {
ImportDeclaration(node) {
if (node.source.value === 'next/document') {
if (node.specifiers.some(({ local }) => local.name === 'Document')) {
documentImport = true
}
}
},
JSXOpeningElement(node) {
const documentClass = context
.getAncestors()
.find(
(ancestorNode) =>
ancestorNode.type === 'ClassDeclaration' &&
ancestorNode.superClass &&
ancestorNode.superClass.name === 'Document'
)

if ((documentImport && documentClass) || node.name.name !== 'link') {
return
}

const attributes = new NodeAttributes(node)
if (!attributes.has('href') || !attributes.hasValue('href')) {
return
}

const hrefValue = attributes.value('href')
const isGoogleFont = hrefValue.includes(
'https://fonts.googleapis.com/css'
)

if (isGoogleFont) {
context.report({
node,
message:
'Custom fonts should be added at the document level. See https://nextjs.org/docs/messages/no-page-custom-font.',
})
}
},
}
},
}
52 changes: 52 additions & 0 deletions packages/eslint-plugin-next/lib/utils/nodeAttributes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// Return attributes and values of a node in a convenient way:
/* example:
<ExampleElement attr1="15" attr2>
{ attr1: {
hasValue: true,
value: 15
},
attr2: {
hasValue: false
}
Inclusion of hasValue is in case an eslint rule cares about boolean values
explicitely assigned to attribute vs the attribute being used as a flag
*/
class NodeAttributes {
constructor(ASTnode) {
this.attributes = {}
ASTnode.attributes.forEach((attribute) => {
if (!attribute.type || attribute.type !== 'JSXAttribute') {
return
}
this.attributes[attribute.name.name] = {
hasValue: !!attribute.value,
}
if (attribute.value) {
if (attribute.value.value) {
this.attributes[attribute.name.name].value = attribute.value.value
} else if (attribute.value.expression) {
this.attributes[attribute.name.name].value =
attribute.value.expression.value
}
}
})
}
hasAny() {
return !!Object.keys(this.attributes).length
}
has(attrName) {
return !!this.attributes[attrName]
}
hasValue(attrName) {
return !!this.attributes[attrName].hasValue
}
value(attrName) {
if (!this.attributes[attrName]) {
return true
}

return this.attributes[attrName].value
}
}

module.exports = NodeAttributes
95 changes: 95 additions & 0 deletions test/eslint-plugin-next/no-page-custom-font.unit.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
const rule = require('@next/eslint-plugin-next/lib/rules/no-page-custom-font')
const RuleTester = require('eslint').RuleTester

RuleTester.setDefaultConfig({
parserOptions: {
ecmaVersion: 2018,
sourceType: 'module',
ecmaFeatures: {
modules: true,
jsx: true,
},
},
})

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

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

export default MyDocument;
`,
],

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

export default function IndexPage() {
return (
<div>
<Head>
<link
href="https://fonts.googleapis.com/css2?family=Inter"
rel="stylesheet"
/>
</Head>
<p>Hello world!</p>
</div>
)
}
`,
errors: [
{
message:
'Custom fonts should be added at the document level. See https://nextjs.org/docs/messages/no-page-custom-font.',
type: 'JSXOpeningElement',
},
],
},
{
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;`,
errors: [
{
message:
'Custom fonts should be added at the document level. See https://nextjs.org/docs/messages/no-page-custom-font.',
type: 'JSXOpeningElement',
},
],
},
],
})