Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

@next/next/no-html-link-for-pages rule does not work with pageExtensions #53473

Open
1 task done
nnmax opened this issue Aug 2, 2023 · 6 comments · May be fixed by #54474 or #68099
Open
1 task done

@next/next/no-html-link-for-pages rule does not work with pageExtensions #53473

nnmax opened this issue Aug 2, 2023 · 6 comments · May be fixed by #54474 or #68099
Labels
good first issue Easy to fix issues, good for newcomers Linting Related to `next lint` or ESLint with Next.js.

Comments

@nnmax
Copy link
Contributor

nnmax commented Aug 2, 2023

Verify canary release

  • I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
      Platform: darwin
      Arch: arm64
      Version: Darwin Kernel Version 22.5.0: Thu Jun  8 22:22:20 PDT 2023; root:xnu-8796.121.3~7/RELEASE_ARM64_T6000
    Binaries:
      Node: 18.14.2
      npm: 9.8.1
      Yarn: 1.22.19
      pnpm: 8.3.1
    Relevant Packages:
      next: 13.4.13-canary.9
      eslint-config-next: 13.4.12
      react: 18.2.0
      react-dom: 18.2.0
      typescript: 5.1.3
    Next.js Config:
      output: N/A

Which area(s) of Next.js are affected? (leave empty if unsure)

ESLint (eslint-config-next)

Link to the code that reproduces this issue or a replay of the bug

https://github.com/nnmax/next-eslint-config-reproduction-app

To Reproduce

  1. git clone https://github.com/nnmax/next-eslint-config-reproduction-app.git
  2. cd next-eslint-config-reproduction-app
  3. npm install
  4. npm run lint
  5. Observe the ESLint output from the terminal.

Describe the Bug

The @next/next/no-html-link-for-pages rule doesn't work after configuring custom pageExtensions.

Expected Behavior

It should recognize pageExtensions.

image

Which browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response
@nnmax nnmax added the bug Issue was opened via the bug report template. label Aug 2, 2023
@balazsorban44 balazsorban44 added Linting Related to `next lint` or ESLint with Next.js. good first issue Easy to fix issues, good for newcomers labels Aug 2, 2023
@balazsorban44
Copy link
Member

Currently, the ESLint rule is indeed unaware of this configuration.

See

next.js/packages/eslint-plugin-next/src/rules/no-html-link-for-pages.ts

Line 95 in e757cac

const pageUrls = getUrlFromPagesDirectories('/', foundPagesDirs)

@balazsorban44 balazsorban44 removed the bug Issue was opened via the bug report template. label Aug 2, 2023
@nnmax
Copy link
Contributor Author

nnmax commented Aug 3, 2023 •
edited
Loading

Can I make a PR to accomplish this? I would be happy to help.

I readed the source code and found only one difficulty, how to get the user configured pageExtensions parameter. I have two ideas for this:

  • Get the pageExtensions parameter from the user's next.config.js or next.config.mjs configuration file.
    • Use the loadConfig function to get the configuration. But unfortunately, this function is an async function, and the ESLint create function does not support async calls. So this solution does not work.
    • Read the configuration file via require(). If the user is only using the CommonJS and exporting a configuration object, this is easy enough to get the pageExtensions parameter. But if the user is using an ES Module or exporting a sync or async function, I can't get the pageExtensions parameter. Because the require() function doesn't support dynamic import of an ES Module. So this solution also fails.
  • Provide an ESLint option named pageExtensions.
    • This is the only option that works so far.

This is my current idea, feel free to discuss it.

@abhishek1810
Copy link

Hello, I am new to open source and want to work on this issue. Do I need to get this assigned first or should I just fork and start working directly?

@jessfeliciano
Copy link

@abhishek1810

It looks like this is already being worked on for a duplicate ticket. Here's the PR: #51783

Maybe reviewing and commenting on that PR first would be helpful.

This was referenced Aug 24, 2023
Closed
Open
@DerickT02
Copy link

Is this still open? I would be happy to fix this issue if someone can guide me where in the project this error is being produced.

@Santosh130602

This comment has been minimized.

Sign in to view
@doz13189 doz13189 linked a pull request Jul 24, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Easy to fix issues, good for newcomers Linting Related to `next lint` or ESLint with Next.js.
Projects
None yet
Milestone
No milestone
6 participants
@balazsorban44 @jessfeliciano @nnmax @DerickT02 @abhishek1810 @Santosh130602