softarc-consulting / sheriff

Lightweight Modularity for TypeScript Projects
MIT License
188 stars 14 forks source link

Support eslint flat-config #87

Closed schreibse closed 1 month ago

schreibse commented 4 months ago

I installed rc-v18 of @angular/eslint today in my Angular 18 workspace after reading that no more breaking changes are to be expected https://github.com/angular-eslint/angular-eslint/pull/1830#issuecomment-2135134938

After migrating my .eslintrc.json to the new flat config, I noticed sheriff is currently not supporting that:

const eslint = require('@eslint/js');
const tseslint = require('typescript-eslint');
const angulareslint = require('angular-eslint');
const softarceslint = require('@softarc/eslint-plugin-sheriff');

module.exports = tseslint.config(
  <other configs>,
  {
    files: ['*.ts'],
    extends: [softarceslint.configs.default],
  }
);

Running eslint . results in A config object is using the "parser" key, which is not supported in flat config system. Flat config uses "languageOptions.parser" to override the default parser. https://github.com/softarc-consulting/sheriff/blob/main/packages/eslint-plugin/src/lib/config.ts

Of course that was to be expected due to @angular/eslint not supporting it before as well. I guess the backwards compatibility to the json config is crucial.

I am not quite sure yet whats the best option to make it work with both configuration formats but I would be happy to help with a pr with some help

rainerhahnekamp commented 4 months ago

Hello, thanks for the info. I was not aware that @angular/eslint targets already the flat config.

If you want to help out by contributing a PR, you are more than welcome to do so.

WolfSoko commented 2 months ago

@rainerhahnekamp, I see that you’ve already opened a PR. Do you anticipate completing it in the near future?

rainerhahnekamp commented 2 months ago

@WolfSoko, I started going through the work @schreibse just this week. We'll stick to Stephan's strategy (renaming the old into legacy), but I have to fix some of the environment (nx doesn't support flat config yet) and also make sure that the tests are inline. I guess we can publish it next week.

slawomir-marek commented 2 months ago

It was nice experience to find a problem in my sheriff config, search a solution, open this topic and see (in real time) that fix is on the way (build was in progress). @rainerhahnekamp do you have a plan to release new npm package in nearest future?

rainerhahnekamp commented 2 months ago

@slawomir-marek I am currently updating the docs. If it is not released today then within the next two days.

rainerhahnekamp commented 2 months ago

@slawomir-marek @WolfSoko @schreibse. 0.16.0 is out. Please give it a try in your projects and let us know if you encounter any issues. Thanks

slawomir-marek commented 2 months ago

@rainerhahnekamp I still have problem with sheriff configuration. There was a change in eslint types in last months and it looks like sheriff configuration is not compatible with these types.

Type '{ languageOptions: { parser: any; sourceType: string; }; plugins: { '@softarc/sheriff': { rules: { 'deep-import': RuleModule; 'dependency-rule': RuleModule; }; }; }; rules: { '@softarc/sheriff/deep-import': string; '@softarc/sheriff/dependency-rule': string; }; }' is not assignable to type 'Config'. The types of 'languageOptions.sourceType' are incompatible between these types. Type 'string' is not assignable to type 'SourceType | undefined'.

I'm using newest Angular 18.1.1 (the same as you in angular-iv test project)

rainerhahnekamp commented 2 months ago

@slawomir-marek, can you verify that you are running on 0.16 via npx sheriff version. Do you use .eslintrc.json or eslint.config.js? What versions of ESLint (including TypeScript) do you have?

Please post the full content of your eslint config file.

slawomir-marek commented 2 months ago

sheriff version

npx sheriff version
0.16.0

devDependencies

"@types/eslint": "^9.6.0",
"@types/eslint-config-prettier": "~6.11.3",
"angular-eslint": "18.1.0",
"eslint": "^9.6.0",
"eslint-config-prettier": "^9.1.0",
"eslint-plugin-prefer-arrow": "^1.2.3",
"eslint-plugin-prettier": "^5.1.3",
"eslint-plugin-simple-import-sort": "^12.1.0",
"typescript-eslint": "8.0.0-alpha.38"

I'm using eslint.config.js

// @ts-check
const eslint = require('@eslint/js');
const tseslint = require('typescript-eslint');
const angular = require('angular-eslint');
const eslintPluginPrettierRecommended = require('eslint-plugin-prettier/recommended');
const simpleImportSort = require('eslint-plugin-simple-import-sort');
const sheriff = require('@softarc/eslint-plugin-sheriff');

module.exports = tseslint.config(
  {
    files: ['**/*.ts'],
    extends: [
      eslint.configs.recommended,
      ...tseslint.configs.recommended,
      ...tseslint.configs.stylistic,
      ...angular.configs.tsRecommended,
    ],
    plugins: {
      'simple-import-sort': simpleImportSort,
    },
    processor: angular.processInlineTemplates,
    rules: {
      '@angular-eslint/component-class-suffix': [
        'error',
        {
          suffixes: ['Page', 'Component'],
        },
      ],
      'simple-import-sort/exports': 'error',
      'simple-import-sort/imports': 'error',
      'prettier/prettier': [
        'error',
        {
          singleQuote: true,
        },
      ],
      '@typescript-eslint/no-explicit-any': 'warn',
      '@typescript-eslint/no-require-imports': 'warn',
      'no-unsafe-optional-chaining': 'warn',
    },
  },
  {
    files: ['*.ts'],
    extends: [sheriff.configs.all],
  },
  {
    files: ['**/*.html'],
    extends: [...angular.configs.templateRecommended],
    rules: {
      'prettier/prettier': [
        'error',
        {
          singleQuote: false,
        },
      ],
    },
  },
  eslintPluginPrettierRecommended
);
rainerhahnekamp commented 2 months ago

Okay, can you give it try and maybe upgrade "typescript-eslint" to "8.0.0-alpha.41"?

Let me know what happens

slawomir-marek commented 2 months ago

Nothing has changed

rainerhahnekamp commented 2 months ago

@slawomir-marek, thanks. My guess is that you have an issue with @types/eslint. Can you remove that? Same with prettier?

slawomir-marek commented 2 months ago

I've removed these packages - nothing has changed. Problem still exist - type from package typescript-eslint looks like that:

interface LanguageOptions {
    ecmaVersion?: EcmaVersion;
    globals?: GlobalsConfig | undefined;
    parser?: Parser;
    parserOptions?: ParserOptions | undefined;
    sourceType?: SourceType;
}

sheriff config looks like that

declare const configs: {
   ...
    all: {
        languageOptions: {
            parser: import("@typescript-eslint/utils/dist/ts-eslint").Parser.LooseParserModule;
            sourceType: string;
        };
        ...
     }

As you can see type of sourceType field is different.

rainerhahnekamp commented 2 months ago

@slawomir-marek please download version 0.16.1.

slawomir-marek commented 2 months ago

@rainerhahnekamp I checked version `0.16.1'.

I had to downgrade eslint to install it. My dependencies right now looks like that:

"@types/eslint": "^8.56.10",
"@types/eslint-config-prettier": "~6.11.3",
"angular-eslint": "18.1.0",
"eslint": "^8.57.0",
"eslint-config-prettier": "^9.1.0",
"eslint-plugin-prefer-arrow": "^1.2.3",
"eslint-plugin-prettier": "^5.1.3",
"eslint-plugin-simple-import-sort": "^12.1.0",
"typescript-eslint": "8.0.0-alpha.38"

There are no ts errors in eslint.config.js so it looks that types are compatible. But...

I don't see sheriff errors on VSCode levels as well as in terminal when I run `lint' command.

To verify it I prepared one function that break boundaries defined in sheriff configuration and tried to verify it directly from terminal

$ npx sheriff verify src/app/media/utils-common/media-helper.ts

Verification Report

Issues found:
  Total Invalid Files: 2
  Total Deep Imports: 2
  Total Dependency Rule Violations: 1
----------------------------------

|-- src\app\media\utils-common\media-helper.ts
|   |-- Deep Imports
|   |   |-- ./../data/entities/media.mapper
|   |   |-- ./../data/entities/media.model
|-- src\app\media\data\entities\media.mapper.ts
|   |-- Dependency Rule Violations
|   |   |-- from tags domain:media,type:data to noTag

As you can see - here I have valuable information. So from my perspective it looks like sheriff rules are not used by eslint

rainerhahnekamp commented 2 months ago

@slawomir-marek Is there a way I can take a look at the project via screen sharing? Are you on LinkedIn? If yes, can you contact me there?

slawomir-marek commented 2 months ago

@rainerhahnekamp It's not possible. But I can create sample repo with the same stack/packages and check if problem occurs there too. I'll let you know here or on LinkedIn.

rainerhahnekamp commented 1 month ago

Closing that one now. @slawomir-marek if you still have the issue, let me know.