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

Enable noImplicitAny for matrix-js-sdk and matrix-react-sdk #21968

Closed
Tracked by #23542
novocaine opened this issue Apr 28, 2022 · 11 comments · Fixed by matrix-org/matrix-react-sdk#11194
Closed
Tracked by #23542
Assignees
Labels

Comments

@novocaine
Copy link
Contributor

Your use case

https://www.typescriptlang.org/tsconfig#noImplicitAny

This will mitigate errors at runtime caused by incorrect types being passed.

Have you considered any alternatives?

No response

Additional context

No response

@t3chguy
Copy link
Member

t3chguy commented Nov 14, 2022

js-sdk now has strict mode enabled, which includes noImplicitAny

@t3chguy
Copy link
Member

t3chguy commented Jan 16, 2023

Found 59 errors in 34 files. to go

@Johennes
Copy link
Contributor

@t3chguy is the plan for you to fix all of these yourself as part of this issue?

@t3chguy
Copy link
Member

t3chguy commented Jan 17, 2023

@Johennes yes

@Johennes
Copy link
Contributor

Ouch, that seems like a lot then? 😬

I wonder if there's a reasonable way to split this up across multiple people? So that everyone puts in some time rather than just one person having to fight through it alone.

@t3chguy
Copy link
Member

t3chguy commented Jan 17, 2023

Not without stepping on each other's feet and causing merge conflicts, given "fixing" a file often needs fixing the files it depends on too. We have tried to spread the load using the PR CI check to prevent you introducing new regressions but people got too unhappy with it making their PRs take too long so it became non-mandatory.

@Johennes
Copy link
Contributor

Yeah, it does seem better at this point to put in time to fix this with a conscious effort rather than relying on PRs. I'm just a little worried about you having to fix a thousand issues all by yourself. Sounds like that might get quite annoying quickly. 😟

The issue of fixes in one file requiring further fixes in dependent files is tricky though. Could we have people work on their chunks sequentially? With 5 devs, each one could take ~50 files but we'd make sure to only have one person work on their subtask at a time?

@t3chguy
Copy link
Member

t3chguy commented Jan 17, 2023

@Johennes I've easily fixed 5x that already in previous efforts.

With 5 devs, each one could take ~50 files but we'd make sure to only have one person work on their subtask at a time?

We've tried to encourage people onto these tasks previously and rarely did anyone do any. At this point, to get the Q1 web roadmap task back on track, it needs someone to power through them.

@Johennes
Copy link
Contributor

@Johennes I've easily fixed 5x that already in previous efforts.

Ugh. 😳

Ok, well if you're happy to power through them, let's do that. 👍

Just keep in mind that we have 5 people in the immediate team now. So if we wanted to, we could easily organize a team effort here without having to depend on good will.

@t3chguy
Copy link
Member

t3chguy commented Jan 17, 2023

@Johennes if you have ideas on how to task those people with further tsc strict tasks in my absence next week to keep the momentum going then that'd be great to roll

clarkf added a commit to clarkf/matrix-react-sdk that referenced this issue Jan 23, 2023
Changes pulled directly from t3chguy's branch. This removes implicit `any`
occurrences in `<RoomView />`.

See
* matrix-org#9940
* element-hq/element-web#21968

Signed-off-by: Clark Fischer <[email protected]>
clarkf added a commit to clarkf/matrix-react-sdk that referenced this issue Jan 23, 2023
Changes pulled directly from t3chguy's branch. This removes implicit `any`
occurrences in `<RoomView />`.

See
* matrix-org#9940
* element-hq/element-web#21968

Signed-off-by: Clark Fischer <[email protected]>
clarkf added a commit to clarkf/matrix-react-sdk that referenced this issue Jan 24, 2023
Changes pulled directly from t3chguy's branch. This removes implicit `any`
occurrences in `<RoomView />`.

See
* matrix-org#9940
* element-hq/element-web#21968

Signed-off-by: Clark Fischer <[email protected]>
@t3chguy
Copy link
Member

t3chguy commented Feb 3, 2023

Blocked on #21967

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.

3 participants