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

Eslint config is broken and is not working + enhancing it by add some rules #9190

Closed
1 task done
MV88 opened this issue May 24, 2023 · 3 comments
Closed
1 task done
Assignees
Labels

Comments

@MV88
Copy link
Contributor

MV88 commented May 24, 2023

Description

I checked current config is broken and has never worked since March 2021 !
check here the broken part https://github.com/geosolutions-it/MapStore2/blob/master/utility/eslint/index.js#L235

now we got
✖4095 problems (4095 errors, 0 warnings)
with --fix becomes
✖ 3869 problems (3869 errors, 0 warnings)

338 PropType is defined but prop is never used "react/no-unused-prop-types": 2
185 render should be placed after
3308 is missing in props validation "react/prop-types": [2, { "ignore": ["children"] }],

enhancements

in order to improve the current codebase we can add the following rules

  • "react/no-unused-prop-types": 2,
  • "object-curly-spacing": [2, "never"], that has autofix
  • "comma-dangle": [ 2, "always-multiline" ], that has autofix

How to reproduce

  • check lint running in actions or locally, both in a project and in the main mapstore repo

Expected Result

to work fine with no errors

Current Result

actually it is not executed because of some errors, you can notice this with vscode if you open

  • Not browser related
Browser info (use this site: https://www.whatsmybrowser.org/ for non expert users)
Browser Affected Version
Internet Explorer
Edge
Chrome
Firefox
Safari

Other useful information

@MV88 MV88 added the bug label May 24, 2023
@MV88 MV88 added this to the 2023.02.00 milestone May 24, 2023
@MV88 MV88 linked a pull request May 24, 2023 that will close this issue
12 tasks
@MV88 MV88 self-assigned this May 24, 2023
@offtherailz
Copy link
Member

My opinions about new rules:

  • "react/no-unused-prop-types": 2, LGTM
  • "object-curly-spacing": [2, "never"], that has autofix - LGTM
  • "comma-dangle": [ 2, "always-multiline" ], that has autofix - I personally do not like it to much, because JSON can not have this form in any case, so for constistency, I'd prefer to continue writing the same way. Anyway I understand the advantages in terms of code manipulation and diff

I can suggest also for the future

  • eslint object-curly-newline: ["error", { "consistent": true }]
  • Formatting JSON too

Anyway, in general, I'd like to reduce as much as possible our style rules in favor of standard ones, if possible, so we do not have to maintain a custom eslint configuration. We should see if there is some common used standard config that matches our requirements, maybe the best option

@MV88
Copy link
Contributor Author

MV88 commented May 24, 2023

just want pin out that "comma-dangle" rule is not applied to json, but i feel it is kinda handy to have it,
feel free to reject it, no problem for me

@tdipisa
Copy link
Member

tdipisa commented Mar 4, 2024

Closed in favor of #10005

@tdipisa tdipisa closed this as completed Mar 4, 2024
@tdipisa tdipisa removed this from the 2024.01.00 milestone Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants