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..4cbaa77b16839 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 @@ -7,6 +7,7 @@ import { getUrlFromPagesDirectories, normalizeURL, execOnce, + getUrlFromAppDirectory, } from '../utils/url' const pagesDirWarning = execOnce((pagesDirs) => { @@ -93,6 +94,9 @@ export = defineRule({ } const pageUrls = getUrlFromPagesDirectories('/', foundPagesDirs) + const appDirUrls = getUrlFromAppDirectory('/', foundAppDirs) + const allUrls = [...pageUrls, ...appDirUrls] + return { JSXOpeningElement(node) { if (node.name.name !== 'a') { @@ -134,8 +138,8 @@ export = defineRule({ return } - pageUrls.forEach((pageUrl) => { - if (pageUrl.test(normalizeURL(hrefPath))) { + allUrls.forEach((foundUrl) => { + if (foundUrl.test(normalizeURL(hrefPath))) { context.report({ node, message: `Do not use an \`\` element to navigate to \`${hrefPath}\`. Use \`\` from \`next/link\` instead. See: ${url}`, diff --git a/packages/eslint-plugin-next/src/utils/url.ts b/packages/eslint-plugin-next/src/utils/url.ts index 7634414372b72..c0d7c22bddc99 100644 --- a/packages/eslint-plugin-next/src/utils/url.ts +++ b/packages/eslint-plugin-next/src/utils/url.ts @@ -33,6 +33,33 @@ function parseUrlForPages(urlprefix: string, directory: string) { return res } +/** + * Recursively parse app directory for URLs. + */ +function parseUrlForAppDir(urlprefix: string, directory: string) { + fsReadDirSyncCache[directory] ??= fs.readdirSync(directory, { + withFileTypes: true, + }) + const res = [] + 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 (/^page(\.(j|t)sx?)$/.test(dirent.name)) { + res.push(`${urlprefix}${dirent.name.replace(/^page(\.(j|t)sx?)$/, '')}`) + } else if (!/^layout(\.(j|t)sx?)$/.test(dirent.name)) { + res.push(`${urlprefix}${dirent.name.replace(/(\.(j|t)sx?)$/, '')}`) + } + } else { + const dirPath = path.join(directory, dirent.name) + if (dirent.isDirectory(dirPath) && !dirent.isSymbolicLink()) { + res.push(...parseUrlForPages(urlprefix + dirent.name + '/', dirPath)) + } + } + }) + return res +} + /** * Takes a URL and does the following things. * - Replaces `index.html` with `/` @@ -54,6 +81,56 @@ export function normalizeURL(url: string) { return url } +/** + * Normalizes an app route so it represents the actual request path. Essentially + * performing the following transformations: + * + * - `/(dashboard)/user/[id]/page` to `/user/[id]` + * - `/(dashboard)/account/page` to `/account` + * - `/user/[id]/page` to `/user/[id]` + * - `/account/page` to `/account` + * - `/page` to `/` + * - `/(dashboard)/user/[id]/route` to `/user/[id]` + * - `/(dashboard)/account/route` to `/account` + * - `/user/[id]/route` to `/user/[id]` + * - `/account/route` to `/account` + * - `/route` to `/` + * - `/` to `/` + * + * @param route the app route to normalize + * @returns the normalized pathname + */ +export function normalizeAppPath(route: string) { + return ensureLeadingSlash( + route.split('/').reduce((pathname, segment, index, segments) => { + // Empty segments are ignored. + if (!segment) { + return pathname + } + + // Groups are ignored. + if (isGroupSegment(segment)) { + return pathname + } + + // Parallel segments are ignored. + if (segment[0] === '@') { + return pathname + } + + // The last segment (if it's a leaf) should be ignored. + if ( + (segment === 'page' || segment === 'route') && + index === segments.length - 1 + ) { + return pathname + } + + return `${pathname}/${segment}` + }, '') + ) +} + /** * Gets the possible URLs from a directory. */ @@ -77,6 +154,27 @@ export function getUrlFromPagesDirectories( }) } +export function getUrlFromAppDirectory( + urlPrefix: string, + directories: string[] +) { + return Array.from( + // De-duplicate similar pages across multiple directories. + new Set( + directories + .map((directory) => parseUrlForAppDir(urlPrefix, directory)) + .flat() + .map( + // Since the URLs are normalized we add `^` and `$` to the RegExp to make sure they match exactly. + (url) => `^${normalizeAppPath(url)}$` + ) + ) + ).map((urlReg) => { + urlReg = urlReg.replace(/\[.*\]/g, '((?!.+?\\..+?).*?)') + return new RegExp(urlReg) + }) +} + export function execOnce( fn: (...args: TArgs) => TResult ): (...args: TArgs) => TResult { @@ -91,3 +189,11 @@ export function execOnce( return result } } + +function ensureLeadingSlash(route: string) { + return route.startsWith('/') ? route : `/${route}` +} + +function isGroupSegment(segment: string) { + return segment[0] === '(' && segment.endsWith(')') +} 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 30fec47cf8705..09ae0ad1ecf1e 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 @@ -225,7 +225,33 @@ export class Blah extends Head { } } ` +const validInterceptedRouteCode = ` +import Link from 'next/link'; +export class Blah extends Head { + render() { + return ( +
+ Photo +

Hello title

+
+ ); + } +} +` +const invalidInterceptedRouteCode = ` +import Link from 'next/link'; +export class Blah extends Head { + render() { + return ( +
+ Photo +

Hello title

+
+ ); + } +} +` describe('no-html-link-for-pages', function () { it('does not print warning when there are "pages" or "app" directories with rootDir in context settings', function () { const consoleSpy = jest.spyOn(console, 'warn').mockImplementation() @@ -398,4 +424,129 @@ 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('valid link element with appDir', function () { + const report = withAppLinter.verify(validCode, linterConfig, { + filename: 'foo.js', + }) + assert.deepEqual(report, []) + }) + + it('valid link element with multiple directories with appDir', function () { + const report = withAppLinter.verify(validCode, linterConfig, { + filename: 'foo.js', + }) + assert.deepEqual(report, []) + }) + + it('valid anchor element with appDir', function () { + const report = withAppLinter.verify(validAnchorCode, linterConfig, { + filename: 'foo.js', + }) + assert.deepEqual(report, []) + }) + + it('valid external link element with appDir', function () { + const report = withAppLinter.verify(validExternalLinkCode, linterConfig, { + filename: 'foo.js', + }) + assert.deepEqual(report, []) + }) + + it('valid download link element with appDir', function () { + const report = withAppLinter.verify(validDownloadLinkCode, linterConfig, { + filename: 'foo.js', + }) + assert.deepEqual(report, []) + }) + + it('valid target="_blank" link element with appDir', function () { + const report = withAppLinter.verify( + validTargetBlankLinkCode, + linterConfig, + { + filename: 'foo.js', + } + ) + assert.deepEqual(report, []) + }) + + it('valid public file link element with appDir', function () { + const report = withAppLinter.verify(validPublicFile, linterConfig, { + filename: 'foo.js', + }) + assert.deepEqual(report, []) + }) + + it('invalid static route with appDir', function () { + const [report] = withAppLinter.verify(invalidStaticCode, linterConfig, { + filename: 'foo.js', + }) + assert.notEqual(report, undefined, 'No lint errors found.') + assert.equal( + report.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' + ) + }) + + it('invalid dynamic route with appDir', function () { + const [report] = withAppLinter.verify(invalidDynamicCode, linterConfig, { + filename: 'foo.js', + }) + assert.notEqual(report, undefined, 'No lint errors found.') + assert.equal( + report.message, + 'Do not use an `` element to navigate to `/list/foo/bar/`. Use `` from `next/link` instead. See: https://nextjs.org/docs/messages/no-html-link-for-pages' + ) + const [secondReport] = withAppLinter.verify( + secondInvalidDynamicCode, + linterConfig, + { + filename: 'foo.js', + } + ) + assert.notEqual(secondReport, undefined, 'No lint errors found.') + assert.equal( + secondReport.message, + 'Do not use an `` element to navigate to `/list/foo/`. Use `` from `next/link` instead. See: https://nextjs.org/docs/messages/no-html-link-for-pages' + ) + const [thirdReport] = withAppLinter.verify( + thirdInvalidDynamicCode, + linterConfig, + { + filename: 'foo.js', + } + ) + assert.notEqual(thirdReport, undefined, 'No lint errors found.') + assert.equal( + thirdReport.message, + '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('valid intercepted route with appDir', function () { + const report = withAppLinter.verify( + validInterceptedRouteCode, + linterConfig, + { + filename: 'foo.js', + } + ) + assert.deepEqual(report, []) + }) + + it('invalid intercepted route with appDir', function () { + const [report] = withAppLinter.verify( + invalidInterceptedRouteCode, + linterConfig, + { + filename: 'foo.js', + } + ) + assert.notEqual(report, undefined, 'No lint errors found.') + assert.equal( + report.message, + 'Do not use an `` element to navigate to `/photo/1/`. Use `` from `next/link` instead. See: https://nextjs.org/docs/messages/no-html-link-for-pages' + ) + }) }) diff --git a/test/unit/eslint-plugin-next/with-app-dir/app/index.jsx b/test/unit/eslint-plugin-next/with-app-dir/app/@modal/(..)photo/[id]/page.tsx similarity index 100% rename from test/unit/eslint-plugin-next/with-app-dir/app/index.jsx rename to test/unit/eslint-plugin-next/with-app-dir/app/@modal/(..)photo/[id]/page.tsx diff --git a/test/unit/eslint-plugin-next/with-app-dir/app/@modal/(..)photo/page.tsx b/test/unit/eslint-plugin-next/with-app-dir/app/@modal/(..)photo/page.tsx new file mode 100644 index 0000000000000..ead516c976e9b --- /dev/null +++ b/test/unit/eslint-plugin-next/with-app-dir/app/@modal/(..)photo/page.tsx @@ -0,0 +1 @@ +export default () => {} diff --git a/test/unit/eslint-plugin-next/with-app-dir/app/[profile]/page.tsx b/test/unit/eslint-plugin-next/with-app-dir/app/[profile]/page.tsx new file mode 100644 index 0000000000000..ead516c976e9b --- /dev/null +++ b/test/unit/eslint-plugin-next/with-app-dir/app/[profile]/page.tsx @@ -0,0 +1 @@ +export default () => {} diff --git a/test/unit/eslint-plugin-next/with-app-dir/app/list/[foo]/[id].tsx b/test/unit/eslint-plugin-next/with-app-dir/app/list/[foo]/[id].tsx new file mode 100644 index 0000000000000..ead516c976e9b --- /dev/null +++ b/test/unit/eslint-plugin-next/with-app-dir/app/list/[foo]/[id].tsx @@ -0,0 +1 @@ +export default () => {} diff --git a/test/unit/eslint-plugin-next/with-app-dir/app/page.jsx b/test/unit/eslint-plugin-next/with-app-dir/app/page.jsx new file mode 100644 index 0000000000000..ead516c976e9b --- /dev/null +++ b/test/unit/eslint-plugin-next/with-app-dir/app/page.jsx @@ -0,0 +1 @@ +export default () => {} diff --git a/test/unit/eslint-plugin-next/with-app-dir/app/photo/[id]/page.tsx b/test/unit/eslint-plugin-next/with-app-dir/app/photo/[id]/page.tsx new file mode 100644 index 0000000000000..ead516c976e9b --- /dev/null +++ b/test/unit/eslint-plugin-next/with-app-dir/app/photo/[id]/page.tsx @@ -0,0 +1 @@ +export default () => {}