From d8720d9c90eb452831e8b8dbdd708002f85e2214 Mon Sep 17 00:00:00 2001 From: Gerrit Alex Date: Thu, 18 Nov 2021 01:15:09 +0000 Subject: [PATCH 1/4] fix: support function components in _document in no-page-custom-font - fixes #29970 --- .../lib/rules/no-page-custom-font.js | 106 ++++++++++++++++-- .../no-page-custom-font.test.ts | 99 ++++++++++------ 2 files changed, 162 insertions(+), 43 deletions(-) diff --git a/packages/eslint-plugin-next/lib/rules/no-page-custom-font.js b/packages/eslint-plugin-next/lib/rules/no-page-custom-font.js index 054d84a68c5d8..acd8f39902ffe 100644 --- a/packages/eslint-plugin-next/lib/rules/no-page-custom-font.js +++ b/packages/eslint-plugin-next/lib/rules/no-page-custom-font.js @@ -1,4 +1,5 @@ const NodeAttributes = require('../utils/node-attributes.js') +const { sep, posix } = require('path') module.exports = { meta: { @@ -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') { @@ -22,17 +38,91 @@ module.exports = { } } }, + + ExportDefaultDeclaration(node) { + if (node.declaration.type === 'FunctionDeclaration') { + localDefaultExportName = node.declaration.id.name + exportDeclarationType = 'FunctionDeclaration' + return + } + + if ( + node.declaration.type === 'ClassDeclaration' && + node.declaration.superClass && + node.declaration.superClass.name === documentImportName + ) { + localDefaultExportName = node.declaration.id.name + exportDeclarationType = 'ClassDeclaration' + } + }, + 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 ` 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 ` + 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 Name() {} export default XY + // class Name extends Document; export default Name + return ancestor.id && ancestor.id.name === localDefaultExportName + }) + + // file starts with _document and this is within the default export + if (is_Document && parentComponent) { return } diff --git a/test/unit/eslint-plugin-next/no-page-custom-font.test.ts b/test/unit/eslint-plugin-next/no-page-custom-font.test.ts index 9887496c4fe6c..643d7afef9651 100644 --- a/test/unit/eslint-plugin-next/no-page-custom-font.test.ts +++ b/test/unit/eslint-plugin-next/no-page-custom-font.test.ts @@ -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 ( @@ -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 ( @@ -49,42 +51,46 @@ ruleTester.run('no-page-custom-font', rule, { ); } } - export default Document; `, - ], - - invalid: [ + filename, + }, { - code: ` - import Head from 'next/head' + code: `export default function CustomDocument() { + return ( + + + + + + ) + }`, + filename, + }, + { + code: `function CustomDocument() { + return ( + + + + + + ) + } - export default function IndexPage() { - return ( -
- - - -

Hello world!

-
- ) - } - `, - 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', - }, - ], + export default CustomDocument; + `, + filename, }, { code: ` import Document, { Html, Head } from "next/document"; - class MyDocument { render() { return ( @@ -101,6 +107,29 @@ ruleTester.run('no-page-custom-font', rule, { } export default MyDocument;`, + filename, + }, + ], + + invalid: [ + { + code: ` + import Head from 'next/head' + export default function IndexPage() { + return ( +
+ + + +

Hello world!

+
+ ) + } + `, + filename: 'pages/index.tsx', errors: [ { message: From 310532260e315676991630cf4faa75bba251694d Mon Sep 17 00:00:00 2001 From: Gerrit Alex Date: Thu, 18 Nov 2021 23:11:01 +0000 Subject: [PATCH 2/4] refactor: hoist var assignment --- .../eslint-plugin-next/lib/rules/no-page-custom-font.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/eslint-plugin-next/lib/rules/no-page-custom-font.js b/packages/eslint-plugin-next/lib/rules/no-page-custom-font.js index acd8f39902ffe..e755b23c0c424 100644 --- a/packages/eslint-plugin-next/lib/rules/no-page-custom-font.js +++ b/packages/eslint-plugin-next/lib/rules/no-page-custom-font.js @@ -40,9 +40,10 @@ module.exports = { }, ExportDefaultDeclaration(node) { + exportDeclarationType = node.declaration.type + if (node.declaration.type === 'FunctionDeclaration') { localDefaultExportName = node.declaration.id.name - exportDeclarationType = 'FunctionDeclaration' return } @@ -52,7 +53,6 @@ module.exports = { node.declaration.superClass.name === documentImportName ) { localDefaultExportName = node.declaration.id.name - exportDeclarationType = 'ClassDeclaration' } }, @@ -116,8 +116,8 @@ module.exports = { ) } - // function Name() {} export default XY - // class Name extends Document; export default Name + // function ...() {} export default ... + // class ... extends ...; export default ... return ancestor.id && ancestor.id.name === localDefaultExportName }) From f7fc4bad135c4b23f49ef7f2ce6907c4ba7be237 Mon Sep 17 00:00:00 2001 From: Gerrit Alex Date: Sat, 20 Nov 2021 21:32:05 +0000 Subject: [PATCH 3/4] refactor: add warn message for separate components within Document rendering links - update docs - update test --- errors/no-page-custom-font.md | 30 ++++++++++++- .../lib/rules/no-page-custom-font.js | 10 ++++- .../no-page-custom-font.test.ts | 43 +++++++++++++++++++ 3 files changed, 79 insertions(+), 4 deletions(-) diff --git a/errors/no-page-custom-font.md b/errors/no-page-custom-font.md index 93766e1d5d36e..a69d05ff99371 100644 --- a/errors/no-page-custom-font.md +++ b/errors/no-page-custom-font.md @@ -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 @@ -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 ( + + + + + +
+ + + + ) +} +``` + ### 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 diff --git a/packages/eslint-plugin-next/lib/rules/no-page-custom-font.js b/packages/eslint-plugin-next/lib/rules/no-page-custom-font.js index e755b23c0c424..d1de7566c3973 100644 --- a/packages/eslint-plugin-next/lib/rules/no-page-custom-font.js +++ b/packages/eslint-plugin-next/lib/rules/no-page-custom-font.js @@ -137,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 not inline within 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, }) } }, diff --git a/test/unit/eslint-plugin-next/no-page-custom-font.test.ts b/test/unit/eslint-plugin-next/no-page-custom-font.test.ts index 643d7afef9651..1f110c373f59c 100644 --- a/test/unit/eslint-plugin-next/no-page-custom-font.test.ts +++ b/test/unit/eslint-plugin-next/no-page-custom-font.test.ts @@ -138,5 +138,48 @@ ruleTester.run('no-page-custom-font', rule, { }, ], }, + { + code: ` + import Head from 'next/head' + + + function Links() { + return ( + <> + + + + ) + } + + export default function IndexPage() { + return ( +
+ + + +

Hello world!

+
+ ) + } + `, + filename, + errors: [ + { + message: + 'Rendering this not inline within of Document disables font optimization. This is discouraged. See https://nextjs.org/docs/messages/no-page-custom-font.', + }, + { + message: + 'Rendering this not inline within of Document disables font optimization. This is discouraged. See https://nextjs.org/docs/messages/no-page-custom-font.', + }, + ], + }, ], }) From 5b99fb2d0a3781f61646535a4a62fc5309d71211 Mon Sep 17 00:00:00 2001 From: Gerrit Alex Date: Mon, 22 Nov 2021 18:16:03 +0000 Subject: [PATCH 4/4] fix(docs): remove unused import --- errors/no-page-custom-font.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/errors/no-page-custom-font.md b/errors/no-page-custom-font.md index a69d05ff99371..51052a34422cb 100644 --- a/errors/no-page-custom-font.md +++ b/errors/no-page-custom-font.md @@ -41,7 +41,7 @@ Or as a function component: ```js // pages/_document.js -import Document, { Html, Head, Main, NextScript } from 'next/document' +import { Html, Head, Main, NextScript } from 'next/document' export default function Document() { return (