[eslint] enable type-specific lint rules #114184
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mshustov
reviewed
Oct 7, 2021
spalger
force-pushed
the
implement/type-specific-linting
branch
3 times, most recently
from
2b53991 to
53d7fd7
Compare
spalger
force-pushed
the
implement/type-specific-linting
branch
from
8ebbfb1 to
ed5dcd9
Compare
|
@elasticmachine merge upstream |
weltenwort
approved these changes
Nov 2, 2021
mshustov
approved these changes
Nov 2, 2021
@spenceralger should we create a meta-issue to track the effort? or will plugins be able to add it on a voluntary basis? |
I'm not planning on forcing plugins to enable it as of right now, but I think it's a reasonable initiative if folks think it's a good idea. The plan is for plugins to opt-into for now. |
dasansol92
approved these changes
Nov 3, 2021
darnautov
approved these changes
Nov 3, 2021
ThomThomson
approved these changes
Nov 3, 2021
flash1293
approved these changes
Nov 3, 2021
mattkime
approved these changes
Nov 3, 2021
|
@elasticmachine merge upstream |
…kibana into implement/type-specific-linting
patrykkopycinski
approved these changes
Nov 3, 2021
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: cc @spalger |
|
Thanks to everyone who reviewed this, the changes still requiring reviews are all made automatically by well a tested eslint rule. I'm going to merge to prevent a 5th conflict resolution today. |
spalger
added
the
auto-backport
💔 Backport failed
To backport manually run: |
spalger
pushed a commit
to spalger/kibana
that referenced
this pull request
Nov 3, 2021
* [eslint] enable type-specific lint rules * autofix violations * duplicate eslint-disable to new export statement Co-authored-by: spalger <[email protected]> Co-authored-by: Kibana Machine <[email protected]> # Conflicts: # src/plugins/share/common/index.ts
This was referenced Nov 3, 2021
spalger
added a commit
that referenced
this pull request
Nov 4, 2021
* [eslint] enable type-specific lint rules (#114184) * [eslint] enable type-specific lint rules * autofix violations * duplicate eslint-disable to new export statement Co-authored-by: spalger <[email protected]> Co-authored-by: Kibana Machine <[email protected]> # Conflicts: # src/plugins/share/common/index.ts * autofix more types Co-authored-by: spalger <[email protected]>
spalger
added a commit
that referenced
this pull request
Nov 4, 2021
* [eslint] enable type-specific lint rules (cherry picked from commit e824fdc) * autofix all violations Co-authored-by: spalger <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #114175
Enables ESLint rules which require types in a separate eslint config. This requires that we run ESLint separately in every typescript project, but it gives us access to very powerful ESLint rules like:
@typescript-eslint/prefer-export-type: prevents using standardexportstatements to export types, which bloats bundles when unnecessary exports are left behind by babel and gets us closed to being able to enableisolatedModules: trueon all TS Projects. This rule is enabled by this PR and autofixed in all code.@typescript-eslint/no-floating-promises: allows us to check that promises are handled when they are created. This rule is not enabled by this plugin but will be the next focus, ideally we will be able to provide contributors with a method for enabling this rule in their plugin and fixing the violations if they decide they want to do so. We can also enable it in all plugins which have few or no issues to prevent new issues from popping up.One drawback of this approach is that the code is being linted by ESLint twice, which it doesn't love. Unexpected
/* eslint-disable*directives make it upset and it requires that we disable the inline directives for this config. Because of this I plan to create a POC which runs all of our ESLint configs this way so we can get back to a single ESLint config for each file. It requires config files in every plugin so that editors can load the correct config for each file, and it might not even work then, but we'll see if we can get it working.The command implemented in this PR is: