ngrx / platform

Reactive State for Angular
https://ngrx.io
Other
8.05k stars 1.98k forks source link

add support for plain eslint 9 flat config without tseslint helper #4550

Closed pumano closed 1 month ago

pumano commented 1 month ago

Which @ngrx/* package(s) are relevant/related to the feature request?

eslint-plugin

Information

currently we have:

const tseslint = require("typescript-eslint");
const ngrx = require("@ngrx/eslint-plugin/v9");

module.exports = tseslint.config({
  files: ["**/*.ts"],
  extends: [
    // 👇 Use all rules at once
    ...ngrx.configs.all,
    // 👇 Or only import the rules for a specific package
    ...ngrx.configs.store,
    ...ngrx.configs.effects,
    ...ngrx.configs.componentStore,
    ...ngrx.configs.operators,
    ...ngrx.configs.signals,
  ],
  rules: {
    // 👇 Configure specific rules
    "@ngrx/with-state-no-arrays-at-root-level": "warn",
  },
});

but it need tseslint wrapper for working because field "extends" does not exist in plain flat config.

Please add example / or provide support for plain eslint v9 flat config support

Describe any alternatives/workarounds you're currently using

not working

const ngrx = require("@ngrx/eslint-plugin/v9");

module.exports = [
{
  files: ["**/*.ts"],
 // extends not working here because it's legacy
  extends: [
    // 👇 Use all rules at once
    ...ngrx.configs.all,
  ],
  rules: {
    // 👇 Configure specific rules
    "@ngrx/with-state-no-arrays-at-root-level": "warn",
  },
});

looks like I need to use something like:

const ngrx = require("@ngrx/eslint-plugin/v9");

module.exports = [

{
  ...ngrx.configs.all,
  // then override config rules with 'warn'
  files: ["**/*.ts"],
  rules: {
    // 👇 Configure specific rules
    "@ngrx/with-state-no-arrays-at-root-level": "warn",
  },
});

but it's not working (multiple errors while trying to load rules)

I would be willing to submit a PR to fix this issue

dobripet commented 1 month ago

I got same issue, approach from docs does not work.

rainerhahnekamp commented 1 month ago

@pumano, @dobripet, just to be clear on this. The issue is that a dependency is required which is missing in our package.json?

If yes, ESLint expects that certain dependencies are already installed and that it is not necessary to explicitly add them to the package.json: https://typescript-eslint.io/developers/eslint-plugins/#package-dependencies

dobripet commented 1 month ago

@rainerhahnekamp

there is no issue for me, I was just blind. In docs is typescript-eslint config used to setup and I had the same setup as @pumano. I even got working the second approach he mentions wtih dependencies fixed.

(but linting single file when i add ngrx for me takes 18 seconds, so I still think there is something wrong with my setup)

timdeschryver commented 1 month ago

@pumano the NgRx ESLint package does require you to install typescript-eslint, which is why it's added a a peer dependency https://github.com/ngrx/platform/blob/ca1b2748845432778da20db95acfb9c19e5da368/modules/eslint-plugin/package.json#L49 I don't see that we'll make this optional in the near future. We can make this more clear in the docs.

@dobripet can you run the timing option https://eslint.org/docs/latest/extend/stats to detect if/which rule takes that long?

pumano commented 1 month ago

Problem only in

module.exports = tseslint.config({

Plain eslint config not contain It, only plain array of configs. And tseslint config contains "extends" while plain not. It's looks like your example is like compat

P.S. I just trying to migrate from my nx compat configs to plain flat config

pumano commented 1 month ago

looks like my fault. So I close issue