next.js icon indicating copy to clipboard operation
next.js copied to clipboard

fix(eslint-plugin): respect custom pageExtensions in no-html-link-for…

Open viniciuspizettadesouza opened this issue 6 months ago • 6 comments

What?

Fixes a bug in the ESLint rule @next/next/no-html-link-for-pages, where it did not recognize internal pages using custom pageExtensions defined in next.config.js.

Why?

When users configure custom extensions (e.g., page.tsx, mdx), the rule still relied on a hardcoded default list (['js', 'jsx', 'ts', 'tsx']). This led to false positives by incorrectly flagging valid internal links as invalid.

This PR aligns the rule behavior with how Next.js resolves routes — respecting the developer’s intended page structure.

How?

  • Introduced a getPageExtensions() utility in eslint-plugin-next to dynamically load and normalize pageExtensions from next.config.js
  • Designed getPageExtensions() as a self-contained closure to:
    • Resolve and memoize config parsing once (avoiding repeated file system reads)
    • Cleanly encapsulate fallback logic and default handling
    • Allow internal utils like parseUrlForPages and parseUrlForAppDir to call it without receiving external params
  • Updated the ESLint rule to use this utility internally
  • Added unit tests to verify behavior when:
    • next.config.js is missing → uses default
    • Valid custom extensions → returned and applied
    • Invalid config (non-array) → safely falls back
  • Refactored getRegexPatterns() to receive page extensions from getPageExtensions(), ensuring:
    • ext and index patterns are correctly built from actual user config
    • Shared regex logic is cleanly encapsulated in a single place
    • All downstream consumers use a consistent pattern set

Benefits of the Closure Design

By using getPageExtensions() as a lazy, memoized closure:

  • There's no need to manually pass extensions through multiple internal layers
  • Consistency is guaranteed across all internal utility functions
  • We reduce boilerplate and cognitive overhead while improving testability
  • getRegexPatterns() now cleanly depends on a single canonical source for extensions

Additional Context

This change complements the fix in #68770, which resolved structural matching issues for App Router, but did not handle pageExtensions in pages/ linting.

It also supersedes partial or outdated approaches in:

Fixes

Fixes #53473

viniciuspizettadesouza avatar Jun 01 '25 22:06 viniciuspizettadesouza

⚠️ No Changeset found

Latest commit: d9e69bc83530693636ef7049be7c4ccc3e91c5fe

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

changeset-bot[bot] avatar Jun 01 '25 22:06 changeset-bot[bot]

Allow CI Workflow Run

  • [ ] approve CI run for commit: d9e69bc83530693636ef7049be7c4ccc3e91c5fe

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

ijjk avatar Jun 01 '25 22:06 ijjk

This doesn't work with next.config.ts.

darthmaim avatar Jun 01 '25 23:06 darthmaim

This doesn't work with next.config.ts.

Good catch! You're right this currently only supports next.config.js. To support next.config.ts, we’d need either ts-node or a custom loader at runtime, which isn’t ideal in a linting context.

I can update the implementation to try both .js and .ts if ts-node is available, or add a fallback warning.

Let me know your thoughts

viniciuspizettadesouza avatar Jun 01 '25 23:06 viniciuspizettadesouza

Hi @balazsorban44 @ijjk is there something missing to get a CI approval?

viniciuspizettadesouza avatar Jun 04 '25 09:06 viniciuspizettadesouza

Hi @balazsorban44 @ijjk is there something missing to get a CI approval?

viniciuspizettadesouza avatar Jun 13 '25 13:06 viniciuspizettadesouza

Hi @balazsorban44 @ijjk, is there anything missing on my side to get CI approval or move this PR (#80035) forward?

Thanks!

viniciuspizettadesouza avatar Jul 08 '25 02:07 viniciuspizettadesouza

Hi @viniciuspizettadesouza, I'm working on this issue as part of my open source contribution course assignment. I noticed the PR doesn't support next.config.ts as mentioned by @darthmaim. I've implemented this fix and created a PR to your branch. Would you be open to reviewing and merging it to help move your PR forward?

alviarm avatar Aug 08 '25 07:08 alviarm