From 13a6c1b65358ff532ef2ebcad39d9b901bef0186 Mon Sep 17 00:00:00 2001 From: Max Date: Thu, 3 Aug 2023 21:13:49 +0800 Subject: [PATCH] feat(eslint-plugin-next): add `pageExtensions` option --- errors/no-html-link-for-pages.mdx | 55 ++++++++++-- .../src/rules/no-html-link-for-pages.ts | 50 +++++++++-- .../src/utils/get-rule-options.ts | 42 +++++++++ packages/eslint-plugin-next/src/utils/url.ts | 65 ++++++++++---- .../no-html-link-for-pages.test.ts | 87 ++++++++++++++++++- .../pages/about.page.tsx | 1 + .../pages/component.tsx | 1 + .../pages/index.page.tsx | 1 + 8 files changed, 272 insertions(+), 30 deletions(-) create mode 100644 packages/eslint-plugin-next/src/utils/get-rule-options.ts create mode 100644 test/unit/eslint-plugin-next/with-pages-and-extensions/pages/about.page.tsx create mode 100644 test/unit/eslint-plugin-next/with-pages-and-extensions/pages/component.tsx create mode 100644 test/unit/eslint-plugin-next/with-pages-and-extensions/pages/index.page.tsx diff --git a/errors/no-html-link-for-pages.mdx b/errors/no-html-link-for-pages.mdx index 16e87fa397ee2..13ed052570c10 100644 --- a/errors/no-html-link-for-pages.mdx +++ b/errors/no-html-link-for-pages.mdx @@ -44,6 +44,26 @@ export default Home ### Options +The rule accepts the following options: + +```ts +type NoHTMLLinkForPagesOptions = + | string + | string[] + | { + pagesDir?: string | string[] + pageExtensions?: string | string[] + } +``` + +The values can be: + +- `string` - It's equivalent to the `pagesDir` argument. +- `string[]` - It's equivalent to the `pagesDir` argument. +- An object with the following properties: + - `pagesDir` - a path to a directory or an array of paths to directories + - `pageExtensions` - a string or an array of strings representing the extensions of the page files + #### `pagesDir` This rule can normally locate your `pages` directory automatically. @@ -52,14 +72,39 @@ If you're working in a monorepo, we recommend configuring the [`rootDir`](/docs/ In some cases, you may also need to configure this rule directly by providing a `pages` directory. This can be a path or an array of paths. -```json filename="eslint.config.json" -{ - "rules": { - "@next/next/no-html-link-for-pages": ["error", "packages/my-app/pages/"] - } +```js filename=".eslintrc.js" highlight={3} +module.exports = { + rules: { + '@next/next/no-html-link-for-pages': ['error', 'packages/foo/pages/'], + // or + // "@next/next/no-html-link-for-pages": ["error", { pagesDir: "packages/foo/pages/" }], + }, +} +``` + +#### `pageExtensions` + +If you set the [`pageExtensions`](/docs/app/api-reference/next-config-js/pageExtensions) config, this rule will not work and the `pageExtensions` option must be set for it to work. + +```js filename="next.config.js" highlight={3} +/** @type {import('next').NextConfig} */ +module.exports = { + pageExtensions: ['page.tsx', 'mdx'], +} +``` + +```js filename=".eslintrc.js" highlight={5} +module.exports = { + rules: { + '@next/next/no-html-link-for-pages': [ + 'error', + { pageExtensions: ['page.tsx', 'mdx'] }, + ], + }, } ``` ## Useful Links - [next/link API Reference](/docs/pages/api-reference/components/link) +- [next.config.js Options: pageExtensions](/docs/app/api-reference/next-config-js/pageExtensions) diff --git a/packages/eslint-plugin-next/src/rules/no-html-link-for-pages.ts b/packages/eslint-plugin-next/src/rules/no-html-link-for-pages.ts index dbb8b43c47ccd..de9b5904db23f 100644 --- a/packages/eslint-plugin-next/src/rules/no-html-link-for-pages.ts +++ b/packages/eslint-plugin-next/src/rules/no-html-link-for-pages.ts @@ -2,6 +2,7 @@ import { defineRule } from '../utils/define-rule' import * as path from 'path' import * as fs from 'fs' import { getRootDirs } from '../utils/get-root-dirs' +import getRuleOptions from '../utils/get-rule-options' import { getUrlFromPagesDirectories, @@ -45,6 +46,40 @@ export = defineRule({ type: 'string', }, }, + { + type: 'object', + additionalProperties: false, + properties: { + pagesDir: { + oneOf: [ + { + type: 'string', + }, + { + type: 'array', + uniqueItems: true, + items: { + type: 'string', + }, + }, + ], + }, + pageExtensions: { + oneOf: [ + { + type: 'string', + }, + { + type: 'array', + uniqueItems: true, + items: { + type: 'string', + }, + }, + ], + }, + }, + }, ], }, ], @@ -54,14 +89,14 @@ export = defineRule({ * Creates an ESLint rule listener. */ create(context) { - const ruleOptions: (string | string[])[] = context.options - const [customPagesDirectory] = ruleOptions + // TODO: get `pageExtensions` from `next.config.js` + const { pagesDir, pageExtensions } = getRuleOptions(context) const rootDirs = getRootDirs(context) const pagesDirs = ( - customPagesDirectory - ? [customPagesDirectory] + pagesDir.length > 0 + ? pagesDir : rootDirs.map((dir) => [ path.join(dir, 'pages'), path.join(dir, 'src', 'pages'), @@ -92,7 +127,12 @@ export = defineRule({ return {} } - const pageUrls = getUrlFromPagesDirectories('/', foundPagesDirs) + const pageUrls = getUrlFromPagesDirectories( + '/', + foundPagesDirs, + pageExtensions + ) + return { JSXOpeningElement(node) { if (node.name.name !== 'a') { diff --git a/packages/eslint-plugin-next/src/utils/get-rule-options.ts b/packages/eslint-plugin-next/src/utils/get-rule-options.ts new file mode 100644 index 0000000000000..8b1b30bed06d2 --- /dev/null +++ b/packages/eslint-plugin-next/src/utils/get-rule-options.ts @@ -0,0 +1,42 @@ +import type { Rule } from 'eslint' + +type RuleOption = + | string + | string[] + | { + pagesDir?: string | string[] + pageExtensions?: string | string[] + } + +/** + * Gets the rule options. + */ +export default function getRuleOptions(context: Rule.RuleContext) { + const options: RuleOption | undefined = context.options?.[0] + + let pagesDir: string[] = [] + let pageExtensions: string[] = ['tsx', 'ts', 'jsx', 'js'] + + if (typeof options === 'string') { + pagesDir = [options] + } else if (Array.isArray(options)) { + pagesDir = options + } else if (typeof options === 'object' && options !== null) { + if (typeof options.pagesDir === 'string') { + pagesDir = [options.pagesDir] + } else if (Array.isArray(options.pagesDir)) { + pagesDir = options.pagesDir + } + + if (typeof options.pageExtensions === 'string') { + pageExtensions = [options.pageExtensions] + } else if (Array.isArray(options.pageExtensions)) { + pageExtensions = options.pageExtensions + } + } + + return { + pagesDir, + pageExtensions, + } +} diff --git a/packages/eslint-plugin-next/src/utils/url.ts b/packages/eslint-plugin-next/src/utils/url.ts index ab70eb8e0d8a5..ece49e1a0d74f 100644 --- a/packages/eslint-plugin-next/src/utils/url.ts +++ b/packages/eslint-plugin-next/src/utils/url.ts @@ -1,35 +1,63 @@ import * as path from 'path' import * as fs from 'fs' +/** + * These characters `(.*+?^${}()|[]\)` are considered special characters in regular expressions, and need to be escaped if they are to be matched literally. + * + * @see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions#escaping + */ +function escapeRegExp(str: string) { + return str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') +} + // Cache for fs.readdirSync lookup. // Prevent multiple blocking IO requests that have already been calculated. -const fsReadDirSyncCache = {} +const fsReadDirSyncCache: Record = {} /** * Recursively parse directory for page URLs. */ -function parseUrlForPages(urlprefix: string, directory: string) { +function parseUrlForPages( + urlprefix: string, + directory: string, + pageExtensions: string[] +) { fsReadDirSyncCache[directory] ??= fs.readdirSync(directory, { withFileTypes: true, }) - const res = [] + + const res: string[] = [] + fsReadDirSyncCache[directory].forEach((dirent) => { - // TODO: this should account for all page extensions - // not just js(x) and ts(x) - if (/(\.(j|t)sx?)$/.test(dirent.name)) { - if (/^index(\.(j|t)sx?)$/.test(dirent.name)) { - res.push( - `${urlprefix}${dirent.name.replace(/^index(\.(j|t)sx?)$/, '')}` + if (dirent.isDirectory() && !dirent.isSymbolicLink()) { + res.push( + ...parseUrlForPages( + urlprefix + dirent.name + '/', + path.join(directory, dirent.name), + pageExtensions ) - } - res.push(`${urlprefix}${dirent.name.replace(/(\.(j|t)sx?)$/, '')}`) + ) + return + } + + const ext = pageExtensions.find((pageExtension) => + new RegExp(`\\.${escapeRegExp(pageExtension)}$`).test(dirent.name) + ) + + if (!ext) return + + const replaced = escapeRegExp(ext) + const startsIndexReg = new RegExp(`^index\\.${replaced}$`) + + if (startsIndexReg.test(dirent.name)) { + res.push(urlprefix + dirent.name.replace(startsIndexReg, '')) } else { - const dirPath = path.join(directory, dirent.name) - if (dirent.isDirectory() && !dirent.isSymbolicLink()) { - res.push(...parseUrlForPages(urlprefix + dirent.name + '/', dirPath)) - } + res.push( + urlprefix + dirent.name.replace(new RegExp(`\\.${replaced}$`), '') + ) } }) + return res } @@ -59,13 +87,16 @@ export function normalizeURL(url: string) { */ export function getUrlFromPagesDirectories( urlPrefix: string, - directories: string[] + directories: string[], + pageExtensions: string[] ) { return Array.from( // De-duplicate similar pages across multiple directories. new Set( directories - .flatMap((directory) => parseUrlForPages(urlPrefix, directory)) + .flatMap((directory) => + parseUrlForPages(urlPrefix, directory, pageExtensions) + ) .map( // Since the URLs are normalized we add `^` and `$` to the RegExp to make sure they match exactly. (url) => `^${normalizeURL(url)}$` diff --git a/test/unit/eslint-plugin-next/no-html-link-for-pages.test.ts b/test/unit/eslint-plugin-next/no-html-link-for-pages.test.ts index fe3ac7b429203..50fa0c17be84d 100644 --- a/test/unit/eslint-plugin-next/no-html-link-for-pages.test.ts +++ b/test/unit/eslint-plugin-next/no-html-link-for-pages.test.ts @@ -15,8 +15,11 @@ const withAppLinter = new Linter({ const withCustomPagesLinter = new Linter({ cwd: withCustomPagesDirectory, }) +const withPagesAndExtensionsLinter = new Linter({ + cwd: path.join(__dirname, 'with-pages-and-extensions'), +}) -const linterConfig: any = { +const linterConfig: Linter.Config = { rules: { 'no-html-link-for-pages': [2], }, @@ -29,7 +32,7 @@ const linterConfig: any = { }, }, } -const linterConfigWithCustomDirectory: any = { +const linterConfigWithCustomDirectory: Linter.Config = { ...linterConfig, rules: { 'no-html-link-for-pages': [ @@ -38,7 +41,7 @@ const linterConfigWithCustomDirectory: any = { ], }, } -const linterConfigWithMultipleDirectories = { +const linterConfigWithMultipleDirectories: Linter.Config = { ...linterConfig, rules: { 'no-html-link-for-pages': [ @@ -51,6 +54,18 @@ const linterConfigWithMultipleDirectories = { }, } +const linterConfigWithPagesAndPageExtensions: Linter.Config = { + ...linterConfig, + rules: { + 'no-html-link-for-pages': [ + 2, + { + pageExtensions: ['page.tsx'], + }, + ], + }, +} + withoutPagesLinter.defineRules({ 'no-html-link-for-pages': rule, }) @@ -60,6 +75,9 @@ withAppLinter.defineRules({ withCustomPagesLinter.defineRules({ 'no-html-link-for-pages': rule, }) +withPagesAndExtensionsLinter.defineRules({ + 'no-html-link-for-pages': rule, +}) const validCode = ` import Link from 'next/link'; @@ -211,6 +229,39 @@ export class Blah extends Head { } ` +const invalidStaticRouterWithPageExtensionsCode = ` +import Link from 'next/link'; + +export class Blah extends Head { + render() { + return ( +
+ Homepage + About +

Hello title

+
+ ); + } +} +` + +const validStaticRouterWithPageExtensionsCode = ` +import Link from 'next/link'; + +export class Blah extends Head { + render() { + return ( +
+ Homepage + About + Component +

Hello title

+
+ ); + } +} +` + describe('no-html-link-for-pages', function () { it('prints warning when there are no "pages" or "app" directories', function () { const consoleSpy = jest.spyOn(console, 'warn').mockImplementation() @@ -370,4 +421,34 @@ describe('no-html-link-for-pages', function () { 'Do not use an `` element to navigate to `/list/lorem-ipsum/`. Use `` from `next/link` instead. See: https://nextjs.org/docs/messages/no-html-link-for-pages' ) }) + + it('invalid static route with pageExtensions', function () { + const report = withPagesAndExtensionsLinter.verify( + invalidStaticRouterWithPageExtensionsCode, + linterConfigWithPagesAndPageExtensions, + { + filename: 'foo.js', + } + ) + assert.equal(report.length, 2) + assert.equal( + report[0].message, + 'Do not use an `` element to navigate to `/`. Use `` from `next/link` instead. See: https://nextjs.org/docs/messages/no-html-link-for-pages' + ) + assert.equal( + report[1].message, + 'Do not use an `` element to navigate to `/about/`. Use `` from `next/link` instead. See: https://nextjs.org/docs/messages/no-html-link-for-pages' + ) + }) + + it('valid static route with pageExtensions', function () { + const report = withPagesAndExtensionsLinter.verify( + validStaticRouterWithPageExtensionsCode, + linterConfigWithPagesAndPageExtensions, + { + filename: 'foo.js', + } + ) + assert.deepEqual(report, []) + }) }) diff --git a/test/unit/eslint-plugin-next/with-pages-and-extensions/pages/about.page.tsx b/test/unit/eslint-plugin-next/with-pages-and-extensions/pages/about.page.tsx new file mode 100644 index 0000000000000..aed553c228a7d --- /dev/null +++ b/test/unit/eslint-plugin-next/with-pages-and-extensions/pages/about.page.tsx @@ -0,0 +1 @@ +export default function About() {} diff --git a/test/unit/eslint-plugin-next/with-pages-and-extensions/pages/component.tsx b/test/unit/eslint-plugin-next/with-pages-and-extensions/pages/component.tsx new file mode 100644 index 0000000000000..c6be8faf00884 --- /dev/null +++ b/test/unit/eslint-plugin-next/with-pages-and-extensions/pages/component.tsx @@ -0,0 +1 @@ +export default function Component() {} diff --git a/test/unit/eslint-plugin-next/with-pages-and-extensions/pages/index.page.tsx b/test/unit/eslint-plugin-next/with-pages-and-extensions/pages/index.page.tsx new file mode 100644 index 0000000000000..41473ff4eedd6 --- /dev/null +++ b/test/unit/eslint-plugin-next/with-pages-and-extensions/pages/index.page.tsx @@ -0,0 +1 @@ +export default function Home() {}