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

Improve prettier / eslint configuration #2909

Merged

Conversation

hdurix
Copy link
Member

@hdurix hdurix commented Aug 3, 2022

Fix #2908

@hdurix hdurix linked an issue Aug 3, 2022 that may be closed by this pull request
@@ -2,13 +2,14 @@ module.exports = {
env: {
node: true,
},
extends: ['eslint:recommended', 'plugin:vue/vue3-recommended', '@vue/eslint-config-typescript', 'prettier'],
extends: ['eslint:recommended', 'plugin:vue/vue3-recommended', '@vue/eslint-config-typescript', '@vue/prettier'],
Copy link
Member Author

Choose a reason for hiding this comment

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

prettier does not seem to hint prettier errors in ts files in IDEs (at least IntelliJ)

Copy link
Member Author

Choose a reason for hiding this comment

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

tried in vscode, I think this was not working either

parserOptions: {
parser: '@typescript-eslint/parser',
},
rules: {
quotes: ['error', 'single', { avoidEscape: true }],
'@typescript-eslint/no-unused-vars': ['error'],
'vue/multi-word-component-names': 'off',
'prettier/prettier': ['error'],
Copy link
Member Author

Choose a reason for hiding this comment

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

to show prettier's errors as errors in IDEs

@@ -1,3 +1,4 @@
module.exports = {
'{,src/**/,webpack/}*.{md,json,yml,html,js,ts,tsx,css,scss,vue,java,xml}': ['prettier --write'],
'{,src/**/}*.{js,ts,tsx,vue}': ['eslint --fix'],
Copy link
Member Author

Choose a reason for hiding this comment

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

  • eslint will do both lint + format (with calling prettier)
  • webpack is not used

@@ -11,15 +11,5 @@ arrowParens: avoid
# jsx and tsx rules:
bracketSameLine: false

# search additional plugin, following prettier 2.6.0
pluginSearchDirs:
Copy link
Member Author

Choose a reason for hiding this comment

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

this is the default configuration, not sure this is still necessary (I checked and noticed nothing wrong, was able to format XML files)

- .

# java rules:
overrides:
Copy link
Member Author

Choose a reason for hiding this comment

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

as we only overrides with same values, not sure this is useful

package.json Outdated
@@ -45,11 +45,11 @@
"@types/sinon": "10.0.13",
"@typescript-eslint/parser": "5.32.0",
"@vitejs/plugin-vue": "3.0.1",
"@vue/eslint-config-prettier": "^7.0.0",
Copy link
Member Author

Choose a reason for hiding this comment

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

to use @vue/prettier extension in .eslintrc.js instead of prettier

Copy link
Member

Choose a reason for hiding this comment

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

@hdurix We use normally fixed version by default

Copy link
Member Author

Choose a reason for hiding this comment

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

good catch, changed!

@@ -36,9 +36,9 @@ public JHipsterModule buildVueModule(JHipsterModuleProperties properties) {
.addDevDependency(packageName("@typescript-eslint/parser"), VUE)
.addDevDependency(packageName("@vitejs/plugin-vue"), VUE)
.addDevDependency(packageName("@vue/eslint-config-typescript"), VUE)
.addDevDependency(packageName("@vue/eslint-config-prettier"), VUE)
Copy link
Member Author

Choose a reason for hiding this comment

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

same changes for generated apps

@codecov
Copy link

codecov bot commented Aug 3, 2022

Codecov Report

Merging #2909 (988f906) into main (af74ed0) will not change coverage.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##                main     #2909    +/-   ##
============================================
  Coverage     100.00%   100.00%            
+ Complexity      2078      2004    -74     
============================================
  Files            482       475     -7     
  Lines           8215      8046   -169     
  Branches         164       156     -8     
============================================
- Hits            8215      8046   -169     
Impacted Files Coverage Δ
...ator/client/vue/core/domain/VueModulesFactory.java 100.00% <100.00%> (ø)
...ch/jhipster/lite/module/domain/JHipsterModule.java 100.00% <0.00%> (ø)
...ipster/lite/module/domain/JHipsterModuleFiles.java 100.00% <0.00%> (ø)
...ster/lite/module/domain/JHipsterModuleChanges.java 100.00% <0.00%> (ø)
...ter/lite/module/domain/JHipsterModulesApplyer.java 100.00% <0.00%> (ø)
...cture/secondary/FileSystemJHipsterModuleFiles.java 100.00% <0.00%> (ø)
...secondary/FileSystemJHipsterModulesRepository.java 100.00% <0.00%> (ø)
...r/security/jwt/domain/AngularJwtDomainService.java 100.00% <0.00%> (ø)
...n/flyway/application/FlywayApplicationService.java 100.00% <0.00%> (ø)
...ty/common/domain/AuthenticationModulesFactory.java 100.00% <0.00%> (ø)
... and 46 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@@ -17,10 +17,10 @@
"@types/sinon": "10.0.13",
"@typescript-eslint/parser": "5.32.0",
"@vitejs/plugin-vue": "3.0.1",
"@vue/eslint-config-prettier": "^7.0.0",
Copy link
Member

Choose a reason for hiding this comment

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

@hdurix We use normally fixed version by default

Copy link
Member Author

Choose a reason for hiding this comment

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

changed in latest version

@pascalgrimaud
Copy link
Member

Thanks @hdurix : just need to check if there is no conflict between the eslint config and prettier config

Plz can you use the fixed version and we could merge it

@pascalgrimaud pascalgrimaud merged commit 6b81306 into jhipster:main Aug 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve prettier / eslint configuration
3 participants