Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

no-html-link-for-pages eslint rule uses cwd instead of project directory #26330

Closed
brandonchinn178 opened this issue Jun 18, 2021 · 9 comments
Closed

no-html-link-for-pages eslint rule uses cwd instead of project directory #26330

brandonchinn178 opened this issue Jun 18, 2021 · 9 comments
Labels
bug Issue was opened via the bug report template.

Comments

@brandonchinn178
Copy link
Contributor

What version of Next.js are you using?

11.0.1-canary.4

What version of Node.js are you using?

14.15.5

What browser are you using?

Chrome

What operating system are you using?

macOS

How are you deploying your application?

vercel

Describe the Bug

I have a project with separate directories for client and server (my project has two servers: nextjs and graphql), with the path client/next.config.js. Running eslint from the root directory using the next eslint plugin shows

Pages directory cannot be found at <repo>/pages or <repo>/src/pages. If using a custom path, please configure with the no-html-link-for-pages rule in your eslint config file

This happens whether I add the plugin in the top-level .eslintrc or in the client/.eslintrc

Expected Behavior

Rule should look for pages directory in the same directory as next.config.js (or some other marker of a NextJS project). Alternatively, rule should look for pages directory in the same directory as the .eslintrc file it's added in.

To Reproduce

Create a NextJS project in a subdirectory and run linting from the parent directory.
@brandonchinn178 brandonchinn178 added the bug Issue was opened via the bug report template. label Jun 18, 2021
@brandonchinn178
Copy link
Contributor Author

To be clear, explicitly setting the path like

'@next/next/no-html-link-for-pages': ['error', 'client/pages']

works. But if possible, it would be great if the rule could automatically find the pages directory from the NextJS project root

@mrmckeb
Copy link
Contributor

mrmckeb commented Aug 10, 2021 •
edited
Loading

Hey @brandonchinn178, I'm currently working on something around this. Are you working from a monorepo? Or just a custom folder in a project?

@mrmckeb mrmckeb mentioned this issue Aug 10, 2021
Merged
7 tasks
@brandonchinn178
Copy link
Contributor Author

Yes, I'm in a monorepo

@mrmckeb
Copy link
Contributor

mrmckeb commented Aug 10, 2021

Great, I think the solution in #27918 will help - but I'm not sure about auto-detecting, as that's more error-prone.

Please let me know what you think.

@brandonchinn178
Copy link
Contributor Author

Looks great! Are there any other rules this is applicable for? Even if not, it's good/it makes sense to have this as a global setting.

Is it possible to have it default to the location of the eslintrc file it's in? for my usecase, i wouldnt even need to set the setting then

@mrmckeb
Copy link
Contributor

mrmckeb commented Aug 10, 2021 •
edited
Loading

Are there any other rules this is applicable for?

I don't think so, but I imagine there will be - and like you, I think it makes more sense to have this as a global setting.

So the way we're rolling out our updated ESLint configs at Vercel is that we have a root config, and then use overrides to set any per-workspace settings. So everything is root-level in the monorepo.

I'm guessing (correct me if I'm wrong) that you're running ESLint from the root of the repo, but the ESLint config file is within the workspace. What happens if you run the ESLint command from within the workspace? I imagine that might work because ESLint's context would then be that workspace.

@brandonchinn178
Copy link
Contributor Author

I'm guessing (correct me if I'm wrong) that you're running ESLint from the root of the repo, but the ESLint config file is within the workspace.

Yes, that's correct.

What happens if you run the ESLint command from within the workspace? I imagine that might work because ESLint's context would then be that workspace.

It would probably work, but I think it would break the dev workflow, or at least complicate the scripts that run eslint.

@mrmckeb
Copy link
Contributor

mrmckeb commented Aug 11, 2021

Yes, I understand. I'll give it thought and see if there are any smart ways to automate this in future.

kodiakhq bot pushed a commit that referenced this issue Aug 11, 2021
@mrmckeb
Add rootDir setting to eslint-plugin-next (#27918)
4cd45aa 
## Introduction

This PR enables setting a `rootDir` for a Next.js project, and follows the same pattern used by [`@typescript-eslint/parser`](https://github.com/typescript-eslint/typescript-eslint/tree/master/packages/parser#parseroptionsproject).

## Details

Previously, users had to pass paths to the rule itself.
```js
module.exports = {
  rules: { 
    "@next/next/no-html-link-for-pages": [
      "error",
      // This could be a string, or array of strings.
      "/packages/my-app/pages",
    ],
  },
};
```

With this PR, this has been simplified (the previous implementation still works as expected).
```js
module.exports = {
  settings: { 
    next: {
      rootDir: "/packages/my-app",
    },
  },
  rules: { 
    "@next/next/no-html-link-for-pages": "error",
  },
};
```

Further, this rule allows the use of globs, again aligning with `@typescript-eslint/parser`.
```js
module.exports = {
  settings: { 
    next: {
      // Globs
      rootDir: "/packages/*",
      rootDir: "/packages/{app-a,app-b}",

      // Arrays
      rootDir: ["/app-a", "/app-b"],
      
      // Arrays with globs
      rootDir: ["/main-app", "/other-apps/*"],
  },
};
```

This enables users to either provide per-workspace configuration with overrides, or to use globs for situations like monorepos where the apps share a domain (micro-frontends).

This doesn't solve, but improves #26330.

## Feature

- [x] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [x] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`

## Documentation / Examples

- [x] Make sure the linting passes
flybayer pushed a commit to blitz-js/next.js that referenced this issue Aug 19, 2021
@mrmckeb
Add rootDir setting to eslint-plugin-next (vercel#27918)
5972726 
## Introduction

This PR enables setting a `rootDir` for a Next.js project, and follows the same pattern used by [`@typescript-eslint/parser`](https://github.com/typescript-eslint/typescript-eslint/tree/master/packages/parser#parseroptionsproject).

## Details

Previously, users had to pass paths to the rule itself.
```js
module.exports = {
  rules: { 
    "@next/next/no-html-link-for-pages": [
      "error",
      // This could be a string, or array of strings.
      "/packages/my-app/pages",
    ],
  },
};
```

With this PR, this has been simplified (the previous implementation still works as expected).
```js
module.exports = {
  settings: { 
    next: {
      rootDir: "/packages/my-app",
    },
  },
  rules: { 
    "@next/next/no-html-link-for-pages": "error",
  },
};
```

Further, this rule allows the use of globs, again aligning with `@typescript-eslint/parser`.
```js
module.exports = {
  settings: { 
    next: {
      // Globs
      rootDir: "/packages/*",
      rootDir: "/packages/{app-a,app-b}",

      // Arrays
      rootDir: ["/app-a", "/app-b"],
      
      // Arrays with globs
      rootDir: ["/main-app", "/other-apps/*"],
  },
};
```

This enables users to either provide per-workspace configuration with overrides, or to use globs for situations like monorepos where the apps share a domain (micro-frontends).

This doesn't solve, but improves vercel#26330.

## Feature

- [x] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [x] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`

## Documentation / Examples

- [x] Make sure the linting passes
@oste oste mentioned this issue Oct 27, 2021
Closed
@jankaifer
Copy link
Contributor

It seems that we don't have anything actionable now (it's not really a bug).
We will track it as a feature request in discussions.

@vercel vercel locked and limited conversation to collaborators Nov 29, 2022
@jankaifer jankaifer converted this issue into discussion #43526 Nov 29, 2022

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Assignees
No one assigned
Labels
bug Issue was opened via the bug report template.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@mrmckeb @brandonchinn178 @jankaifer