Skip to content
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

Upgrade eslint-config in dApp module #99

Closed
1 of 2 tasks
Tracked by #20
nkuba opened this issue Dec 28, 2023 · 0 comments · Fixed by #179
Closed
1 of 2 tasks
Tracked by #20

Upgrade eslint-config in dApp module #99

nkuba opened this issue Dec 28, 2023 · 0 comments · Fixed by #179
Labels

Comments

@nkuba
Copy link
Member

nkuba commented Dec 28, 2023

In #98 we updated eslint configuration to the latest version of @thesis-co/eslint-config for Core, SDK, and Website modules.
When working on this issue we should introduce a similar change in the dApp module.

This issue will be handled in two stages:

  • Upgrade configuration to the latest version (Thesis - eslint config #143)
  • Fix problems reported by eslint but disabled in dapp/.eslintrc.
@nkuba nkuba added the 🎨 dapp dApp label Dec 28, 2023
r-czajkowski added a commit that referenced this issue Jan 4, 2024
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:
```ts
        it("should emit StakeReferral event", () => {
          expect(tx)
            .to.emit(acre, "StakeReferral")
            .withArgs(referral, amountToStake)
        })
```
Without the `await` the test returned a false-positive result and hadn't
checked if expect resolves correctly.

⚠️ Typing checks in the Core module require `typechain/` directory to be
generated before eslint verification is run. It is recommended that on
local environments `pnpm run build` is run before `pnpm 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.
@nkuba nkuba assigned ioay and unassigned ioay Jan 4, 2024
nkuba added a commit that referenced this issue Jan 11, 2024
Refs: #99 

Updated "@thesis-co/eslint-config" version to
"github:thesis/eslint-config#7b9bc8.
Due to the above change, it was necessary to add a few eslint rules.

If we have time for it, we can also address changes to the related
components to limit the number of added rules. The rule
"@typescript-eslint/no-unsafe-member-access", applies to an external
library that does not fit into our default eslint configuration, hence
the need to leave it.
r-czajkowski added a commit that referenced this issue Jan 23, 2024
Fix problems reported by eslint but disabled in dapp/.eslintrc.

Closes: #99
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants