Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Conform more of the codebase to strictNullChecks #10842

Merged
merged 10 commits into from
May 11, 2023
Merged

Conversation

t3chguy
Copy link
Member

@t3chguy t3chguy commented May 10, 2023

For element-hq/element-web#21967

Split into meaningful commits to aid review


This change is marked as an internal change (Task), so will not be included in the changelog.

@t3chguy t3chguy added the T-Task Refactoring, enabling or disabling functionality, other engineering tasks label May 10, 2023
@t3chguy t3chguy requested a review from a team as a code owner May 10, 2023 13:20
Copy link
Contributor

@alunturner alunturner left a comment

Choose a reason for hiding this comment

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

Just the one non-blocking question, looks good to me

Comment on lines 41 to 44
export function isAppWidget(widget: IWidget | IApp): widget is IApp {
return typeof widget["roomId"] === "string";
}

Copy link
Contributor

Choose a reason for hiding this comment

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

In a piece of work I started a Typeguards.ts file in the src folder. I'm still not entirely sure if it would be better to put typeguards in one file, or always keep them closer to what they're guarding for, like here.

What do you think? Better to keep here or consolidate in a single typeguards file?

Copy link
Member Author

Choose a reason for hiding this comment

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

My personal preference is closer to the object, in you case I think it makes sense for generic ones which don't apply to a specific type, like null

@t3chguy t3chguy merged commit 82e3203 into develop May 11, 2023
@t3chguy t3chguy deleted the t3chguy/types667 branch May 11, 2023 08:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Task Refactoring, enabling or disabling functionality, other engineering tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants