-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
hide never values in intellisense for jsonify #3264
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Can we add some tests here? https://github.com/trpc/trpc/blob/main/packages/tests/server/jsonify.test.ts
Pick<T, FilterDefinedKeys<T>> & { | ||
// Property _is_ a union with `defined`. Set as optional (via `?`) and remove `undefined` from the union | ||
[k in keyof Omit<T, FilterDefinedKeys<T>>]?: Exclude<T[k], undefined>; | ||
}; |
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.
This looks quite "heavy" but before looked heavy too... reminds me of our OmitNeverKeys
that ended up being slow.
FWIW, I think it's fine to expose never keys.... maybe an even better thing would be some DX like this:
const output = {
bigint: 0n,
}
// ... then
serializedOutput.bigint
// ^? Error<"This type is not JSON-serializable">
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.
yeah this is the same as before, I just refactored it a bit. This part doesn't change with the never
keys. It just has to be done unless we somehow come up with something more optimized.
continuation of #3261