You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2021/04/21 16:48:14 UTC
[GitHub] [druid] jgoz opened a new pull request #11142: Web console: Switch to ESLint
jgoz opened a new pull request #11142:
URL: https://github.com/apache/druid/pull/11142
### Description
TSLint is deprecated and they recommend switching to ESLint which now has great support for TypeScript and more feature-rich lint plugins for React, React+hooks, etc.
I used the `tslint-to-eslint-config` automated tool to translate the base config from `awesome-code-style` + local extensions and then reorganized it as additions/overrides to the "recommended" configs provided by ESLint, typescript-eslint, etc.
Finally, I enabled a few more rules that didn't require significant manual changes, favoring those that included fixers.
Once these rules are lifted into `awesome-code-style`, there will be a follow-up PR to use the base config it provides instead of specifying everything locally.
#### Note on Prettier
In the previous TSLint config, it used a plugin that treated Prettier as a lint rule. A similar plugin exists for ESLint, but doing this is [actively discouraged by Prettier](https://prettier.io/docs/en/integrating-with-linters.html). Instead, they recommend keeping the two separate, which avoids red squiggly noise in IDEs and is faster (so they say).
But note that this requires a couple of changes:
- Explicitly checking formatting during `npm test` — added the `prettify-check` script to deal with this
- Enabling format-on-save in your IDE — e.g., in WebStorm, you can run Prettier on format and on save:
<img width="734" alt="image" src="https://user-images.githubusercontent.com/132233/115590688-7e1fc880-a28e-11eb-90ce-c7d67ec819e4.png">
If we want the old behavior back, it would be trivial to add `eslint-plugin-prettier` to the setup, but I think we should give this a fair shake.
<hr>
This PR has:
- [x] been self-reviewed.
- [x] been tested in a test Druid cluster.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] vogievetsky commented on a change in pull request #11142: Web console: Switch to ESLint
Posted by GitBox <gi...@apache.org>.
vogievetsky commented on a change in pull request #11142:
URL: https://github.com/apache/druid/pull/11142#discussion_r618602820
##########
File path: web-console/README.md
##########
@@ -41,18 +41,39 @@ To try the console in (say) coordinator mode you could run it as such:
You should use a TypeScript friendly IDE (such as [WebStorm](https://www.jetbrains.com/webstorm/), or [VS Code](https://code.visualstudio.com/)) to develop the web console.
-The console relies on [tslint](https://palantir.github.io/tslint/), [sass-lint](https://github.com/sasstools/sass-lint), and [prettier](https://prettier.io/) to enforce the code style.
-
-If you are going to do any non-trivial development you should set up file watchers in your IDE to automatically fix your code as you type.
-
-If you do not set up auto file watchers then even a trivial change such as a typo fix might draw the ire of the code style enforcement (it might require some lines to be re-wrapped).
-If you find yourself in that position you should run on or more of:
-
-- `npm run tslint-fix`
-- `npm run sasslint-fix`
-- `npm run prettify`
-
-To get your code into an acceptable state.
+The console relies on [eslint](https://eslint.org) (and various plugins), [sass-lint](https://github.com/sasstools/sass-lint), and [prettier](https://prettier.io/) to enforce code style. If you are going to do any non-trivial development you should set up your IDE to automatically lint and fix your code as you make changes.
+
+#### Configuring WebStorm
+
+- **Preferences | Languages & Frameworks | JavaScript | Code Quality Tools | ESLint**
+ - Select "Automatic ESLint Configuration"
+ - Check "Run eslint --fix on save"
+
+- **Preferences | Languages & Frameworks | JavaScript | Prettier**
+ - Set "Run for files" to `{**/*,*}.{js,ts,jsx,tsx,css,scss}`
+ - Check "On code reformat"
+ - Check "On save"
+
+#### Configuring VS Code
+- Install `dbaeumer.vscode-eslint` extension
+- Install `esbenp.prettier-vscode` extension
+- Open User Settings (JSON) and set the following:
+ ```json
+ "editor.defaultFormatter": "esbenp.prettier-vscode",
+ "editor.formatOnSave": true,
+ "editor.codeActionsOnSave": {
+ "source.fixAll.eslint": true
+ }
+ ```
+
+#### Auto-fixing manually
+It is also possible to auto-fix and format code without making IDE changes by running the following scripts:
+
+- `npm run eslint-fix` — run code linter and fix issues
+- `npm run sasslint-fix` — run style linter and fix issues
+- `npm run prettify` — reformat code and styles
Review comment:
Should there be an `npm run autofix` that just does all 3 of those in order?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] vogievetsky commented on a change in pull request #11142: Web console: Switch to ESLint
Posted by GitBox <gi...@apache.org>.
vogievetsky commented on a change in pull request #11142:
URL: https://github.com/apache/druid/pull/11142#discussion_r617743179
##########
File path: web-console/.eslintrc.js
##########
@@ -0,0 +1,167 @@
+module.exports = {
Review comment:
This file (`.js`) should have the Apache header
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] jgoz commented on a change in pull request #11142: Web console: Switch to ESLint
Posted by GitBox <gi...@apache.org>.
jgoz commented on a change in pull request #11142:
URL: https://github.com/apache/druid/pull/11142#discussion_r617778340
##########
File path: web-console/.eslintrc.js
##########
@@ -0,0 +1,167 @@
+module.exports = {
Review comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] vogievetsky commented on a change in pull request #11142: Web console: Switch to ESLint
Posted by GitBox <gi...@apache.org>.
vogievetsky commented on a change in pull request #11142:
URL: https://github.com/apache/druid/pull/11142#discussion_r617740304
##########
File path: web-console/src/views/services-view/services-view.tsx
##########
@@ -438,14 +437,15 @@ ORDER BY "rank" DESC, "service" DESC`;
},
Aggregated: row => {
switch (row.row._pivotVal) {
- case 'historical':
+ case 'historical': {
Review comment:
Been working with ECMAScript languages since 2008 and did not know that you can have blocks in `switch` statements.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] vogievetsky merged pull request #11142: Web console: Switch to ESLint
Posted by GitBox <gi...@apache.org>.
vogievetsky merged pull request #11142:
URL: https://github.com/apache/druid/pull/11142
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] jgoz commented on a change in pull request #11142: Web console: Switch to ESLint
Posted by GitBox <gi...@apache.org>.
jgoz commented on a change in pull request #11142:
URL: https://github.com/apache/druid/pull/11142#discussion_r618613238
##########
File path: web-console/README.md
##########
@@ -41,18 +41,39 @@ To try the console in (say) coordinator mode you could run it as such:
You should use a TypeScript friendly IDE (such as [WebStorm](https://www.jetbrains.com/webstorm/), or [VS Code](https://code.visualstudio.com/)) to develop the web console.
-The console relies on [tslint](https://palantir.github.io/tslint/), [sass-lint](https://github.com/sasstools/sass-lint), and [prettier](https://prettier.io/) to enforce the code style.
-
-If you are going to do any non-trivial development you should set up file watchers in your IDE to automatically fix your code as you type.
-
-If you do not set up auto file watchers then even a trivial change such as a typo fix might draw the ire of the code style enforcement (it might require some lines to be re-wrapped).
-If you find yourself in that position you should run on or more of:
-
-- `npm run tslint-fix`
-- `npm run sasslint-fix`
-- `npm run prettify`
-
-To get your code into an acceptable state.
+The console relies on [eslint](https://eslint.org) (and various plugins), [sass-lint](https://github.com/sasstools/sass-lint), and [prettier](https://prettier.io/) to enforce code style. If you are going to do any non-trivial development you should set up your IDE to automatically lint and fix your code as you make changes.
+
+#### Configuring WebStorm
+
+- **Preferences | Languages & Frameworks | JavaScript | Code Quality Tools | ESLint**
+ - Select "Automatic ESLint Configuration"
+ - Check "Run eslint --fix on save"
+
+- **Preferences | Languages & Frameworks | JavaScript | Prettier**
+ - Set "Run for files" to `{**/*,*}.{js,ts,jsx,tsx,css,scss}`
+ - Check "On code reformat"
+ - Check "On save"
+
+#### Configuring VS Code
+- Install `dbaeumer.vscode-eslint` extension
+- Install `esbenp.prettier-vscode` extension
+- Open User Settings (JSON) and set the following:
+ ```json
+ "editor.defaultFormatter": "esbenp.prettier-vscode",
+ "editor.formatOnSave": true,
+ "editor.codeActionsOnSave": {
+ "source.fixAll.eslint": true
+ }
+ ```
+
+#### Auto-fixing manually
+It is also possible to auto-fix and format code without making IDE changes by running the following scripts:
+
+- `npm run eslint-fix` — run code linter and fix issues
+- `npm run sasslint-fix` — run style linter and fix issues
+- `npm run prettify` — reformat code and styles
Review comment:
Yes!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] vogievetsky commented on pull request #11142: Web console: Switch to ESLint
Posted by GitBox <gi...@apache.org>.
vogievetsky commented on pull request #11142:
URL: https://github.com/apache/druid/pull/11142#issuecomment-824462738
Could you please update the readme to include instructions for new devs on how to set up their IDEs (WebStorm, VSCode) to make the most of all the auto code goodness?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org