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

chore: Use correct eslint deps #5034

Merged
merged 10 commits into from
Sep 30, 2024
Merged

chore: Use correct eslint deps #5034

merged 10 commits into from
Sep 30, 2024

Conversation

marikaner
Copy link
Contributor

@marikaner marikaner commented Sep 26, 2024

We currently use the eslint import plugin incorrectly without the correct resolver and typescript eslint plugin. According to the documentation this is required: https://github.com/import-js/eslint-plugin-import/tree/main?tab=readme-ov-file#typescript

When adjusting the config many of the rules did report errors. I fixed them all except some that would otherwise fail the circular dependency check (@ZhongpinWang please take a look at this, maybe you remember some hidden issues that were not tested, but I hope we have everything covered by tests).
I also removed the "no-relative-parent-imports" rule because this actually breaks to many things and it has likely never really worked. If we would like to keep that rule we would need to make major changes to the code.

I found this because I thought there was an incorrect report for one specific dependency in the import plugin: import-js/eslint-plugin-import#3069

dependabot bot and others added 4 commits September 26, 2024 18:56
Bumps [eslint-plugin-import](https://github.com/import-js/eslint-plugin-import) from 2.29.1 to 2.30.0.
- [Release notes](https://github.com/import-js/eslint-plugin-import/releases)
- [Changelog](https://github.com/import-js/eslint-plugin-import/blob/main/CHANGELOG.md)
- [Commits](import-js/eslint-plugin-import@v2.29.1...v2.30.0)

---
updated-dependencies:
- dependency-name: eslint-plugin-import
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
'import/no-useless-path-segments': [
'error',
{
noUselessIndex: true
Copy link
Member

@tomfrenken tomfrenken Sep 30, 2024

Choose a reason for hiding this comment

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

I'm surprised this isn't the default

@@ -47,7 +47,7 @@
},
"devDependencies": {
"@sap-cloud-sdk/test-services-odata-v4": "^3.21.0",
"nock": "^14.0.0-beta.6",
"nock": "^14.0.0-beta.14",
Copy link
Member

Choose a reason for hiding this comment

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

I assume this change is not directly related to the PR?

Copy link
Member

@tomfrenken tomfrenken left a comment

Choose a reason for hiding this comment

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

LGTM, just have another look at the few exceptions for no-internal-modules to make sure they should still be excluded after all the changes.

@marikaner marikaner enabled auto-merge (squash) September 30, 2024 11:26
@marikaner marikaner merged commit a729a72 into main Sep 30, 2024
10 checks passed
@marikaner marikaner deleted the use-correct-eslint-deps branch September 30, 2024 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants