fix(eslint): setup eslint to run on every package - N8N-4553

[netroy]

Also, unify eslint config and dependencies into a private package in the workspace.

I'll separate PRs to remove all the exceptions I had to add to make the linter pass.

Ticket: N8N-4553

[ivov]

1. I would expect the npm package name to be eslint-config-n8n like others in npm, e.g. eslint-config-airbnb. Why the @n8n-io/ namespace? Do we want to start publishing under this in future?

2. I would simplify the workspace/dir name to eslint-config because it is clear from context that it is for n8n and to remain consistent with other workspaces. For example, we use nodes-base for workspace name and n8n-nodes-base for npm package name.

3. Getting this on npm run lint at root:

n8n-design-system:lint: Tried to lint /Users/ivov/Development/n8n/packages/design-system/src/locale/lang/en.js but found no valid, enabled rules for this file type and file path in the resolved configuration.

4. Getting also this on npm run lint at root:

n8n-nodes-base:lint:
n8n-nodes-base:lint: Oops! Something went wrong! :(
n8n-nodes-base:lint:
n8n-nodes-base:lint: ESLint: 8.23.0
n8n-nodes-base:lint:
n8n-nodes-base:lint: Error: Failed to load plugin 'eslint-plugin-n8n-nodes-base' declared in '.eslintrc.js#overrides[1]': Cannot find module '/Users/ivov/Development/n8n/packages/nodes-base/node_modules/eslint'
n8n-nodes-base:lint: Require stack:
n8n-nodes-base:lint: - /Users/ivov/Development/n8n/node_modules/eslint-docgen/src/rule-tester.js
n8n-nodes-base:lint: - /Users/ivov/Development/n8n/node_modules/eslint-plugin-n8n-nodes-base/dist/lib/ast/utils/rule.js
n8n-nodes-base:lint: - /Users/ivov/Development/n8n/node_modules/eslint-plugin-n8n-nodes-base/dist/lib/ast/utils/index.js
n8n-nodes-base:lint: - /Users/ivov/Development/n8n/node_modules/eslint-plugin-n8n-nodes-base/dist/lib/rules/community-package-json-author-email-still-default.js
n8n-nodes-base:lint: - /Users/ivov/Development/n8n/node_modules/eslint-plugin-n8n-nodes-base/dist/index.js
n8n-nodes-base:lint: - /Users/ivov/Development/n8n/node_modules/@eslint/eslintrc/dist/eslintrc.cjs

5. Getting this when opening TS files:

6. Ticket number in PR description should be 4553.

7. Would it be reasonable to pin the ESLint version so we can very deliberate when upgrading it both here and in the linter? Changes in the ESLint AST have caused linter checks to fail in the past: https://linear.app/n8n/issue/N8N-3968

8. Do you know of a way to detect lint exceptions i.e. eslint-disable-next-line that no longer have effect?

[netroy]

1. I would expect the npm package name to be eslint-config-n8n like others in npm, e.g. eslint-config-airbnb. Why the @n8n-io/ namespace? Do we want to start publishing under this in future?

We already have the @n8n_io org on npm, and we have published there in the past. But this package is private, and only a dev-dependency. It'll not be published.

2. I would simplify the workspace/dir name to `eslint-config` because it is clear from context that it is for n8n and to remain consistent with other workspaces. For example, we use `nodes-base` for workspace name and `n8n-nodes-base` for npm package name.

The current setup with many of our package folder names not being the same as the package name, is an anti-pattern. I'd like us to become more consistent with how other monorepos do this, to reduce confusion for the contributors.
But, that's a bigger discussion, that I'll like to start a thread about elsewhere later.

3. Getting this on `npm run lint` at root:

n8n-design-system:lint: Tried to lint /Users/ivov/Development/n8n/packages/design-system/src/locale/lang/en.js but found no valid, enabled rules for this file type and file path in the resolved configuration.

This is coming from tslint. Unrelated to the changes in this PR.

4. Getting also this on `npm run lint` at root:

n8n-nodes-base:lint:
n8n-nodes-base:lint: Oops! Something went wrong! :(
n8n-nodes-base:lint:
n8n-nodes-base:lint: ESLint: 8.23.0
n8n-nodes-base:lint:
n8n-nodes-base:lint: Error: Failed to load plugin 'eslint-plugin-n8n-nodes-base' declared in '.eslintrc.js#overrides[1]': Cannot find module '/Users/ivov/Development/n8n/packages/nodes-base/node_modules/eslint'
n8n-nodes-base:lint: Require stack:
n8n-nodes-base:lint: - /Users/ivov/Development/n8n/node_modules/eslint-docgen/src/rule-tester.js
n8n-nodes-base:lint: - /Users/ivov/Development/n8n/node_modules/eslint-plugin-n8n-nodes-base/dist/lib/ast/utils/rule.js
n8n-nodes-base:lint: - /Users/ivov/Development/n8n/node_modules/eslint-plugin-n8n-nodes-base/dist/lib/ast/utils/index.js
n8n-nodes-base:lint: - /Users/ivov/Development/n8n/node_modules/eslint-plugin-n8n-nodes-base/dist/lib/rules/community-package-json-author-email-still-default.js
n8n-nodes-base:lint: - /Users/ivov/Development/n8n/node_modules/eslint-plugin-n8n-nodes-base/dist/index.js
n8n-nodes-base:lint: - /Users/ivov/Development/n8n/node_modules/@eslint/eslintrc/dist/eslintrc.cjs

Yeah. this is why the CI is failing. I'm trying to fix it. but in vain so far. It's caused by this weird hack in eslint-docgen.

5. Getting this when opening TS files:

This is fixed by #4049

6. Ticket number in PR description should be 4553.

👍

7. Would it be reasonable to pin the ESLint version so we can very deliberate when upgrading it both here and in the linter? Changes in the ESLint AST have caused linter checks to fail in the past: https://linear.app/n8n/issue/N8N-3968

👍

8. Do you know of a way to detect lint exceptions i.e. `eslint-disable-next-line` that no longer have effect?

not aware of a way. but I can investigate. My plan was to get rid of the most of the eslint-disable-* exceptions.

[ivov]

It's caused by this weird hack in eslint-docgen.

Does it help if I remove eslint-docgen from the linter's published files?

The linter uses eslint-docgen to wrap ESLint's native RuleTester to generate lint rule docs, so eslint-docgen does not belong in its regular deps. But the linter's import/no-extraneous-dependencies flags this: 'eslint-docgen' should be listed in the project's dependencies, not devDependencies

If removing it helps, I can do it now and later look into why this is being flagged, maybe related to the issue you linked to.

[netroy]

It's caused by this weird hack in eslint-docgen.

Does it help if I remove eslint-docgen from the linter's published files?

The linter uses eslint-docgen to wrap ESLint's native RuleTester to generate lint rule docs, so eslint-docgen does not belong in its regular deps. But the linter's import/no-extraneous-dependencies flags this: 'eslint-docgen' should be listed in the project's dependencies, not devDependencies

If removing it helps, I can do it now and later look into why this is being flagged, maybe related to the issue you linked to.

I think removing eslint-docgen will most likely help.

[ivov]

Builds fine, runs fine, lints fine. Only minor comments.

1/2. This package name and workspace/dir name moves away from the antipattern but is inconsistent with the convention, so if you can please clear this with Ben or Omar, etc. Unless done already.

9. @vue/eslint-config-standard is here and in master but not directly specified in .eslintrc.js. Are we using this plugin somehow? I need to double check this.

10. Maybe we should account for .mjs files.

Great work!

[netroy]

9. `@vue/eslint-config-standard` is here and in `master` but not directly specified in `.eslintrc.js`. Are we using this plugin somehow? I need to double check this.

Good catch. removed.

[janober]

Got released with n8n@0.194.0