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.

Sign up for GitHub

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
Open

fix(eslint): flat config support #51

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
dc4b8e0
fix(eslint): flat config support
AMorgaut Aug 21, 2024
56a9dab
fix: fix wrong plugin reference
AMorgaut Aug 21, 2024
97b58c1
fix: postpone plugin config initialisation to fix reference
AMorgaut Aug 21, 2024
b24c6c1
chore: prettier fix
AMorgaut Aug 21, 2024
d17d6b5
doc: update rules and eslint documentations
AMorgaut Aug 21, 2024
d667571
Update eslint-plugin/README.md
AMorgaut Oct 16, 2024
154179c
Update eslint-plugin/lib/standalone.js
AMorgaut Oct 18, 2024
230c6e5
fix: revert package version change
AMorgaut Oct 18, 2024
0e1fa6e
Merge branch 'main' into eslint-flat-config
AMorgaut Oct 18, 2024
aa8c858
doc: update config titles
AMorgaut Oct 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 53 additions & 18 deletions eslint-plugin/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,52 @@ yarn add -D eslint @green-code-initiative/ecocode-eslint-plugin
npm install -D eslint @green-code-initiative/ecocode-eslint-plugin
```

### Enable whole plugin
### Enable plugin with recommended configuration

Add `@ecocode` recommended configuration to `extends` section of your `.eslintrc`:
#### With modern `eslint.config.js`

```jsonc
Add `@ecocode` **"flat/recommended"** configuration to your `eslint.config.js`:

```js
import ecocode from '@ecocode/eslint-plugin'

export default [
/* other eslint configurations */
ecocode.configs['flat/recommended'],
]
```

#### With the legacy `.eslintrc`

Add `@ecocode` **"recommended"** configuration to `extends` section of your `.eslintrc`:

```json
{
"extends": ["plugin:@ecocode/recommended"]
}
```

### Enable only some rules
### Enable specific rules

#### With modern `eslint.config.js`
Copy link
Member

Choose a reason for hiding this comment

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

ESLint documentation talks about "flat configurations", so I think we can keep this term to remain consistent.

Suggested change
#### With modern `eslint.config.js`
#### With modern flat configuration

Copy link
Author

Choose a reason for hiding this comment

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

I'm unsure on that one

I think you might be right for new projects from scratch, but for existing projects and projects generated by framework generators (Nx, Angular, preact, Vue, ...) that still generate project with previous eslint configurations (which is what happened to ecocode-dashboard), it it really not obvious, and actually, it took me some time to figure out about the new eslint "flat config". What you first see when you want to add a plugin to your project is the name of your eslint configuration file

I based this part of the documentation on what I saw on another eslint plugin and which looked cristal clear to apply.

Copy link
Author

Choose a reason for hiding this comment

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

Not sure from which plugin I originally took it but the playwright one use both "flat config" and the file name to describe how to add it. It looks to be a good compromise

Flat config (eslint.config.js)
...

Legacy config (.eslintrc)
...

https://github.com/playwright-community/eslint-plugin-playwright/blob/main/README.md

Copy link
Member

Choose a reason for hiding this comment

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

Thank you for the feedback! I think this is a great idea, we can go for it 💯


Add the `ecocode` plugin configuration to your `eslint.config.js` and select the rules to activate:
AMorgaut marked this conversation as resolved.
Show resolved Hide resolved

```js
import ecocode from '@ecocode/eslint-plugin'

export default [
/* other eslint configurations */
{
plugins: { "@ecocode": ecocode },
rules: {
"@ecocode/no-multiple-access-dom-element": "error"
}
}
]
```

#### With the legacy `.eslintrc`
Copy link
Member

Choose a reason for hiding this comment

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

Same wording as ESLint documentation here

Suggested change
#### With the legacy `.eslintrc`
#### With the deprecated eslintrc format


Add `@ecocode` to the `plugins` section of your `.eslintrc`, followed by rules configuration:

Expand All @@ -67,20 +102,20 @@ Add `@ecocode` to the `plugins` section of your `.eslintrc`, followed by rules c
⚠️ Configurations set to warn in.\
✅ Set in the `recommended` configuration.

