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

Strictify on all layers #23542

Closed
22 tasks done
Tracked by #24660
turt2live opened this issue Oct 18, 2022 · 4 comments · Fixed by #25735
Closed
22 tasks done
Tracked by #24660

Strictify on all layers #23542

turt2live opened this issue Oct 18, 2022 · 4 comments · Fixed by #25735
Assignees
Labels
Help Wanted Extra attention is needed T-Epic Issue is at Epic level T-Task Tasks for the team like planning

Comments

@turt2live
Copy link
Member

turt2live commented Oct 18, 2022

Why?

The non-strict aspects of TypeScript allow for a lot of flexibility but also make it easy for errors to slip in, e.g. when there is no strong contract on null-ness.

What?

We’ll fix all existing strict-mode violations and enable all strict-mode checks to run on PRs. This will allow us to eliminate entire classes of errors through static code analysis.

Reference: https://www.typescriptlang.org/tsconfig#strict

Plan

matrix-js-sdk

matrix-react-sdk

  1. A-Developer-Experience A-Testing T-Enhancement
    t3chguy
  2. 54 of 54
    A-Developer-Experience A-Testing T-Enhancement Z-MadLittleMods
    t3chguy

element-web

element-desktop

@turt2live turt2live added T-Task Tasks for the team like planning A-Developer-Experience T-Epic Issue is at Epic level labels Oct 18, 2022
@t3chguy t3chguy self-assigned this Nov 14, 2022
@Johennes Johennes changed the title Epic: Turn on TypeScript strict checks on all layers in Q4 Epic: Turn on TypeScript strict checks on all layers Jan 19, 2023
clarkf added a commit to clarkf/matrix-react-sdk that referenced this issue Jan 23, 2023
This gets RoomView.ts to pass `tsc --strict`. Should be functionally unchanged.

A number of other files were touched to correct signatures.

See:
* element-hq/element-web#23542
* element-hq/element-web#23692

Signed-off-by: Clark Fischer <clark.fischer@gmail.com>
clarkf added a commit to clarkf/matrix-react-sdk that referenced this issue Jan 23, 2023
This gets RoomView.ts to pass `tsc --strict`. Should be functionally unchanged.

A number of other files were touched to correct signatures.

See:
* element-hq/element-web#23542
* element-hq/element-web#23692

Signed-off-by: Clark Fischer <clark.fischer@gmail.com>
clarkf added a commit to clarkf/matrix-react-sdk that referenced this issue Jan 24, 2023
This gets RoomView.ts to pass `tsc --strict`. Should be functionally unchanged.

A number of other files were touched to correct signatures.

See:
* element-hq/element-web#23542
* element-hq/element-web#23692

Signed-off-by: Clark Fischer <clark.fischer@gmail.com>
@daniellekirkwood daniellekirkwood added the Z-AirFocus Moving issues from GH to AirFocus purposefully using this tag. label Feb 2, 2023
@Johennes
Copy link
Contributor

Johennes commented Feb 3, 2023

@t3chguy did a quick pulse-check of what's left in matrix-react-sdk right now:

Strict attribute Errors Files
useUnknownInCatchVariables 151 64
strictFunctionTypes 102 57
strictNullChecks 4749 699
strictPropertyInitialization 167 ~20

Note that strictPropertyInitialization cannot be enabled until strictNullChecks is enabled.

We also have some statistics on https://arewetsyet.bit.ovh/ but they're currently broken.

@t3chguy
Copy link
Member

t3chguy commented Feb 3, 2023

noImplicitAny cannot be enabled until strictNullChecks either in our case due to some of the js-sdk relying on the strict parser for implicit typing

@t3chguy
Copy link
Member

t3chguy commented Feb 15, 2023

I am making good progress with strictNullChecks - down to 2645 errors in 400 files

@Johennes Johennes changed the title Epic: Turn on TypeScript strict checks on all layers Strictify on all layers Mar 1, 2023
@Johennes Johennes added Help Wanted Extra attention is needed and removed Z-AirFocus Moving issues from GH to AirFocus purposefully using this tag. A-Developer-Experience labels Mar 1, 2023
@paboum
Copy link

paboum commented Mar 9, 2023

The non-strict aspects of TypeScript allow for a lot of flexibility but also make it easy for errors to slip in, e.g. when there is no strong contract on null-ness.

I never understood the so called "front end" programming, but it seems to me this is basically removal of https://en.wikipedia.org/wiki/Undefined_behavior and, as such, a technical debt. It is somewhat interesting how new generations of programmers believe they can "solve" the issues of having to controlling the flow by selectively ignoring it, but luckily, everyone learns it is unavoidable sooner or later.

Though I cannot contribute here due to lack of js/ts expertise, I would like to cheer @t3chguy up, if you find any errors that cannot simply be solved by single line edits, you will probably be able to isolate meaningful bug reports for others to resolve, or even find specification omissions that will impact other projects. It's better if you do it, not the users filing bug reports, so your effort matters.

@Johennes Johennes added the Z-AirFocus Moving issues from GH to AirFocus purposefully using this tag. label Mar 27, 2023
@Johennes Johennes added T-Epic Issue is at Epic level and removed T-Epic Issue is at Epic level Z-AirFocus Moving issues from GH to AirFocus purposefully using this tag. labels Apr 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Help Wanted Extra attention is needed T-Epic Issue is at Epic level T-Task Tasks for the team like planning
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants