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

fix(eslint): flat config support #51

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

AMorgaut
Copy link

Fix issue #50 (ESLint flat config support)

see: https://eslint.org/docs/latest/extend/plugin-migration-flat-config

recommended === false ? "off" : recommended;
return map;
}, {});
for (let { ruleName, ruleModule } of rulesList) {
Copy link
Author

@AMorgaut AMorgaut Aug 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previous implementation did 2 loops

The updated code does it only once

};

plugin.configs = {
recommended: {
Copy link
Author

@AMorgaut AMorgaut Aug 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

legacy configuration is maintained as-is

no expected regressions

plugins: ["@ecocode"],
rules: recommendedRules,
},
["flat/recommended"]: {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the new flat configuration follows the plugin naming convention

Comment on lines 106 to 118
| :------------------------------------------------------------------------------------- | :--------------------------------------------------------- | :---------------------------- |
| [avoid-brightness-override](docs/rules/avoid-brightness-override.md) | Should avoid to override brightness | ✅ ![badge-flat/recommended][] |
| [avoid-css-animations](docs/rules/avoid-css-animations.md) | Avoid usage of CSS animations | ✅ ![badge-flat/recommended][] |
| [avoid-high-accuracy-geolocation](docs/rules/avoid-high-accuracy-geolocation.md) | Avoid using high accuracy geolocation in web applications. | ✅ ![badge-flat/recommended][] |
| [limit-db-query-results](docs/rules/limit-db-query-results.md) | Should limit the number of returns for a SQL query | ✅ ![badge-flat/recommended][] |
| [no-empty-image-src-attribute](docs/rules/no-empty-image-src-attribute.md) | Disallow usage of image with empty source attribute | ✅ ![badge-flat/recommended][] |
| [no-import-all-from-library](docs/rules/no-import-all-from-library.md) | Should not import all from library | ✅ ![badge-flat/recommended][] |
| [no-multiple-access-dom-element](docs/rules/no-multiple-access-dom-element.md) | Disallow multiple access of same DOM element. | ✅ ![badge-flat/recommended][] |
| [no-multiple-style-changes](docs/rules/no-multiple-style-changes.md) | Disallow multiple style changes at once. | ✅ ![badge-flat/recommended][] |
| [no-torch](docs/rules/no-torch.md) | Should not programmatically enable torch mode | ✅ ![badge-flat/recommended][] |
| [prefer-collections-with-pagination](docs/rules/prefer-collections-with-pagination.md) | Prefer API collections with pagination. | ✅ ![badge-flat/recommended][] |
| [prefer-shorthand-css-notations](docs/rules/prefer-shorthand-css-notations.md) | Encourage usage of shorthand CSS notations | ✅ ![badge-flat/recommended][] |
| [provide-print-css](docs/rules/provide-print-css.md) | Enforce providing a print stylesheet | ✅ ![badge-flat/recommended][] |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the addition of ![badge-flat/recommended][]?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't edited it myself
These changes were done by calling the yarn run update:eslint-docs command
as mentioned in the CONTRIBUTING documentation
https://github.com/green-code-initiative/ecoCode-javascript/blob/main/CONTRIBUTING.md#generate-rule-documentation

Copy link
Member

@utarwyn utarwyn Oct 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we need to edit the configuration, because the current result is pretty strange 😞

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

manually fixed for now

const plugin = {
meta: {
name: "@ecocode/eslint-plugin",
version: "1.6.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a big fan of version being hard-coded here.
This makes it necessary to modify the version bump workflow to change this value dynamically.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree not fan neither

The ESLint configuration says:

you should add a meta key that contains at least a name key, and ideally, a version key

So version is not mandatory and looking a other plugins, it looks like they don't include it, so we could drop it.

I'll just do some check because it also says:

Without this meta information, your plugin will not be usable with the --cache and --print-config command line options.

Even if the version is not mandatory, I would like to make sure about its impact regarding the cache feature

@utarwyn utarwyn added 🚀 enhancement New feature or request 🏗️ refactoring Refactoring for best practices labels Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚀 enhancement New feature or request 🏗️ refactoring Refactoring for best practices
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make the ESLint plugin compatible with the last ESLint versions with flat configurations
2 participants