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

Jump to bottom

ESLint 9 #10598

Merged
merged 1 commit into from
Sep 2, 2024
Merged

ESLint 9 #10598

merged 1 commit into from
Sep 2, 2024

Conversation

qmonmert
Copy link
Contributor

Fix #9496

murdos
murdos reviewed Aug 18, 2024
View reviewed changes
src/main/resources/generator/client/react/eslint.config.js Outdated
'node_modules/',
'src/main/docker/',
'src/test/webapp/cypress/',
'src/test/webapp/protractor.conf.js',
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't use protractor, jest, webpack, so this can be removed.

src/main/resources/generator/client/react/eslint.config.js Outdated
'webpack/',
'target/',
'build/',
'node/',
Copy link
Contributor

Choose a reason for hiding this comment

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

The node folder is inside the target folder now, so this is not required too.

src/main/resources/generator/client/react/eslint.config.js Outdated
'jest.conf.js',
'src/test/webapp/jest.conf.js',
'webpack/',
'target/',
Copy link
Contributor

Choose a reason for hiding this comment

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

This (and build) could be templatized, by using {{projectBuildDirectory}}

@qmonmert qmonmert mentioned this pull request Aug 19, 2024
Merged
murdos
murdos reviewed Aug 22, 2024
View reviewed changes
src/main/resources/generator/client/react/eslint.config.js Outdated
},
{
ignores: [
'node_modules/',
Copy link
Contributor

Choose a reason for hiding this comment

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

node_modules is already ignored by default, see https://eslint.org/docs/latest/use/configure/ignore#ignoring-files

murdos
murdos reviewed Aug 22, 2024
View reviewed changes
src/main/java/tech/jhipster/lite/generator/client/vue/core/domain/VueModulesFactory.java Outdated
@@ -55,7 +55,8 @@ public JHipsterModule buildVueModule(JHipsterModuleProperties properties) {
.addDependency(packageName("vue-router"), VUE)
.addDevDependency(packageName("@typescript-eslint/parser"), COMMON)
.addDevDependency(packageName("@vitejs/plugin-vue"), VUE)
.addDevDependency(packageName("@vue/eslint-config-typescript"), VUE)
.addDevDependency(packageName("typescript-eslint"), COMMON)
.addDevDependency(packageName("globals"), COMMON)
.addDevDependency(packageName("@vue/eslint-config-prettier"), VUE)
Copy link
Contributor

Choose a reason for hiding this comment

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

This one could probably be removed, I think it's not used anymore:

Suggested change
.addDevDependency(packageName("@vue/eslint-config-prettier"), VUE)

murdos
murdos reviewed Aug 22, 2024
View reviewed changes
src/main/java/tech/jhipster/lite/generator/client/vue/core/domain/VueModulesFactory.java
@@ -55,7 +55,8 @@ public JHipsterModule buildVueModule(JHipsterModuleProperties properties) {
.addDependency(packageName("vue-router"), VUE)
.addDevDependency(packageName("@typescript-eslint/parser"), COMMON)
.addDevDependency(packageName("@vitejs/plugin-vue"), VUE)
.addDevDependency(packageName("@vue/eslint-config-typescript"), VUE)
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't forget to remove this dependency (and @vue/eslint-config-prettier) from src/main/resources/generator/dependencies/vue/package.json

Copy link
Contributor

Choose a reason for hiding this comment

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

This comment sill applies, some cleanup of unused dependencies are needed in src/main/resources/generator/dependencies/vue/package.json

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

src/main/java/tech/jhipster/lite/generator/typescript/core/domain/TypescriptModuleFactory.java Show resolved Hide resolved
@qmonmert qmonmert force-pushed the eslint990 branch 5 times, most recently from 8f4be46 to 338e987 Compare September 1, 2024 19:23
@qmonmert qmonmert changed the title WIP: ESLint 9 ESLint 9 Sep 1, 2024
@qmonmert qmonmert force-pushed the eslint990 branch 2 times, most recently from dc4784c to b69b828 Compare September 1, 2024 20:42
murdos
murdos reviewed Sep 1, 2024
View reviewed changes
src/main/java/tech/jhipster/lite/generator/typescript/core/domain/TypescriptModuleFactory.java Outdated
.addDevDependency(packageName("eslint-import-resolver-typescript"), COMMON)
.addDevDependency(packageName("eslint-plugin-import"), COMMON)
.addDevDependency(packageName("eslint-plugin-import-x"), COMMON)
.addDevDependency(packageName("eslint-plugin-prettier"), COMMON)
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't use anymore `eslint-plugin-prettier', see #10638

src/main/resources/generator/client/react/core/eslint.config.js.mustache Outdated
},
},
{
ignores: ['src/main/docker/', 'jest.conf.js', 'src/test/webapp/jest.conf.js', 'postcss.config.js', '{{projectBuildDirectory}}/'],
Copy link
Contributor

Choose a reason for hiding this comment

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

Jest is not used by react:

Suggested change
ignores: ['src/main/docker/', 'jest.conf.js', 'src/test/webapp/jest.conf.js', 'postcss.config.js', '{{projectBuildDirectory}}/'],
ignores: ['src/main/docker/', 'postcss.config.js', '{{projectBuildDirectory}}/'],

src/main/resources/generator/client/vue/eslint.config.js.mustache Outdated
},
},
{
ignores: ['src/main/docker/', 'jest.conf.js', 'src/test/webapp/jest.conf.js', 'postcss.config.js', '{{projectBuildDirectory}}/'],
Copy link
Contributor

Choose a reason for hiding this comment

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

Jest and postcss are not used by Vue:

Suggested change
ignores: ['src/main/docker/', 'jest.conf.js', 'src/test/webapp/jest.conf.js', 'postcss.config.js', '{{projectBuildDirectory}}/'],
ignores: ['src/main/docker/', '{{projectBuildDirectory}}/'],

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we actually have javascript or typescript content in src/main/docker?

src/main/resources/generator/dependencies/common/package.json Outdated
"eslint-plugin-cypress": "3.5.0",
"eslint-plugin-import": "2.29.1",
"eslint-plugin-import-x": "4.1.0",
"eslint-plugin-prettier": "5.2.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

eslint-plugin-prettier is not used anymore

src/main/resources/generator/typescript/eslint.config.js.mustache Outdated
{
ignores: [
'src/main/docker/',
'jest.conf.js',
Copy link
Contributor

Choose a reason for hiding this comment

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

Neither Jest, nor postcss is used.
Also why ignore vitest.config.ts and eslint.config.js?

@qmonmert qmonmert force-pushed the eslint990 branch 2 times, most recently from c83bfeb to b2fb702 Compare September 2, 2024 12:47
@qmonmert qmonmert marked this pull request as ready for review September 2, 2024 13:02
@qmonmert
Copy link
Contributor Author

qmonmert commented Sep 2, 2024

thanks @murdos for your reviews

@qmonmert qmonmert force-pushed the eslint990 branch 2 times, most recently from a94bbd9 to 18d3425 Compare September 2, 2024 19:47
murdos
murdos previously approved these changes Sep 2, 2024
View reviewed changes
Copy link
Contributor

@murdos murdos left a comment

Choose a reason for hiding this comment

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

LGTM. Could you fix conflict with the main branch?

@qmonmert qmonmert dismissed murdos’s stale review September 2, 2024 20:02

The merge-base changed after approval.

@murdos murdos merged commit 6f77e5a into jhipster:main Sep 2, 2024
34 checks passed
@qmonmert
Copy link
Contributor Author

qmonmert commented Sep 2, 2024

@murdos thanks for the reviews

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@murdos murdos murdos left review comments

@pascalgrimaud pascalgrimaud Awaiting requested review from pascalgrimaud

Assignees
No one assigned
Labels
client: angular client: react client: vue theme: client theme: typescript
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Migration ESLint 9.0.0 (Angular React Vue apps)
2 participants
@qmonmert @murdos