From b75cc8e119aa8f934b1e65823995c052a4604346 Mon Sep 17 00:00:00 2001 From: Houssein Djirdeh Date: Fri, 30 Apr 2021 14:40:33 -0400 Subject: [PATCH 1/4] adds lint rule for missing passHref --- packages/eslint-plugin-next/lib/index.js | 2 + .../lib/rules/link-passhref.js | 62 +++++++++++++++++++ .../lib/utils/nodeAttributes.js | 52 ++++++++++++++++ 3 files changed, 116 insertions(+) create mode 100644 packages/eslint-plugin-next/lib/rules/link-passhref.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..bfbdbe26a3252 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'), + 'link-passhref': require('./rules/link-passhref'), }, 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/link-passhref': 1, }, }, }, diff --git a/packages/eslint-plugin-next/lib/rules/link-passhref.js b/packages/eslint-plugin-next/lib/rules/link-passhref.js new file mode 100644 index 0000000000000..4753dd7efed0a --- /dev/null +++ b/packages/eslint-plugin-next/lib/rules/link-passhref.js @@ -0,0 +1,62 @@ +const NodeAttributes = require('../utils/nodeAttributes.js') + +module.exports = { + meta: { + docs: { + description: + 'Ensure passHref is assigned if child of Link component is a custom component', + category: 'HTML', + recommended: true, + }, + fixable: null, + }, + + create: function (context) { + return { + JSXOpeningElement(node) { + const attributes = new NodeAttributes(node) + const children = node.parent.children + + if (node.name.name !== 'Link') { + return + } + + if ( + !attributes.hasAny() || + !children.some((attr) => attr.type === 'JSXElement') + ) { + return + } + + const hasHref = attributes.has('href') + + if (!hasHref) { + return + } + + const hasPassHref = + attributes.has('passHref') && + (typeof attributes.value('passHref') === 'undefined' || + attributes.value('passHref') === true) + + const hasAnchorChild = children.some( + (attr) => + attr.type === 'JSXElement' && + attr.openingElement.name.name === 'a' && + attr.closingElement.name.name === 'a' + ) + + if (!hasAnchorChild && !hasPassHref) { + context.report({ + node, + message: `passHref ${ + attributes.value('passHref') !== true + ? 'must be set to true' + : 'is missing' + }. See: https://nextjs.org/docs/messages/link-passhref.`, + }) + } + }, + } + }, +} 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 401dd045c97f9b57fc98685b2c3eb24a64914a85 Mon Sep 17 00:00:00 2001 From: Houssein Djirdeh Date: Fri, 30 Apr 2021 15:01:15 -0400 Subject: [PATCH 2/4] adds unit test --- .../lib/rules/link-passhref.js | 9 +-- .../link-passhref.unit.test.js | 67 +++++++++++++++++++ 2 files changed, 69 insertions(+), 7 deletions(-) create mode 100644 test/eslint-plugin-next/link-passhref.unit.test.js diff --git a/packages/eslint-plugin-next/lib/rules/link-passhref.js b/packages/eslint-plugin-next/lib/rules/link-passhref.js index 4753dd7efed0a..749d102595b82 100644 --- a/packages/eslint-plugin-next/lib/rules/link-passhref.js +++ b/packages/eslint-plugin-next/lib/rules/link-passhref.js @@ -23,17 +23,12 @@ module.exports = { if ( !attributes.hasAny() || + !attributes.has('href') || !children.some((attr) => attr.type === 'JSXElement') ) { return } - const hasHref = attributes.has('href') - - if (!hasHref) { - return - } - const hasPassHref = attributes.has('passHref') && (typeof attributes.value('passHref') === 'undefined' || @@ -53,7 +48,7 @@ module.exports = { attributes.value('passHref') !== true ? 'must be set to true' : 'is missing' - }. See: https://nextjs.org/docs/messages/link-passhref.`, + }. See https://nextjs.org/docs/messages/link-passhref.`, }) } }, diff --git a/test/eslint-plugin-next/link-passhref.unit.test.js b/test/eslint-plugin-next/link-passhref.unit.test.js new file mode 100644 index 0000000000000..5f0caa3998b22 --- /dev/null +++ b/test/eslint-plugin-next/link-passhref.unit.test.js @@ -0,0 +1,67 @@ +const rule = require('@next/eslint-plugin-next/lib/rules/link-passhref') +const RuleTester = require('eslint').RuleTester + +RuleTester.setDefaultConfig({ + parserOptions: { + ecmaVersion: 2018, + sourceType: 'module', + ecmaFeatures: { + modules: true, + jsx: true, + }, + }, +}) + +var ruleTester = new RuleTester() +ruleTester.run('link-passhref', rule, { + valid: [ + `export const Home = () => ( + + )`, + + `export const Home = () => ( + + Test + + )`, + + `export const Home = () => ( + + Test + + )`, + ], + + invalid: [ + { + code: ` + export const Home = () => ( + + Test + + )`, + errors: [ + { + message: + 'passHref is missing. See https://nextjs.org/docs/messages/link-passhref.', + type: 'JSXOpeningElement', + }, + ], + }, + { + code: ` + export const Home = () => ( + + Test + + )`, + errors: [ + { + message: + 'passHref must be set to true. See https://nextjs.org/docs/messages/link-passhref.', + type: 'JSXOpeningElement', + }, + ], + }, + ], +}) From 39db4d36b82f1773de299dba93a95c1aad75974b Mon Sep 17 00:00:00 2001 From: Houssein Djirdeh Date: Fri, 30 Apr 2021 15:18:25 -0400 Subject: [PATCH 3/4] adds doc page --- errors/link-passhref.md | 32 ++++++++++++++++++++++++++++++++ errors/manifest.json | 4 ++++ 2 files changed, 36 insertions(+) create mode 100644 errors/link-passhref.md diff --git a/errors/link-passhref.md b/errors/link-passhref.md new file mode 100644 index 0000000000000..33fec658066e0 --- /dev/null +++ b/errors/link-passhref.md @@ -0,0 +1,32 @@ +# Link passHref + +### Why This Error Occurred + +`passHref` was not used for a `Link` component that wraps a custom component. This is needed in order to pass the `href` to the child `` tag. + +### Possible Ways to Fix It + +If you're using a custom component that wraps an `` tag, make sure to add `passHref`: + +```jsx +import Link from 'next/link' +import styled from 'styled-components' + +const StyledLink = styled.a` + color: red; +` + +function NavLink({ href, name }) { + return ( + + {name} + + ) +} + +export default NavLink +``` + +### Useful Links + +- [next/link - Custom Component](https://nextjs.org/docs/api-reference/next/link#if-the-child-is-a-custom-component-that-wraps-an-a-tag) diff --git a/errors/manifest.json b/errors/manifest.json index 431783539f372..42f98a612e853 100644 --- a/errors/manifest.json +++ b/errors/manifest.json @@ -199,6 +199,10 @@ "title": "invalid-webpack-5-version", "path": "/errors/invalid-webpack-5-version.md" }, + { + "title": "link-passhref", + "path": "/errors/link-passhref.md" + }, { "title": "manifest.json", "path": "/errors/manifest.json" }, { "title": "minification-disabled", From 8b98b3c2a613e5286663cda72645e02d55cd0e08 Mon Sep 17 00:00:00 2001 From: Houssein Djirdeh Date: Mon, 3 May 2021 10:52:52 -0400 Subject: [PATCH 4/4] add check for link import --- .../lib/rules/link-passhref.js | 16 ++++++++++++---- .../link-passhref.unit.test.js | 12 ++++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/packages/eslint-plugin-next/lib/rules/link-passhref.js b/packages/eslint-plugin-next/lib/rules/link-passhref.js index 749d102595b82..cfde6bdc943e3 100644 --- a/packages/eslint-plugin-next/lib/rules/link-passhref.js +++ b/packages/eslint-plugin-next/lib/rules/link-passhref.js @@ -12,15 +12,23 @@ module.exports = { }, create: function (context) { + let linkImport = null + return { - JSXOpeningElement(node) { - const attributes = new NodeAttributes(node) - const children = node.parent.children + ImportDeclaration(node) { + if (node.source.value === 'next/link') { + linkImport = node.specifiers[0].local.name + } + }, - if (node.name.name !== 'Link') { + JSXOpeningElement(node) { + if (node.name.name !== 'Link' || node.name.name !== linkImport) { return } + const attributes = new NodeAttributes(node) + const children = node.parent.children + if ( !attributes.hasAny() || !attributes.has('href') || diff --git a/test/eslint-plugin-next/link-passhref.unit.test.js b/test/eslint-plugin-next/link-passhref.unit.test.js index 5f0caa3998b22..b980fff496917 100644 --- a/test/eslint-plugin-next/link-passhref.unit.test.js +++ b/test/eslint-plugin-next/link-passhref.unit.test.js @@ -30,11 +30,21 @@ ruleTester.run('link-passhref', rule, { Test )`, + + `const Link = () =>
Fake Link
+ + const Home = () => ( + + + + )`, ], invalid: [ { code: ` + import Link from 'next/link' + export const Home = () => ( Test @@ -50,6 +60,8 @@ ruleTester.run('link-passhref', rule, { }, { code: ` + import Link from 'next/link' + export const Home = () => ( Test