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.

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

Override eslint rules in the root eslintrc.json #5866

Closed
AnkitaSood opened this issue Jun 2, 2021 · 10 comments
Closed

Override eslint rules in the root eslintrc.json #5866

AnkitaSood opened this issue Jun 2, 2021 · 10 comments
Assignees
Labels
outdated scope: linter Issues related to Eslint support in Nx type: question / discussion

Comments

@AnkitaSood
Copy link

Current Behavior

What is the behavior that currently you experience?
Overriding @angular-eslint rules in the base eslintrc.json file doesn't work as expected. Overriding rules like @angular-eslint/no-empty-lifecycle-method, @angular-eslint/no-host-metadata-property etc. is impossible.

Expected Behavior

What is the behavior that you expect to happen?
Rules should be configurable at the base eslintrc.json level.

Steps to Reproduce

Current .eslintrc.json :

{
  "root": true,
  "ignorePatterns": ["**/*"],
  "plugins": ["@nrwl/nx"],
  "overrides": [
  ...
    {
      "files": ["*.ts"],
      "extends": ["plugin:@nrwl/nx/angular"],
      "rules": {
        "@angular-eslint/no-empty-lifecycle-method": "warn",
        "@angular-eslint/no-host-metadata-property": "warn"
      },
      "plugins": ["@angular-eslint/eslint-plugin"]
    },
  ]
}

Adding similar override to a library's .eslintrc.json converts the error into a warning. This is what works for a library's .eslintrc.json

{
  "extends": ["../../.eslintrc.json"],
  "ignorePatterns": ["!**/*"],
  "overrides": [
    {
      "files": ["*.ts"],
      "extends": ["plugin:@nrwl/nx/angular", "plugin:@angular-eslint/template/process-inline-templates"],
      "parserOptions": {
        "project": ["libs/docs-site/tsconfig.*?.json"]
      },
      "rules": {
        "@angular-eslint/directive-selector": [
          "error",
          {
            "type": "attribute",
            "prefix": "foo",
            "style": "camelCase"
          }
        ],
        "@angular-eslint/component-selector": [
          "error",
          {
            "type": "element",
            "prefix": "foo",
            "style": "kebab-case"
          }
        ],
        "@angular-eslint/no-empty-lifecycle-method": "warn",
        "@angular-eslint/no-host-metadata-property": "warn"
      },
      "plugins": ["@angular-eslint/eslint-plugin", "@typescript-eslint"]
    },
    {
      "files": ["*.html"],
      "extends": ["plugin:@nrwl/nx/angular-template"],
      "rules": {}
    }
  ]
}

Instead of having to add this for each library, the override in the base .eslintrc.json should work.

Environment

nx report

@nrwl/angular : 12.3.5
@nrwl/cli : 12.3.5
@nrwl/cypress : 12.3.5
@nrwl/devkit : 12.3.5
@nrwl/eslint-plugin-nx : 12.3.5
@nrwl/express : Not Found
@nrwl/jest : 12.3.5
@nrwl/linter : 12.3.5
@nrwl/nest : Not Found
@nrwl/next : Not Found
@nrwl/node : Not Found
@nrwl/react : Not Found
@nrwl/schematics : 8.12.11
@nrwl/tao : 12.3.5
@nrwl/web : Not Found
@nrwl/workspace : 12.3.5
@nrwl/storybook : 12.3.5
@nrwl/gatsby : Not Found
typescript : 4.2.4
@GaryB432
Copy link

I posted this over on StackOverfow a few days ago.

@AnkitaSood
Copy link
Author

I posted this over on StackOverfow a few days ago.

I've posted on stack overflow as well https://stackoverflow.com/questions/67797166/esslint-configuration-for-nx-angular-project-no-host-metadata-property

@vsavkin vsavkin added the scope: linter Issues related to Eslint support in Nx label Jun 17, 2021
@meeroslav meeroslav self-assigned this Jun 21, 2021
@meeroslav
Copy link
Contributor

meeroslav commented Jun 21, 2021

Thank you for this question @AnkitaSood. This is something we witnessed ourselves at Nrwl while working on projects. Unfortunately, this is not a bug, but rather a feature. Although it might not seem that way.

Why this doesn't work on root level?

Your lib's .eslintrc.json config overrides root one with "extends": ["plugin:@nrwl/nx/angular", "plugin:@angular-eslint/template/process-inline-templates"]. Part of @nrwl/nx/angular rules are also the ones you are trying to set in the root. This means that whatever you set in root will be overridden by lib's overrides/extends.

Why this works on lib level?

By applying your overrides after the extends, you are effectively overriding the override.

Will there be a fix?

Unfortunately not, as this would mean we would have to change the default configuration of @nrwl/nx/angular. The current ruleset is result of investigation of what works for the majority of use cases/projects. There are unfortunately projects (like yours) where this doesn't fully apply.

Solution?

As you found out already, a solution would be to apply changes to your project's .eslintrc.json. If you have several changes on the base you could write something like this:

{
  "extends": ["../../.eslintrc.json"],
  "ignorePatterns": ["!**/*"],
  "overrides": [
    {
      "files": ["*.ts"],
      "extends": [
        "plugin:@nrwl/nx/angular",
        "plugin:@angular-eslint/template/process-inline-templates",
        "../../eslintrc-custom-overrides.json"
      ],
   },
   ...
  ],
  ...
}

Where eslintrc-custom-overrides would look like this:

{
  "overrides": [
    {
      "files": ["*.ts"],
      "rules": {
        "@angular-eslint/no-empty-lifecycle-method": "warn",
        "@angular-eslint/no-host-metadata-property": "warn"
      }
    }
  ]
}

Maybe one day there will be a way how to deal with such issues in an elegant way, but currently limitation of eslint does not allow it.

Hope this helps, nevertheless.

@meeroslav
Copy link
Contributor

@meeroslav meeroslav changed the title convert-tslint-to-eslint - override rules Override eslint rules in the root eslintrc.json Jun 22, 2021
@vincentpalita
Copy link

vincentpalita commented Jun 22, 2021

@meeroslav thank you for the heads up. However I think there should be then a different way to create angular libs with Nx as most of the time I think the default project eslint configuration would be used and very little time people would want to change it.
With the current configuration, it is very unclear that all libs with their eslint config just don't take any default angular-eslint configuration.
With hundred of libs, this is for me, not manageable to touch every lib's eslint config. I would rather use the default configuration as it would clearly plays its role of making the code style standard in the whole monorepo.

@meeroslav
Copy link
Contributor

@vincentpalita I understand your position and we have carefully assessed the situation. The reality is that majority of the projects do not consist only of angular apps and libs. More common is to have at least one nest, express or plain typescript library. In those cases having the angular-specific extends on the root level could mess up with all the non-angular projects.

When approaching this situation we usually ask three questions:

  • Does your workspace consist only of Angular projects?
  • Will your future projects consist of only Angular code?
  • Do you generate projects more often than you update nx?

If answer to all of these is Yes then absolutely, you should move all those breaking overrides to root level and loosen up libraries. At the end of the day, only you (and your team) can decide what approach generates less boilerplate for your project.

@vincentpalita
Copy link

@meeroslav thank you for you reply. This clarify your strategy and I like it that way.
What would you say if when creating the lib we could simply just tell the CLI if we want an eslint override or not?

@meeroslav
Copy link
Contributor

I would say: Please make a detailed feature request, link this issue and I'll try to work on it as soon as possible. 😃

Also, you are more than welcome to start working on it yourself if your time permits. We love community PRs ❤️

@johnculviner
Copy link

johnculviner commented Feb 3, 2022

Here is what I'm doing. Copy pasting rules across a bunch of projects was not an acceptable solution for me. The key for me was not using the file based overrides and do extends instead. It works great and does exactly what I want it to do (and not seeing any downsides):

A NextJS / React UI project
image

A config for a NodeJS GraphQL project
image

A config for all NextJS / UI Projects
image

A config for everything that can be overridden by anything above
image

@github-actions
Copy link

This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated scope: linter Issues related to Eslint support in Nx type: question / discussion
Projects
None yet
Development

No branches or pull requests

6 participants