Add Prettier and Eslint linting and apply it. #57
Conversation
.eslintrc.js
Outdated
| }, | ||
| env: { | ||
| browser: true, | ||
| es2021: true, |
There was a problem hiding this comment.
If we are using latest ECMA in line 14, why are we not also including it here? Saw this. Do we need to constantly be updating this environment variable as ECMA progresses?
There was a problem hiding this comment.
I don't believe so, but this was actually generated by eslint itself so I'm hesitant about changing it. I'm reading up on it in https://eslint.org/docs/latest/use/configure/language-options
There was a problem hiding this comment.
For documentation purposes - this was addressed here
ProsperousHeart
added
enhancement
|
FYI any changes I made to /src came as a response to eslint error or prettier formatting. Looks like there a few rules we can disable, but just wanted to give you a heads up that that is where the changes came from. |
|
OK. So I realized I messed up the eslint config a bit cause I wasn't letting it play nice with create react app. I fixed that per the docs here: https://create-react-app.dev/docs/setting-up-your-editor/#extending-or-replacing-the-default-eslint-config. Might want to include that in your notes as it also has setups for IDE/code editors (mine is already working with WebStorm). |
Added here |
There was a problem hiding this comment.
Updated documentation with latest changes in #60 PR
A few inquiries and changes requested
| module.exports = { | ||
| extends: ["react-app", "plugin:jsx-a11y/recommended", "prettier"], | ||
| plugins: ["jsx-a11y"], | ||
| }; |
There was a problem hiding this comment.
I saw the env section was removed (from here) - were these no longer needed?
There was a problem hiding this comment.
No that would be defining it twice. If we want to define that now we could do it here.
There was a problem hiding this comment.
if you think it's better to do it the other way (as is now in the code) then I'm good with keeping it like this
Co-authored-by: Kassandra Keeton <ProsperousHeart@users.noreply.github.com>
Spelling change. Co-authored-by: Kassandra Keeton <ProsperousHeart@users.noreply.github.com>
There was a problem hiding this comment.
All items have been addressed, code reviewed, and successfully run locally. Approved! Thank you for your time and patience. Documentation for this has been updated under #60 - to be reviewed and integrated into branch v1-mvp
| module.exports = { | ||
| extends: ["react-app", "plugin:jsx-a11y/recommended", "prettier"], | ||
| plugins: ["jsx-a11y"], | ||
| }; |
There was a problem hiding this comment.
if you think it's better to do it the other way (as is now in the code) then I'm good with keeping it like this
.eslintrc.js
Outdated
| }, | ||
| env: { | ||
| browser: true, | ||
| es2021: true, |
There was a problem hiding this comment.
For documentation purposes - this was addressed here
Adding prettier and eslint for repo linting.
Prettier
Prettier is a code formatter used by the javascript community to automatically enforce one code style so there is no arguing about "spaces/tabs" or single quotes or double quotes, etc. It leaves what we call "bike shedding" (arguing about dumb pointless things) at the door. See https://prettier.io/ for more details.
Eslint
Eslint is used to check code for common errors and automatically correct them. This keeps it easy for reviewers to focus on more complex issues and helps you quickly fix issues. See https://eslint.org/ for more details. We're using the React extension here, which is a common extension for react apps.
Usage
To use these tools run
npm run lintfor eslint ornpm run formatfor prettier or their "check" versions which doesn't apply fixes, but only throws warnings or errors.