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

Added client type checking to webpack #4619

Merged
merged 2 commits into from
Aug 23, 2022

Conversation

antoniomika
Copy link
Contributor

This PR includes changes to add type checking (via TypeScript) for the client-side code base. webpack does not currently report TypeScript errors (which poses an issue).

This was done out of a message on IRC via @MaxLeiter.

The changes here are broken up between two commits, one to enable type checking and another to fix the issues the compiler has found.

Copy link
Member

@brunnre8 brunnre8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a bunch for your help here, build system work is not particularly fun to do so that's very much appreciated.

There are some areas where we should be a bit more careful with the error handling though, rather than just asserting things we can't possibly guarantee.

Mind fixing those up?

There's also some questions out of interest.

client/js/helpers/collapseNetwork.ts Outdated Show resolved Hide resolved
client/js/helpers/contextMenu.ts Outdated Show resolved Hide resolved
client/tsconfig.json Show resolved Hide resolved
server/client.ts Outdated Show resolved Hide resolved
server/plugins/auth.ts Outdated Show resolved Hide resolved
server/plugins/inputs/index.ts Outdated Show resolved Hide resolved
server/server.ts Outdated Show resolved Hide resolved
server/server.ts Outdated Show resolved Hide resolved
@brunnre8 brunnre8 added Meta: Internal This is an internal codebase change (testing, linting, etc.). Status: changes-requested Review happened but some changes need to be made labels Aug 14, 2022
@antoniomika antoniomika force-pushed the am/client-type-checking branch 2 times, most recently from c3991ae to 4d15fb7 Compare August 14, 2022 16:19
@brunnre8 brunnre8 added Status: needs-review PR not yet reviewed by enough maintainers and removed Status: changes-requested Review happened but some changes need to be made labels Aug 15, 2022
@MaxLeiter
Copy link
Member

Thank you! This seems to work great. I left one small comment and then I'm okay with merging.

@MaxLeiter MaxLeiter added Status: changes-requested Review happened but some changes need to be made and removed Status: needs-review PR not yet reviewed by enough maintainers labels Aug 16, 2022
@MaxLeiter MaxLeiter merged commit 117c5fa into thelounge:master Aug 23, 2022
@MaxLeiter MaxLeiter removed the Status: changes-requested Review happened but some changes need to be made label Aug 23, 2022
@MaxLeiter MaxLeiter added this to the 5.0.0 milestone Aug 23, 2022
@MaxLeiter MaxLeiter modified the milestones: 5.0.0, 4.4.0 Apr 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Meta: Internal This is an internal codebase change (testing, linting, etc.).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants