Use eslint 9 to lint js and ts repos with arc lint. Specify a config file that supports current ruleset.
Note that some js/ts repos are not currently covered by .arclint. Will add these in subsequent diffs (ecash-lib, ecash-agora)
Details
Details
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- eslint-to-9
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 32591 Build 64670: Build Diff ecash-herald-tests Build 64669: arc lint + arc unit
Event Timeline
| .arclint | ||
|---|---|---|
| 314 ↗ | (On Diff #52898) | arguably the removal of alias-server is unrelated to this diff, but we are doing some other generalized improvements in line with the eslint upgrade (including js files now that we can globally configure eslint to lint them properly) |
| apps/ecash-herald/tsconfig.json | ||
| 11 ↗ | (On Diff #52898) | this is definitely unrelated but once this was fixed with arc lint I was not able to back it out. I think it's ok to let this change stay in this diff given that it was a lint autofix |
| arcanist/linter/ESLinter.php | ||
| 39 ↗ | (On Diff #52898) | this caused issues users would have to have eslint globally installed, and it would have to be the correct version now the version is pinned top level of the monorepo and this is the binary used for linting the js files. |
| apps/ecash-herald/tsconfig.json | ||
|---|---|---|
| 11 ↗ | (On Diff #52898) | The linter is correct here, the previous comma was a bug. Trailing comma are not allowed in json format unfortunately. |
| arcanist/linter/ESLinter.php | ||
| 27 ↗ | (On Diff #52898) | |
| eslint.config.mjs | ||
| 1 ↗ | (On Diff #52898) | What is a .mjs file ? And also it should be added to the list of supported extensions for the linter (in the .arclint file) otherwise it won't be linted. |
| 49 ↗ | (On Diff #52898) | We already define this in the .arclint file, can we avoid duplicating it ? it's guaranteed to go out of sync, in fact it's already the case. |
| eslint.config.mjs | ||
|---|---|---|
| 1 ↗ | (On Diff #52898) | |
| 49 ↗ | (On Diff #52898) | the .arclint file defines the routes that get linted by eslint at all this file defines linting rules on a per-route basis, e.g. nodejs apps use different linting rules than nextjs apps, cashtab uses jest instead of mocha, etc going forward -- i think .arclint should just eslint "all js, ts, and related files -- everywhere in the monorepo" -- however I think this should be a separate diff. ecash-lib and ecash-agora for example have not been in arclint, so implementing them will bring in numerous linting changes unique to those repos. Could just do it here? they are just lint changes, would show that this approach is working? |
| eslint.config.mjs | ||
|---|---|---|
| 49 ↗ | (On Diff #52898) | I disagree with the approach, this will kill the performance of arc lint. You will run eslint for nothing on files that should be ignored. |
| eslint.config.mjs | ||
|---|---|---|
| 49 ↗ | (On Diff #52898) | Can't you simply remove the matching pattern from the eslint config, so it accepts all ? |
| eslint.config.mjs | ||
|---|---|---|
| 49 ↗ | (On Diff #52898) | per tg discussion -- we need some route-level rules in this top-level eslint.config.mjs ultimately, we will specify "all js and ts files" should be linted in the arcconfig, then the eslint config will funnel them into the relevant linter rules. will patch this in a subsequent diff as the curently-ignored-by-arc-lint js/ts files are expected to have lint changes unrelated to this "use eslint9 instead of eslint8" diff |
Comment Actions
going to put some more thought into this, prob do a one-shot diff that implements this upgrade everywhere.
the current eslint situation is a mess. difficult to fix incrementally as we want all the repos to be on the same standard.