-
Notifications
You must be signed in to change notification settings - Fork 4
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
Update @thesis-co/eslint-config to the latest 0.8.0-pre version #98
Conversation
When using ethers we should import it from hardhat.
This version enables `@typescript-eslint/recommended-type-checked` rules. The commit hash refers to the merge commit of thesis/eslint-config#12.
cc567ea
to
58576bd
Compare
The problems were reported after the latest upgrade of eslint-config.
Since the eslint rules we enabled for typescript with the latest eslint-config upgrade check typings, we need to ensure that `typechain/` directory is generated before we run eslint. For this reason we first run build and later format.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left 2 non-blocking comments. Overall LGTM 👍
Instead of adding inline comments for unbound-method rule we define it in the eslint config, as the `const { log } = deployments` is and will be broadly used in the scripts.
core/.eslintrc
Outdated
"overrides": [ | ||
{ | ||
"files": ["deploy/*.ts"], | ||
"rules": { | ||
"@typescript-eslint/unbound-method": "off" | ||
} | ||
} | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we are touching the linting config I thought we could also override @typescript-eslint/no-unused-expressions
for test files to use to.be.true/false
. Otherwise, we need to use to.be.equal(true)
see https://github.com/thesis/acre/blob/main/core/test/Dispatcher.test.ts#L68.
{
"files": ["*.test.ts"],
"rules": {
"@typescript-eslint/no-unused-expressions": "off"
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want to use `to.be.true` instead of `to.be.equal(true)`, so we need to disable the rule.
26:70 error Async arrow function has no 'await' expression @typescript-eslint/require-await
This fixes: Error: 861:11 error Unsafe assignment of an `any` value @typescript-eslint/no-unsafe-assignment Error: 861:33 error Unsafe call of an `any` typed value @typescript-eslint/no-unsafe-call Error: 861:33 error Unsafe call of an `any` typed value @typescript-eslint/no-unsafe-call Error: 861:40 error Unsafe member access .Wallet on an `any` value @typescript-eslint/no-unsafe-member-access Error: 861:62 error Unsafe member access .getAddress on an `any` value @typescript-eslint/no-unsafe-member-access
Update
@thesis-co/eslint-config
to the latest version. The commit hash refers to the merge commit of thesis/eslint-config#12. This version enables@typescript-eslint/recommended-type-checked
rules.The configuration enforces multiple typing check rules that I found very useful in the core module's typescript tests, where we were missing
await
in a couple of places.Example:
Without the
await
the test returned a false-positive result and hadn't checked if expect resolves correctly.typechain/
directory to be generated before eslint verification is run. It is recommended that on local environmentspnpm run build
is run beforepnpm run format
. In this PR we update the CI process accordingly.This PR updates the configuration of the Core, SDK, and Website modules.
This PR doesn't update the configuration of the dApp module, we should handle that in #99.