-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Enhance client data type inference #8269
Conversation
🦋 Changeset detectedLatest commit: b84bf03 The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
packages/remix-react/components.tsx
Outdated
? Awaited<ReturnType<T>> | ||
: SerializeFrom<T> | ||
: SerializeFrom<T> | ||
); |
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.
We return Awaited<ReturnType<T>>
only when the generic is a function that doesn't return a Response
and has parameters of ClientLoaderFunctionArgs
or ClientActionFunctionArgs
: Awaited<Output> // client naked object is just the object | ||
: SerializeFromImpl<T> | ||
: SerializeFromImpl<T> | ||
); |
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.
Exporting this as part of SerializeFrom
ensures we get this across the board in useLoaderData
, useActionData
, useFetcher
, useRouteLoaderData
, meta matches.data
, etc.
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.
Could you add test cases to the bottom of the serialize.ts
file?
@@ -21,6 +21,9 @@ export type Jsonify<T> = | |||
T extends Number ? number : | |||
T extends Boolean ? boolean : | |||
|
|||
// Promises JSON.stringify to an empty object | |||
T extends Promise<unknown> ? EmptyObject : |
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.
Parameters<T> extends [ClientLoaderFunctionArgs | ClientActionFunctionArgs] ? | ||
// Client data functions may not serialize | ||
SerializeClient<Awaited<Output>> | ||
: |
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.
Cleaned it up so that this is all that's new and we don't touch the flow for existing loaders/actions
// note: cannot be inlined as logic requires union distribution | ||
// prettier-ignore | ||
type SerializeClient<Output> = | ||
Output extends TypedDeferredData<infer U> ? | ||
// top-level promises | ||
& { | ||
[K in keyof U as K extends symbol | ||
? never | ||
: Promise<any> extends U[K] | ||
? K | ||
: never]: DeferValueClient<U[K]>; // use generic to distribute over union | ||
} | ||
// non-promises | ||
& { | ||
[K in keyof U as Promise<any> extends U[K] ? never : K]: U[K]; | ||
} | ||
: | ||
Output extends TypedResponse<infer U> ? Jsonify<U> : | ||
Awaited<Output> | ||
|
||
// prettier-ignore | ||
type DeferValueClient<T> = | ||
T extends undefined ? undefined : | ||
T extends Promise<unknown> ? Promise<Awaited<T>> : | ||
T; | ||
|
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.
Copies of the Serialize
/DeferValue
types with Jsonify
stuff removed (unless it's a TypedResponse
and then we still call Jsonify
)
| TypedResponse<T> // returned responses | ||
| TypedResponse<never> // thrown responses | ||
>; | ||
type Loader<T> = () => Promise<TypedResponse<T>>; |
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.
@pcattori and I worked through some examples and we don't need to represent the never types since they don't show up in the signature if you throw conditionally.
🤖 Hello there, We just published version Thanks! |
🤖 Hello there, We just published version Thanks! |
WIP for enhanced type inference for client data functions. We should be able to infer when it's a
clientLoader
/clientAction
based on the types parameters and avoid deserializing if it doesn't return aResponse
...