| Name | Description | ⚠️ |
| :------------------------------------------------------------------------------------- | :--------------------------------------------------------- | :- |
| [avoid-brightness-override](docs/rules/avoid-brightness-override.md) | Should avoid to override brightness | ✅ |
| [avoid-css-animations](docs/rules/avoid-css-animations.md) | Avoid usage of CSS animations | ✅ |
| [avoid-high-accuracy-geolocation](docs/rules/avoid-high-accuracy-geolocation.md) | Avoid using high accuracy geolocation in web applications. | ✅ |
| [limit-db-query-results](docs/rules/limit-db-query-results.md) | Should limit the number of returns for a SQL query | ✅ |
| [no-empty-image-src-attribute](docs/rules/no-empty-image-src-attribute.md) | Disallow usage of image with empty source attribute | ✅ |
| [no-import-all-from-library](docs/rules/no-import-all-from-library.md) | Should not import all from library | ✅ |
| [no-multiple-access-dom-element](docs/rules/no-multiple-access-dom-element.md) | Disallow multiple access of same DOM element. | ✅ |
| [no-multiple-style-changes](docs/rules/no-multiple-style-changes.md) | Disallow multiple style changes at once. | ✅ |
| [no-torch](docs/rules/no-torch.md) | Should not programmatically enable torch mode | ✅ |
| [prefer-collections-with-pagination](docs/rules/prefer-collections-with-pagination.md) | Prefer API collections with pagination. | ✅ |
| [prefer-shorthand-css-notations](docs/rules/prefer-shorthand-css-notations.md) | Encourage usage of shorthand CSS notations | ✅ |
| [provide-print-css](docs/rules/provide-print-css.md) | Enforce providing a print stylesheet | ✅ |
| Name | Description | ⚠️ |
| :------------------------------------------------------------------------------------- | :--------------------------------------------------------- | :---------------------------- |
| [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 •
edited
Loading

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


<!-- end auto-generated rules list -->

Expand Down
2 changes: 1 addition & 1 deletion eslint-plugin/docs/rules/avoid-brightness-override.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Should avoid to override brightness (`@ecocode/avoid-brightness-override`)

⚠️ This rule _warns_ in the ✅ `recommended` config.
⚠️ This rule _warns_ in the following configs: `flat/recommended`, ✅ `recommended`.

<!-- end auto-generated rule header -->

Expand Down
2 changes: 1 addition & 1 deletion eslint-plugin/docs/rules/avoid-css-animations.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Avoid usage of CSS animations (`@ecocode/avoid-css-animations`)

⚠️ This rule _warns_ in the ✅ `recommended` config.
⚠️ This rule _warns_ in the following configs: `flat/recommended`, ✅ `recommended`.

<!-- end auto-generated rule header -->

Expand Down
2 changes: 1 addition & 1 deletion eslint-plugin/docs/rules/avoid-high-accuracy-geolocation.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Avoid using high accuracy geolocation in web applications (`@ecocode/avoid-high-accuracy-geolocation`)

⚠️ This rule _warns_ in the ✅ `recommended` config.
⚠️ This rule _warns_ in the following configs: `flat/recommended`, ✅ `recommended`.

<!-- end auto-generated rule header -->

Expand Down
2 changes: 1 addition & 1 deletion eslint-plugin/docs/rules/limit-db-query-results.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Should limit the number of returns for a SQL query (`@ecocode/limit-db-query-results`)

⚠️ This rule _warns_ in the ✅ `recommended` config.
⚠️ This rule _warns_ in the following configs: `flat/recommended`, ✅ `recommended`.

<!-- end auto-generated rule header -->

Expand Down
2 changes: 1 addition & 1 deletion eslint-plugin/docs/rules/no-empty-image-src-attribute.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Disallow usage of image with empty source attribute (`@ecocode/no-empty-image-src-attribute`)

⚠️ This rule _warns_ in the ✅ `recommended` config.
⚠️ This rule _warns_ in the following configs: `flat/recommended`, ✅ `recommended`.

<!-- end auto-generated rule header -->

Expand Down
2 changes: 1 addition & 1 deletion eslint-plugin/docs/rules/no-import-all-from-library.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Should not import all from library (`@ecocode/no-import-all-from-library`)

⚠️ This rule _warns_ in the ✅ `recommended` config.
⚠️ This rule _warns_ in the following configs: `flat/recommended`, ✅ `recommended`.

<!-- end auto-generated rule header -->

Expand Down
2 changes: 1 addition & 1 deletion eslint-plugin/docs/rules/no-multiple-access-dom-element.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Disallow multiple access of same DOM element (`@ecocode/no-multiple-access-dom-element`)

⚠️ This rule _warns_ in the ✅ `recommended` config.
⚠️ This rule _warns_ in the following configs: `flat/recommended`, ✅ `recommended`.

<!-- end auto-generated rule header -->

Expand Down
2 changes: 1 addition & 1 deletion eslint-plugin/docs/rules/no-multiple-style-changes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Disallow multiple style changes at once (`@ecocode/no-multiple-style-changes`)

⚠️ This rule _warns_ in the ✅ `recommended` config.
⚠️ This rule _warns_ in the following configs: `flat/recommended`, ✅ `recommended`.

<!-- end auto-generated rule header -->

Expand Down
2 changes: 1 addition & 1 deletion eslint-plugin/docs/rules/no-torch.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Should not programmatically enable torch mode (`@ecocode/no-torch`)

⚠️ This rule _warns_ in the ✅ `recommended` config.
⚠️ This rule _warns_ in the following configs: `flat/recommended`, ✅ `recommended`.

<!-- end auto-generated rule header -->
## Why is this an issue?
Expand Down
2 changes: 1 addition & 1 deletion eslint-plugin/docs/rules/prefer-collections-with-pagination.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Prefer API collections with pagination (`@ecocode/prefer-collections-with-pagination`)

⚠️ This rule _warns_ in the ✅ `recommended` config.
⚠️ This rule _warns_ in the following configs: `flat/recommended`, ✅ `recommended`.

<!-- end auto-generated rule header -->

Expand Down
2 changes: 1 addition & 1 deletion eslint-plugin/docs/rules/prefer-shorthand-css-notations.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Encourage usage of shorthand CSS notations (`@ecocode/prefer-shorthand-css-notations`)

⚠️ This rule _warns_ in the ✅ `recommended` config.
⚠️ This rule _warns_ in the following configs: `flat/recommended`, ✅ `recommended`.

<!-- end auto-generated rule header -->

Expand Down
2 changes: 1 addition & 1 deletion eslint-plugin/docs/rules/provide-print-css.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Enforce providing a print stylesheet (`@ecocode/provide-print-css`)

⚠️ This rule _warns_ in the ✅ `recommended` config.
⚠️ This rule _warns_ in the following configs: `flat/recommended`, ✅ `recommended`.

<!-- end auto-generated rule header -->

Expand Down
42 changes: 28 additions & 14 deletions eslint-plugin/lib/standalone.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,21 +22,35 @@
*/
"use strict";

const rules = require("./rule-list");
const rulesList = require("./rule-list");

const ruleModules = rules.reduce((map, rule) => {
map[rule.ruleName] = rule.ruleModule;
return map;
}, {});
const allRules = {};
const recommendedRules = {}; // recommended rules
AMorgaut marked this conversation as resolved.
Show resolved Hide resolved

const ruleConfigs = rules.reduce((map, rule) => {
const recommended = rule.ruleModule.meta.docs.recommended;
map[`@ecocode/${rule.ruleName}`] =
recommended === false ? "off" : recommended;
return map;
}, {});
for (let { ruleName, ruleModule } of rulesList) {
Copy link
Author

@AMorgaut AMorgaut Aug 21, 2024 •
edited
Loading

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

allRules[ruleName] = ruleModule;
const { recommended } = ruleModule.meta.docs;
const ruleConfiguration = recommended === false ? "off" : recommended;
recommendedRules[`@ecocode/${ruleName}`] = ruleConfiguration;
}

module.exports = {
rules: ruleModules,
configs: { recommended: { plugins: ["@ecocode"], rules: ruleConfigs } },
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

},
rules: allRules,
};

plugin.configs = {
recommended: {
Copy link
Author

@AMorgaut AMorgaut Aug 21, 2024 •
edited
Loading

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

plugins: { "@ecocode": plugin },
rules: recommendedRules,
},
};

module.exports = plugin;
2 changes: 1 addition & 1 deletion eslint-plugin/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@ecocode/eslint-plugin",
"version": "1.5.0",
"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.

You don't need to edit this yourself 😉

Copy link
Author

Choose a reason for hiding this comment

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

sure I can revert it
I guess it is done by the package release workflow

"description": "JavaScript linter of ecoCode project",
"keywords": [
"eslint",
Expand Down
Loading