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

Issue configuring recommended as a plugin extension #48

Closed
woodrunner opened this issue Jan 10, 2025 · 6 comments
Closed

Issue configuring recommended as a plugin extension #48

woodrunner opened this issue Jan 10, 2025 · 6 comments
Labels
bug Something isn't working

Comments

@woodrunner
Copy link

woodrunner commented Jan 10, 2025

Describe the bug
On 7 dec 2024 you introduced a name property to the recommended config. I'm getting an error about this name property:

ESLint configuration in » ./tools/eslint-plugin-wgk/typescript/eslintrc.js » plugin:@smarttools/rxjs/recommended is invalid:

  • Unexpected top-level property "name".

When I remove it from your library it succeeds. Is it possible that this is because name is not a valid property of the configs field of a plugin?

I changed this in your library:

  Object.assign(plugin.configs, {
    recommended: {
      name: '@smarttools/rxjs/recommended',
      plugins: {
        '@smarttools/rxjs': plugin,
      },
      rules: {
        ...recommended.rules,
      },
    },
    'recommended-legacy': recommended,
  });

To this (without name property):

  Object.assign(plugin.configs, {
    recommended: {
      plugins: {
        '@smarttools/rxjs': plugin,
      },
      rules: {
        ...recommended.rules,
      },
    },
    'recommended-legacy': recommended,
  });

Repro
Version of @smarttools/eslint-plugin-rxjs you are using: 1.0.13

Additional context
My eslint.config.cjs file:

const rxjs = require('@smarttools/eslint-plugin-rxjs');

module.exports = [
  {
    ignores: ['**/dist']
  },
  {
    plugins: {
      '@smarttools/rxjs': rxjs,
      ...
    }
  },
  ...compat
    .config({
      extends: [
        './tools/eslint-plugin-wgk/typescript/eslintrc.js'
      ]
    })
    .map((config) => ({
      ...config,
      files: ['**/*.ts'],
      languageOptions: {
        parser: require('@typescript-eslint/parser'),
        parserOptions: {
          project: join(__dirname, './tsconfig.base.json')
        }
      },
      rules: {
        ...config.rules
      }
    }))
];

My extended eslintrc.js file:

module.exports = {
  extends: [
    ...
    'plugin:@smarttools/rxjs/recommended'
  ],
  rules: {
    '@smarttools/rxjs/no-async-subscribe': 'warn',
    ...
  }
};
```
@DaveMBush
Copy link
Owner

name is a new property of flat config, which it appears you are using.

I have a small sample using flat config that I put together working on another issue that you can view here: https://github.com/DaveMBush/eslint-plugin-rxjs/tree/dmb/circular-dependency-error-when-using-recommended/49/tests/using-recommended

I'm using a js file for the config, but you'll notice the project file specifies commonjs. I've also tried changing the file type from js to csj and still can't replicate your issue

Do you have a minimal project that illustrates your problem that might help me track this down.

@woodrunner
Copy link
Author

woodrunner commented Jan 13, 2025

I reproduced the error on this stackblitz:
https://stackblitz.com/~/github.com/woodrunner/angular-eslint-bug?branch=recommended-name-bug

I think it has something todo with the separate eslintrc.js file.

If you run the patch that removes the name property, the error is gone:

npm run apply-remove-name-patch
npm run eslint

You still get the circular dependency error, but I created another patch for this in another branch. I referenced this in the other github issue on your plugin (#49)

@nspire909
Copy link

I am getting the same error with the legacy config:

Error: ESLint configuration in .eslintrc.json#overrides[4] » plugin:@smarttools/rxjs/recommended-legacy is invalid:
	- Unexpected top-level property "name".

@DaveMBush
Copy link
Owner

@woodrunner if you upgrade to 1.0.16 AND use recommended-legacy in your tools/eslint.config.js file (because it is still using legacy format) this problem should be resolved.

@nspire909 1.0.16 fixes your issue as well.

@github-project-automation github-project-automation bot moved this from New to Done in eslint-plugin-rxjs Jan 15, 2025
@woodrunner
Copy link
Author

woodrunner commented Jan 15, 2025

@DaveMBush thank you for your quick fix. Why are you saying I'm using legacy format? It is flat config, no? I indeed use a external eslintrc.js file, but I merge it in my main eslint.config.cjs file with FlatCompat.

EDIT: I see the issue. My eslintrc.js is still in the legacy format, and I import the recommended configurations there. Should I keep this external file and convert it to a flat config, or should I copy the entire configuration into the main eslint.config.cjs file? That makes it a much larger file. What is your opinion?

@DaveMBush
Copy link
Owner

If you convert the one in tools to flat config, make sure you import it without the compat.config in the main config and more like a plugin like I've done with baseConfig here https://github.com/DaveMBush/SmartNgRX/blob/v-next/libs/smart-ngrx/eslint.config.js#L11

If you can, two files would be preferred. Sometimes you can't get the same inheritance/override behaviour we've had with legacy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

No branches or pull requests

3 participants