Skip to content

Conversation

@joshuaobrien
Copy link
Contributor

@joshuaobrien joshuaobrien commented Nov 5, 2022

This PR narrows the types in TimelineEvent a bit by being explicit about the event types. This means that we can treat TimelineEvent as a tagged union, which results in improved type safety and fewer lines of code.


export interface CommentEvent {
id: number;
graphNodeId: string;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

After narrowing the types, this line will throw an error without the addition of graphNodeId here:

graphNodeId: commentEvent.id,

I think TS was only incidentally happy before.

}, Object.create(null));
}

export class UnreachableCaseError extends Error {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A small helper for switch statements. It can be used to ensure that all variants of a tagged union are handled in a switch statement

@joshuaobrien joshuaobrien changed the title Narrow types in TimelineEvent so that it may be treated as a disriminated union Nov 5, 2022
Copy link
Member

@alexr00 alexr00 left a comment

Choose a reason for hiding this comment

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

NIce change, thank you!

@alexr00 alexr00 self-assigned this Nov 7, 2022
@alexr00 alexr00 added this to the November 2022 milestone Nov 7, 2022
@alexr00 alexr00 merged commit 32de7cc into microsoft:main Nov 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants