Skip to content

Commit

Permalink
Fix linting of no-html-link in appDir
Browse files Browse the repository at this point in the history
  • Loading branch information
DerTimonius committed Jun 25, 2023
1 parent 5e36c34 commit fa30abd
Show file tree
Hide file tree
Showing 6 changed files with 152 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
getUrlFromPagesDirectories,
normalizeURL,
execOnce,
getUrlFromAppDirectory,
} from '../utils/url'

const pagesDirWarning = execOnce((pagesDirs) => {
Expand Down Expand Up @@ -93,6 +94,8 @@ export = defineRule({
}

const pageUrls = getUrlFromPagesDirectories('/', foundPagesDirs)
const appDirUrls = getUrlFromAppDirectory('/', foundAppDirs)
const allUrls = [...pageUrls, ...appDirUrls]
return {
JSXOpeningElement(node) {
if (node.name.name !== 'a') {
Expand Down Expand Up @@ -134,8 +137,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 \`<a>\` element to navigate to \`${hrefPath}\`. Use \`<Link />\` from \`next/link\` instead. See: ${url}`,
Expand Down
47 changes: 47 additions & 0 deletions packages/eslint-plugin-next/src/utils/url.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,32 @@ function parseUrlForPages(urlprefix: string, directory: string) {
return res
}

/**
* Recursively parse app directory for URLs.
*/
function parseUrlForAppDir(urlprefix: string, directory: string) {
fsReadDirSyncCache[directory] =
fsReadDirSyncCache[directory] || fs.readdirSync(directory)
const res = []
fsReadDirSyncCache[directory].forEach((fname) => {
// TODO: this should account for all page extensions
// not just js(x) and ts(x)
if (/(\.(j|t)sx?)$/.test(fname)) {
if (/^page(\.(j|t)sx?)$/.test(fname)) {
res.push(`${urlprefix}${fname.replace(/^page(\.(j|t)sx?)$/, '')}`)
} else if (!/^layout(\.(j|t)sx?)$/.test(fname)) {
res.push(`${urlprefix}${fname.replace(/(\.(j|t)sx?)$/, '')}`)
}
} else {
const dirPath = path.join(directory, fname)
if (isDirectory(dirPath) && !isSymlink(dirPath)) {
res.push(...parseUrlForPages(urlprefix + fname + '/', dirPath))
}
}
})
return res
}

/**
* Takes a URL and does the following things.
* - Replaces `index.html` with `/`
Expand Down Expand Up @@ -97,6 +123,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) => `^${normalizeURL(url)}$`
)
)
).map((urlReg) => {
urlReg = urlReg.replace(/\[.*\]/g, '((?!.+?\\..+?).*?)')
return new RegExp(urlReg)
})
}

export function execOnce<TArgs extends any[], TResult extends unknown>(
fn: (...args: TArgs) => TResult
): (...args: TArgs) => TResult {
Expand Down
98 changes: 98 additions & 0 deletions test/unit/eslint-plugin-next/no-html-link-for-pages.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -370,4 +370,102 @@ describe('no-html-link-for-pages', function () {
'Do not use an `<a>` element to navigate to `/list/lorem-ipsum/`. Use `<Link />` 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 `<a>` element to navigate to `/`. Use `<Link />` 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 `<a>` element to navigate to `/list/foo/bar/`. Use `<Link />` 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 `<a>` element to navigate to `/list/foo/`. Use `<Link />` 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 `<a>` element to navigate to `/list/lorem-ipsum/`. Use `<Link />` from `next/link` instead. See: https://nextjs.org/docs/messages/no-html-link-for-pages'
)
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default () => {}
1 change: 1 addition & 0 deletions test/unit/eslint-plugin-next/with-app-dir/app/page.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default () => {}

0 comments on commit fa30abd

Please sign in to comment.