Upgrade ESLint to 8.x #2386
Upgrade ESLint to 8.x #2386
Conversation
There was a problem hiding this comment.
Thanks for working on this. While we're updating the major bits of eslint, let's also take the opportunity to update all the plugins. I've left comments on the ones I think we should change.
You might want to update them one-by-one locally and run eslint each time to make sure nothing breaks.
docs/environment-setup.md
Outdated
| @@ -35,8 +35,9 @@ There are two methods to install Redis on Windows. We strongly recommend the fir | |||
| ##### Option 1: Using [WSL2 (Windows Subsystem Linux)](https://docs.microsoft.com/en-us/windows/wsl/about) | |||
|
|
|||
| 1. Follow Microsoft WSL2 [installation guide](https://docs.microsoft.com/en-us/windows/wsl/install-win10) to complete the installation | |||
| 2. Depending on the distribution you chose for WSL2, install redis using that distro's package manager. For example, if using Ubuntu, run `sudo apt-get install redis` | |||
| 2. Depending on the distribution you chose for WSL2, install redis using that distro's package manager. For example, if using Ubuntu, run `sudo apt-get install redis` . Note: make sure to run `sudo apt update` after WSL2 has been installed. | |||
There was a problem hiding this comment.
This change is good, but really belongs in a separate PR. We want to try and isolate changes so that we only update one thing at a time, and everything that gets updated is related. The reason for this is that later on, if something breaks, we can track down exactly what happened. If we mix multiple things into a fix, it makes it harder to undo something, since there are layers of changes all combined.
In this case, the change is innocuous, and unlikely to be a problem. However, in the future, prefer multiple, small PRs to combining things in one.
package.json
Outdated
| @@ -104,10 +104,10 @@ | |||
| "babel-jest": "26.6.3", | |||
| "babel-preset-next": "1.4.0", | |||
| "cross-env": "7.0.3", | |||
| "eslint": "7.23.0", | |||
| "eslint": "8.0.0", | |||
| "eslint-config-airbnb": "18.2.1", | |||
| "eslint-config-prettier": "8.1.0", | |||
There was a problem hiding this comment.
Let's update this to 8.3.0 while you're here.
package.json
Outdated
| "eslint-config-airbnb": "18.2.1", | ||
| "eslint-config-prettier": "8.1.0", | ||
| "eslint-plugin-import": "2.22.1", | ||
| "eslint-plugin-import": "2.25.2", | ||
| "eslint-plugin-jest-playwright": "0.2.1", |
package.json
Outdated
| "eslint-config-airbnb": "18.2.1", | ||
| "eslint-config-prettier": "8.1.0", | ||
| "eslint-plugin-import": "2.22.1", | ||
| "eslint-plugin-import": "2.25.2", | ||
| "eslint-plugin-jest-playwright": "0.2.1", | ||
| "eslint-plugin-jsx-a11y": "6.4.1", | ||
| "eslint-plugin-prettier": "3.4.0", |
There was a problem hiding this comment.
Let's update eslint-plugin-promise to 5.1.1
There was a problem hiding this comment.
Let's update eslint-plugin-react to 7.26.1
|
@BeAmazedVariable referring to your #2369 (comment). The environment-setup.md changes in our two PRs (#2386 and #2375) will not conflict much because they're targeting different problems. I don't mind the minor scope creep here and I'll review this PR once you address the other comments mentioned by @humphd . Thank you for your contribution :) |
package.json
Outdated
| @@ -104,10 +104,10 @@ | |||
| "babel-jest": "26.6.3", | |||
| "babel-preset-next": "1.4.0", | |||
| "cross-env": "7.0.3", | |||
| "eslint": "7.23.0", | |||
| "eslint": "8.0.0", | |||
There was a problem hiding this comment.
This is at 8.1.0 now, see https://www.npmjs.com/package/eslint
|
@humphd is it a good practice to commit those pluggin changes seperately and if the code don't break I will squash them into one commit ? |
|
I think they are sufficiently related to this change, since we're doing a major update to eslint, so updating plugins at the same time is a logical step. |
|
@BeAmazedVariable can we wrap this up soon please? |
|
@humphd Yes I will be able to |
|
@humphd I just push my commit again, please kindly review it. |
cindyledev
changed the title
Issue This PR Addresses
Type of Change
Description
#2362 I use
npm install eslint@8.0.0to upgrade our ESLint to the newest version. Since theeslint-plugin-importalso need to be update to support ESLint 8.x I use thenpm install eslint-plugin-import@latestbecause I can not safely update it usingnpm update <package-name>@lastestdue to our semver range.I also add some changes inside of our docs/environment-setup.md suggest running
sudo apt updatefor newly installed WSL2 and adding a link on how to install Node for WSL2Checklist