-
Notifications
You must be signed in to change notification settings - Fork 938
chore(site): fix typegen usage #868
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
Conversation
Codecov Report
@@ Coverage Diff @@
## main #868 +/- ##
==========================================
+ Coverage 63.90% 66.05% +2.15%
==========================================
Files 199 203 +4
Lines 11830 13226 +1396
Branches 87 86 -1
==========================================
+ Hits 7560 8737 +1177
- Misses 3442 3603 +161
- Partials 828 886 +58
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks for the investigation and write-up and updating the conventions. Really thorough stuff!
@@ -3,7 +3,7 @@ import * as Types from "../../api/types" | |||
import * as API from "../../api" | |||
|
|||
export interface UserContext { | |||
getUserError?: Error | unknown // unknown is a concession while I work out typing issues | |||
getUserError?: Error | unknown |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Open Question: Maybe this is something we talk about at FE V or down the line; I am documenting it just for completion. I'm wondering if we will strategize around discerning errors using predicates:
const isUserError = (e: any): e is UserError => {
// ...
}
If so, we might want to associate errors with types in some meaningful way and logically group these predicates. I'll want to think on that architecturally speaking with you, and I do not feel compelled to make a quick decision yet, as we're OK right now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding to FE V!
as Error
, type checking as they come in, or be defensive throughout?)event.type
wasSIGN_IN
even though the typegen file knew that it was alwaysSIGN_IN
is that I had annotatedevent
asUserEvent
. The lesson is, let typegen figure out your types for you.I updated the Code Conventions doc with info on how to use typegen.