Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/eslint-pageextensions-support.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@next/eslint-plugin-next": patch
---

Respect `pageExtensions` config in `no-html-link-for-pages` rule. Supports `.mdx`, `.md`, and custom file extensions. Backwards compatible.
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ export default defineRule({
return {}
}

const pageUrls = cachedGetUrlFromPagesDirectories('/', foundPagesDirs)
const appDirUrls = cachedGetUrlFromAppDirectory('/', foundAppDirs)
const pageUrls = cachedGetUrlFromPagesDirectories('/', foundPagesDirs, pageExtensions)
const appDirUrls = cachedGetUrlFromAppDirectory('/', foundAppDirs, pageExtensions)
Comment on lines +110 to +111

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P0 Badge Define pageExtensions before passing to cached URL helpers

The rule now forwards a pageExtensions variable to cachedGetUrlFromPagesDirectories and cachedGetUrlFromAppDirectory, but no variable with that name is declared in the function. When ESLint executes this rule the call will throw a ReferenceError, preventing the rule from running at all. You likely intended to resolve the extensions from options or context.settings before these calls.

Useful? React with 👍 / 👎.

Comment on lines +110 to +111
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable pageExtensions is used but never defined in the rule's create function. This will cause a ReferenceError at runtime.

View Details
📝 Patch Details
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 c522cda0ed..81a063ce12 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
@@ -73,6 +73,10 @@ export default defineRule({
     const [customPagesDirectory] = ruleOptions
 
     const rootDirs = getRootDirs(context)
+    
+    const nextSettings: { pageExtensions?: string[] } =
+      context.settings.next || {}
+    const pageExtensions = nextSettings.pageExtensions
 
     const pagesDirs = (
       customPagesDirectory

Analysis

Missing variable definition in no-html-link-for-pages rule causes ReferenceError

What fails: Lines 110-111 in packages/eslint-plugin-next/src/rules/no-html-link-for-pages.ts reference undefined variable pageExtensions, causing TypeScript compilation error TS2304: "Cannot find name 'pageExtensions'".

How to reproduce:

cd packages/eslint-plugin-next
pnpm types

Result: TypeScript compiler error:

src/rules/no-html-link-for-pages.ts(110,76): error TS2304: Cannot find name 'pageExtensions'.
src/rules/no-html-link-for-pages.ts(111,72): error TS2304: Cannot find name 'pageExtensions'.

Root cause: Commit 7496bce updated the function signatures of getUrlFromPagesDirectories and getUrlFromAppDirectory to accept an optional pageExtensions parameter, but failed to extract and define pageExtensions in the rule's create function before using it on lines 110-111.

Fix: Extract pageExtensions from ESLint settings following the same pattern used by getRootDirs utility function, which reads from context.settings.next. Added extraction of pageExtensions from settings with proper TypeScript typing to match the expected optional string array parameter.

const allUrlRegex = [...pageUrls, ...appDirUrls]

return {
Expand Down
187 changes: 102 additions & 85 deletions packages/eslint-plugin-next/src/utils/url.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,28 +5,40 @@ import * as fs from 'fs'
// Prevent multiple blocking IO requests that have already been calculated.
const fsReadDirSyncCache = {}

/**
* Build regex for matching page extensions.
* Defaults to ['js', 'jsx', 'ts', 'tsx'] if not provided.
*/
function buildPageExtensionRegex(pageExtensions?: string[]): RegExp {
const defaultExts = ['js', 'jsx', 'ts', 'tsx']
const exts = pageExtensions && pageExtensions.length ? pageExtensions : defaultExts
const escapedExts = exts.map((ext) => ext.replace(/\./g, '\\.')).join('|')
return new RegExp(`\\.(${escapedExts})$`)
}

/**
* 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 pageExtensionRegex = buildPageExtensionRegex(pageExtensions)
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 (pageExtensionRegex.test(dirent.name)) {
const indexMatch = dirent.name.match(/^index(\..+?)$/)
const replaceMatch = dirent.name.match(/(\..+?)$/)
if (indexMatch) {
res.push(`${urlprefix}${dirent.name.replace(indexMatch[0], '')}`)
}
if (replaceMatch) {
Comment on lines +34 to +35
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
}
if (replaceMatch) {
} else if (replaceMatch) {

The parseUrlForPages function incorrectly generates multiple URLs for index files, creating invalid routes like /index in addition to the correct / route.

View Details

Analysis

Bug: parseUrlForPages generates invalid /index routes for index files

What fails: The parseUrlForPages() function in packages/eslint-plugin-next/src/utils/url.ts (lines 29-37) generates both / and /index URLs for index files, when only / is a valid Next.js route.

How to reproduce:

// Create a pages directory with index.tsx file
parseUrlForPages('/', 'pages/', undefined)
// With a file named: pages/index.tsx

Result: Returns ['/', '/index'] instead of just ['/']

Expected: Should return only ['/'] because in Next.js, pages/index.tsx creates a route at /, not /index. The /index route does not exist in Next.js.

Impact: The ESLint rule no-html-link-for-pages incorrectly flags <a href="/index"> as needing to use <Link> component, when /index is not an actual Next.js route. Additionally, it may miss flagging actual invalid links if the URL normalization deduplicates the patterns.

Root cause: Lines 32-37 use two independent if statements. For index files:

  • indexMatch correctly matches and creates /
  • replaceMatch also matches (since .tsx matches /(\..+?)$/) and creates /index

Both conditions execute for index files, creating the invalid route. The second condition should only execute if NOT an index file (using else if).

Note: The parallel parseUrlForAppDir() function (lines 45-72) already implements the correct pattern using else if logic, confirming this is the intended behavior.

res.push(`${urlprefix}${dirent.name.replace(replaceMatch[1], '')}`)
}
res.push(`${urlprefix}${dirent.name.replace(/(\.(j|t)sx?)$/, '')}`)
} else {
const dirPath = path.join(directory, dirent.name)
if (dirent.isDirectory() && !dirent.isSymbolicLink()) {
res.push(...parseUrlForPages(urlprefix + dirent.name + '/', dirPath))
res.push(...parseUrlForPages(urlprefix + dirent.name + '/', dirPath, pageExtensions))
}
}
})
Expand All @@ -36,24 +48,27 @@ function parseUrlForPages(urlprefix: string, directory: string) {
/**
* Recursively parse app directory for URLs.
*/
function parseUrlForAppDir(urlprefix: string, directory: string) {
function parseUrlForAppDir(urlprefix: string, directory: string, pageExtensions?: string[]) {
fsReadDirSyncCache[directory] ??= fs.readdirSync(directory, {
withFileTypes: true,
})
const res = []
const pageExtensionRegex = buildPageExtensionRegex(pageExtensions)
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?)$/, '')}`)
if (pageExtensionRegex.test(dirent.name)) {
const pageMatch = dirent.name.match(/^page(\..+?)$/)
const layoutMatch = dirent.name.match(/^layout(\..+?)$/)
const replaceMatch = dirent.name.match(/(\..+?)$/)

if (pageMatch) {
res.push(`${urlprefix}${dirent.name.replace(pageMatch[0], '')}`)
} else if (!layoutMatch && replaceMatch) {
res.push(`${urlprefix}${dirent.name.replace(replaceMatch[1], '')}`)
}
} else {
const dirPath = path.join(directory, dirent.name)
if (dirent.isDirectory(dirPath) && !dirent.isSymbolicLink()) {
res.push(...parseUrlForPages(urlprefix + dirent.name + '/', dirPath))
if (dirent.isDirectory() && !dirent.isSymbolicLink()) {
res.push(...parseUrlForAppDir(urlprefix + dirent.name + '/', dirPath, pageExtensions))
}
}
})
Expand All @@ -67,82 +82,38 @@ function parseUrlForAppDir(urlprefix: string, directory: string) {
* - Removes query string
*/
export function normalizeURL(url: string) {
if (!url) {
return
if (url.includes('?')) {
url = url.split('?')[0]
}
url = url.split('?', 1)[0]
url = url.split('#', 1)[0]
url = url = url.replace(/(\/index\.html)$/, '/')
// Empty URLs should not be trailed with `/`, e.g. `#heading`
if (url === '') {
return url
}
url = url.endsWith('/') ? url : url + '/'
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
}
const urlWithoutExtension = url.replace(/\.html$/, '')
// Encode all characters except `/` and `.`
const encoded = urlWithoutExtension
.split('/')
.map((segment) => encodeURIComponent(segment))
.join('/')
.replace(/%2E/g, '.')

// The last segment (if it's a leaf) should be ignored.
if (
(segment === 'page' || segment === 'route') &&
index === segments.length - 1
) {
return pathname
}
const withoutTrailingSlash =
encoded === '/' ? '/' : encoded.replace(/\/$/, '')
return withoutTrailingSlash
Comment on lines +85 to +99
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new normalizeURL implementation appears to be missing several behaviors from the original function:

  1. No handling for hash fragments (url.split('#')[0])
  2. Different trailing slash behavior than the original implementation
  3. No special handling for empty URLs
  4. No null/undefined check before processing the URL

For compatibility with existing code, consider preserving these behaviors from the original implementation:

if (!url) {
  return
}
url = url.split('?', 1)[0]
url = url.split('#', 1)[0]
// Empty URLs should not be trailed with `/`
if (url === '') {
  return url
}

The trailing slash behavior should also match the original function which added a trailing slash if not present.

Suggested change
if (url.includes('?')) {
url = url.split('?')[0]
}
url = url.split('?', 1)[0]
url = url.split('#', 1)[0]
url = url = url.replace(/(\/index\.html)$/, '/')
// Empty URLs should not be trailed with `/`, e.g. `#heading`
if (url === '') {
return url
}
url = url.endsWith('/') ? url : url + '/'
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
}
const urlWithoutExtension = url.replace(/\.html$/, '')
// Encode all characters except `/` and `.`
const encoded = urlWithoutExtension
.split('/')
.map((segment) => encodeURIComponent(segment))
.join('/')
.replace(/%2E/g, '.')
// The last segment (if it's a leaf) should be ignored.
if (
(segment === 'page' || segment === 'route') &&
index === segments.length - 1
) {
return pathname
}
const withoutTrailingSlash =
encoded === '/' ? '/' : encoded.replace(/\/$/, '')
return withoutTrailingSlash
if (!url) {
return
}
// Remove query parameters and hash fragments
url = url.split('?', 1)[0]
url = url.split('#', 1)[0]
// Empty URLs should not be trailed with `/`
if (url === '') {
return url
}
const urlWithoutExtension = url.replace(/\.html$/, '')
// Encode all characters except `/` and `.`
const encoded = urlWithoutExtension
.split('/')
.map((segment) => encodeURIComponent(segment))
.join('/')
.replace(/%2E/g, '.')
// Ensure trailing slash behavior matches original implementation
const withTrailingSlash = encoded.endsWith('/') ? encoded : `${encoded}/`
return withTrailingSlash

Spotted by Graphite Agent

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

}

return `${pathname}/${segment}`
}, '')
)
export function normalizeAppPath(path: string) {
const withoutTrailingSlash = path.replace(/\/$/, '')
return withoutTrailingSlash === '' ? '/' : withoutTrailingSlash
Comment on lines +102 to +104

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge normalizeAppPath no longer strips grouping/parallel segments

The new normalizeAppPath implementation only trims a trailing slash, whereas the previous version removed group segments like (dashboard), parallel segments (@foo), and the terminal page/route leaf. Because getUrlFromAppDirectory still relies on normalizeAppPath to normalize the paths collected from the app directory, URLs containing these segment types will now produce regexes that include the literal group names and won’t match real request paths (e.g. /user/[id] won’t match / (dashboard)/user/[id]/page). This causes the rule to miss links for most app-directory routes that use grouping.

Useful? React with 👍 / 👎.

}

/**
* Gets the possible URLs from a directory.
*/
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)}$`
Expand All @@ -156,13 +127,14 @@ export function getUrlFromPagesDirectories(

export function getUrlFromAppDirectory(
urlPrefix: string,
directories: string[]
directories: string[],
pageExtensions?: string[]
) {
return Array.from(
// De-duplicate similar pages across multiple directories.
new Set(
directories
.map((directory) => parseUrlForAppDir(urlPrefix, directory))
.map((directory) => parseUrlForAppDir(urlPrefix, directory, pageExtensions))
.flat()
.map(
// Since the URLs are normalized we add `^` and `$` to the RegExp to make sure they match exactly.
Expand Down Expand Up @@ -197,3 +169,48 @@ function ensureLeadingSlash(route: string) {
function isGroupSegment(segment: string) {
return segment[0] === '(' && segment.endsWith(')')
}

function removeGroupSegments(segment: string): string {
return segment.replace(/\([^)]*\)\//g, '').replace(/^!\(/, '(')
}

export function getUrlsForFile(
file: string
): Array<{
file: string
urls: string[]
}> | null {
// Handle both `pages` and `app` files
const pagesMatch = file.match(/[\\/]pages[\\/](.*?)$/)
if (pagesMatch) {
const path = pagesMatch[1].replace(/\\/g, '/').replace(/\.[^.]+$/, '')
return [{ file, urls: [ensureLeadingSlash(path)] }]
}

const appMatch = file.match(/[\\/]app[\\/](.*?)(?:[\\/])?(?:page|layout)\.[^.]+$/)
if (appMatch) {
let path = appMatch[1]
.replace(/\\/g, '/')
.split('/')
.filter((segment) => !isGroupSegment(segment))
.map(removeGroupSegments)
.join('/')

path = path.replace(/\/$/, '')
return [{ file, urls: [ensureLeadingSlash(path)] }]
}

return null
}

export const fsExistsCacheGet = (cache: {}, key: string) => {
return cache[key]
}

export const fsExistsCacheSet = (
cache: {},
key: string,
value: boolean
) => {
cache[key] = value
}
Loading