hackforla
Adding prettier and eslint to frontend
Fixes #588
Changes
- In package.json under devDependencies I installed prettier and EsLint and wrote scripts to run them.
- Created a eslint.config.mjs file to configure ESLint.
Side Notes:
- To the linter rules config I just added basic rules for now like indentation, no whitespaces, etc.. we can discuss others we'd like to add.
- I also tested the linter with poorly written ts code and tsx code and the linter catches the errors and prettier fixes them.
Running eslint . works for me -- so I can confirm eslint is set up correctly. Nice!
Also checked if it detects typescript errors and it works - good job 👍
Adding a formatter and a linter to the frontend will be very important to long term health of our project. So thank you for doing this.
There are some problems I would like you to fix
- Running the
npm run lintcommand doesnt work for me:
Can you fix that script please? I think eslint . should be enough
- There's 471 issues found by the linter:
If we merge this into develop, everyone will have a lot of linter errors next time they pull from the develop branch.
Could you try running eslint . --fix to automatically resolve as many of them as possible? Lets keep it in this PR.
We can inform the team of the new changes and make an issue to resolve the rest of them after.
Update: I ranESLint . --fixto automatically resolve most linting errors. The remaining issues include console logs, some unused variables, type errors declared as "any," and prop validation concerns. I went through and fixed some of these manually as well. While these issues won't impact our project at the moment, they should be addressed in the future. @LoTerence What are your thoughts on whether we should fix the "any" type errors and prop validation now, or can we prioritize them for later?
Additionally, I've added the TailwindCSS plugin with the following rules:
"tailwindcss/no-contradicting-classname": "error",
"tailwindcss/no-unnecessary-arbitrary-value": "error",
I tested the configuration, and the linter successfully caught the relevant errors.
Thank you for the updated PR! Great job on fixing the npm script and running the autofix 💯 Thank you for taking the time to manually fix others as well! The tailwind plugins look good too 👌
What are your thoughts on whether we should fix the "any" type errors and prop validation now, or can we prioritize them for later?
Yes, lets prioritize them later. Could you make an issue with the details you found about the remaining linter errors? (Or add it to the already existing issue, I will leave it up to you).
We will get the whole frontend team to help with this. It will be good practice for you all.
Hey @LoTerence, it seems I don't have write access to fix the merge conflicts. Is this something I need to do on my end to give myself access?
Hey @LoTerence, it seems I don't have write access to fix the merge conflicts. Is this something I need to do on my end to give myself access?
Its not something you need access to on github, you have to pull the newest changes from hackforla:develop into your branch, make changes to resolve the merge conflicts, then push them back up to github.
Its a lot to explain, check out this video to see what I'm talking about: https://www.youtube.com/watch?v=xNVM5UxlFSA
@LoTerence, I fixed the merged conflicts. Should be able to merge the PR now :)
Awesome! Amazing job! Go ahead and click the merge button whenever you are ready 😄
@LoTerence, thanks! I don't think I can merge it says "You’re not to merge this pull request." Is there something I do on my end?
Sorry about that, just fixed the project settings so you should be able to click the merge button now 👌