From 7989b7917ddbe2025906a35022b25a48242db77f Mon Sep 17 00:00:00 2001 From: Houssein Djirdeh Date: Mon, 3 May 2021 14:51:30 -0400 Subject: [PATCH 1/4] adds rule for google font display --- packages/eslint-plugin-next/lib/index.js | 2 + .../lib/rules/google-font-display.js | 56 +++++++++++++++++++ .../lib/utils/nodeAttributes.js | 52 +++++++++++++++++ 3 files changed, 110 insertions(+) create mode 100644 packages/eslint-plugin-next/lib/rules/google-font-display.js create mode 100644 packages/eslint-plugin-next/lib/utils/nodeAttributes.js diff --git a/packages/eslint-plugin-next/lib/index.js b/packages/eslint-plugin-next/lib/index.js index 876aa2e91630e..7ec7b9bc79273 100644 --- a/packages/eslint-plugin-next/lib/index.js +++ b/packages/eslint-plugin-next/lib/index.js @@ -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'), + 'google-font-display': require('./rules/google-font-display'), }, configs: { recommended: { @@ -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/google-font-display': 1, }, }, }, diff --git a/packages/eslint-plugin-next/lib/rules/google-font-display.js b/packages/eslint-plugin-next/lib/rules/google-font-display.js new file mode 100644 index 0000000000000..ccd2b4ab0534c --- /dev/null +++ b/packages/eslint-plugin-next/lib/rules/google-font-display.js @@ -0,0 +1,56 @@ +const NodeAttributes = require('../utils/nodeAttributes.js') + +module.exports = { + meta: { + docs: { + description: + 'Ensure correct font-display property is assigned for Google Fonts', + recommended: true, + }, + }, + create: function (context) { + return { + JSXOpeningElement(node) { + let message + + if (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) { + const params = new URLSearchParams(hrefValue.split('?')[1]) + console.log(params) + const displayValue = params.get('display') + if (!params.has('display')) { + message = 'Display parameter is missing.' + } else if ( + displayValue === 'block' || + displayValue === 'fallback' || + displayValue === 'auto' + ) { + message = `${ + displayValue[0].toUpperCase() + displayValue.slice(1) + } behavior is not recommended.` + } + } + + if (message) { + context.report({ + node, + message: `${message}`, + }) + } + }, + } + }, +} diff --git a/packages/eslint-plugin-next/lib/utils/nodeAttributes.js b/packages/eslint-plugin-next/lib/utils/nodeAttributes.js new file mode 100644 index 0000000000000..c9eb583275c8d --- /dev/null +++ b/packages/eslint-plugin-next/lib/utils/nodeAttributes.js @@ -0,0 +1,52 @@ +// Return attributes and values of a node in a convenient way: +/* example: + + { 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 From ae993d1995a0bab1f0821ed5e43ffa3c476923da Mon Sep 17 00:00:00 2001 From: Houssein Djirdeh Date: Mon, 3 May 2021 15:11:54 -0400 Subject: [PATCH 2/4] adds doc error page --- errors/google-font-display.md | 36 +++++++++++++++++++ errors/manifest.json | 4 +++ .../lib/rules/google-font-display.js | 2 +- 3 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 errors/google-font-display.md diff --git a/errors/google-font-display.md b/errors/google-font-display.md new file mode 100644 index 0000000000000..61328e8976282 --- /dev/null +++ b/errors/google-font-display.md @@ -0,0 +1,36 @@ +# Google Font Display + +### Why This Error Occurred + +For a Google Font, the `display` descriptor was either not assigned or set to `auto`, `fallback`, or `block`. + +### Possible Ways to Fix It + +For most cases, the best font display strategy for custom fonts is `optional`. + +```jsx +import Head from 'next/head' + +export default function IndexPage() { + return ( +
+ + + +
+ ) +} +``` + +Specifying `display=optional` minimizes the risk of invisible text or layout shift. If swapping to the custom font after it has loaded is important to you, then use `display=swap` instead. + +### When Not To Use It + +If you want to specifically display a font using a `block` or `fallback` strategy, then you can disable this rule. + +### Useful Links + +- [Font-display](https://font-display.glitch.me/) diff --git a/errors/manifest.json b/errors/manifest.json index 431783539f372..740c9a3672cb8 100644 --- a/errors/manifest.json +++ b/errors/manifest.json @@ -106,6 +106,10 @@ "title": "generatebuildid-not-a-string", "path": "/errors/generatebuildid-not-a-string.md" }, + { + "title": "google-font-display", + "path": "/errors/google-font-display.md" + }, { "title": "get-initial-props-as-an-instance-method", "path": "/errors/get-initial-props-as-an-instance-method.md" diff --git a/packages/eslint-plugin-next/lib/rules/google-font-display.js b/packages/eslint-plugin-next/lib/rules/google-font-display.js index ccd2b4ab0534c..454f7a056d465 100644 --- a/packages/eslint-plugin-next/lib/rules/google-font-display.js +++ b/packages/eslint-plugin-next/lib/rules/google-font-display.js @@ -47,7 +47,7 @@ module.exports = { if (message) { context.report({ node, - message: `${message}`, + message: `${message} See https://nextjs.org/docs/messages/google-font-display.`, }) } }, From b1324f6228979c73e7f4acf3acc6b9777eb3de75 Mon Sep 17 00:00:00 2001 From: Houssein Djirdeh Date: Mon, 3 May 2021 15:34:19 -0400 Subject: [PATCH 3/4] adds unit test --- .../lib/rules/google-font-display.js | 2 +- .../google-font-display.unit.test.js | 143 ++++++++++++++++++ 2 files changed, 144 insertions(+), 1 deletion(-) create mode 100644 test/eslint-plugin-next/google-font-display.unit.test.js diff --git a/packages/eslint-plugin-next/lib/rules/google-font-display.js b/packages/eslint-plugin-next/lib/rules/google-font-display.js index 454f7a056d465..af6529f407e12 100644 --- a/packages/eslint-plugin-next/lib/rules/google-font-display.js +++ b/packages/eslint-plugin-next/lib/rules/google-font-display.js @@ -29,8 +29,8 @@ module.exports = { if (isGoogleFont) { const params = new URLSearchParams(hrefValue.split('?')[1]) - console.log(params) const displayValue = params.get('display') + if (!params.has('display')) { message = 'Display parameter is missing.' } else if ( diff --git a/test/eslint-plugin-next/google-font-display.unit.test.js b/test/eslint-plugin-next/google-font-display.unit.test.js new file mode 100644 index 0000000000000..7f1bca556b1cd --- /dev/null +++ b/test/eslint-plugin-next/google-font-display.unit.test.js @@ -0,0 +1,143 @@ +const rule = require('@next/eslint-plugin-next/lib/rules/google-font-display') +const RuleTester = require('eslint').RuleTester + +RuleTester.setDefaultConfig({ + parserOptions: { + ecmaVersion: 2018, + sourceType: 'module', + ecmaFeatures: { + modules: true, + jsx: true, + }, + }, +}) + +var ruleTester = new RuleTester() +ruleTester.run('google-font-display', rule, { + valid: [ + `import Head from "next/head"; + + export default Test = () => { + return ( + + + + ); + }; + `, + + `import Document, { Html, Head } from "next/document"; + + class MyDocument extends Document { + render() { + return ( + + + + + + ); + } + } + + export default MyDocument; + `, + ], + + invalid: [ + { + code: `import Head from "next/head"; + + export default Test = () => { + return ( + + + + ); + }; + `, + errors: [ + { + message: + 'Display parameter is missing. See https://nextjs.org/docs/messages/google-font-display.', + type: 'JSXOpeningElement', + }, + ], + }, + { + code: `import Head from "next/head"; + + export default Test = () => { + return ( + + + + ); + }; + `, + errors: [ + { + message: + 'Block behavior is not recommended. See https://nextjs.org/docs/messages/google-font-display.', + type: 'JSXOpeningElement', + }, + ], + }, + { + code: `import Head from "next/head"; + + export default Test = () => { + return ( + + + + ); + }; + `, + errors: [ + { + message: + 'Auto behavior is not recommended. See https://nextjs.org/docs/messages/google-font-display.', + type: 'JSXOpeningElement', + }, + ], + }, + { + code: `import Head from "next/head"; + + export default Test = () => { + return ( + + + + ); + }; + `, + errors: [ + { + message: + 'Fallback behavior is not recommended. See https://nextjs.org/docs/messages/google-font-display.', + type: 'JSXOpeningElement', + }, + ], + }, + ], +}) From 599f844e14f316d4d01424071740c043e4275cca Mon Sep 17 00:00:00 2001 From: Houssein Djirdeh Date: Fri, 7 May 2021 13:59:53 -0400 Subject: [PATCH 4/4] adds eslint rule to preconnect to Google fonts domain --- errors/google-font-preconnect.md | 17 ++++++ errors/manifest.json | 4 ++ packages/eslint-plugin-next/lib/index.js | 2 + .../lib/rules/google-font-preconnect.js | 40 +++++++++++++ .../google-font-preconnect.unit.test.js | 60 +++++++++++++++++++ 5 files changed, 123 insertions(+) create mode 100644 errors/google-font-preconnect.md create mode 100644 packages/eslint-plugin-next/lib/rules/google-font-preconnect.js create mode 100644 test/eslint-plugin-next/google-font-preconnect.unit.test.js diff --git a/errors/google-font-preconnect.md b/errors/google-font-preconnect.md new file mode 100644 index 0000000000000..de629ac4e1bf3 --- /dev/null +++ b/errors/google-font-preconnect.md @@ -0,0 +1,17 @@ +# Google Font Preconnect + +### Why This Error Occurred + +A preconnect resource hint was not used with a request to the Google Fonts domain. Adding `preconnect` is recommended to initiate an early connection to the origin. + +### Possible Ways to Fix It + +Add `rel="preconnect"` to the Google Font domain `` tag: + +```jsx + +``` + +### Useful Links + +- [Preconnect to required origins](https://web.dev/uses-rel-preconnect/) diff --git a/errors/manifest.json b/errors/manifest.json index e9f68415779dd..593779eb6fc25 100644 --- a/errors/manifest.json +++ b/errors/manifest.json @@ -110,6 +110,10 @@ "title": "google-font-display", "path": "/errors/google-font-display.md" }, + { + "title": "google-font-preconnect", + "path": "/errors/google-font-preconnect.md" + }, { "title": "get-initial-props-as-an-instance-method", "path": "/errors/get-initial-props-as-an-instance-method.md" diff --git a/packages/eslint-plugin-next/lib/index.js b/packages/eslint-plugin-next/lib/index.js index dcccaf2de4a22..3212e7a21e8a3 100644 --- a/packages/eslint-plugin-next/lib/index.js +++ b/packages/eslint-plugin-next/lib/index.js @@ -6,6 +6,7 @@ module.exports = { 'no-unwanted-polyfillio': require('./rules/no-unwanted-polyfillio'), 'no-title-in-document-head': require('./rules/no-title-in-document-head'), 'google-font-display': require('./rules/google-font-display'), + 'google-font-preconnect': require('./rules/google-font-preconnect'), }, configs: { recommended: { @@ -17,6 +18,7 @@ module.exports = { '@next/next/no-unwanted-polyfillio': 1, '@next/next/no-title-in-document-head': 1, '@next/next/google-font-display': 1, + '@next/next/google-font-preconnect': 1, }, }, }, diff --git a/packages/eslint-plugin-next/lib/rules/google-font-preconnect.js b/packages/eslint-plugin-next/lib/rules/google-font-preconnect.js new file mode 100644 index 0000000000000..acf8b6f1a50ee --- /dev/null +++ b/packages/eslint-plugin-next/lib/rules/google-font-preconnect.js @@ -0,0 +1,40 @@ +const NodeAttributes = require('../utils/nodeAttributes.js') + +module.exports = { + meta: { + docs: { + description: 'Ensure preconnect is used with Google Fonts', + recommended: true, + }, + }, + create: function (context) { + return { + JSXOpeningElement(node) { + if (node.name.name !== 'link') { + return + } + + const attributes = new NodeAttributes(node) + if (!attributes.has('href') || !attributes.hasValue('href')) { + return + } + + const hrefValue = attributes.value('href') + const preconnectMissing = + !attributes.has('rel') || + !attributes.hasValue('rel') || + attributes.value('rel') !== 'preconnect' + + if ( + hrefValue.includes('https://fonts.gstatic.com') && + preconnectMissing + ) { + context.report({ + node, + message: `Preconnect is missing. See https://nextjs.org/docs/messages/google-font-preconnect.`, + }) + } + }, + } + }, +} diff --git a/test/eslint-plugin-next/google-font-preconnect.unit.test.js b/test/eslint-plugin-next/google-font-preconnect.unit.test.js new file mode 100644 index 0000000000000..90b1a374db1ba --- /dev/null +++ b/test/eslint-plugin-next/google-font-preconnect.unit.test.js @@ -0,0 +1,60 @@ +const rule = require('@next/eslint-plugin-next/lib/rules/google-font-preconnect') +const RuleTester = require('eslint').RuleTester + +RuleTester.setDefaultConfig({ + parserOptions: { + ecmaVersion: 2018, + sourceType: 'module', + ecmaFeatures: { + modules: true, + jsx: true, + }, + }, +}) + +var ruleTester = new RuleTester() +ruleTester.run('google-font-preconnect', rule, { + valid: [ + `export const Test = () => ( +
+ +
+ ) + `, + ], + + invalid: [ + { + code: ` + export const Test = () => ( +
+ +
+ ) + `, + errors: [ + { + message: + 'Preconnect is missing. See https://nextjs.org/docs/messages/google-font-preconnect.', + type: 'JSXOpeningElement', + }, + ], + }, + { + code: ` + export const Test = () => ( +
+ +
+ ) + `, + errors: [ + { + message: + 'Preconnect is missing. See https://nextjs.org/docs/messages/google-font-preconnect.', + type: 'JSXOpeningElement', + }, + ], + }, + ], +})