Closed nullndr closed 11 months ago
Maybe this is related to TypeScript, but should I use the LoaderData
in this case?
Thought we should completely replace it
I confirm I have the same "bug" of TS types with the tutorial
Another example:
import { useLoaderData } from "@remix-run/react";
import { json } from "@remix-run/server-runtime";
export function loader() {
const x = Math.random();
if (x === 1) {
return json({ a: "stringA" });
}
if (x === 2) {
return json({ a: 2});
}
if (x === 3) {
return json({ a: 2, b: 2 });
}
return json({ c: "stringC" });
}
export default function AppTest() {
const loaderData = useLoaderData<typeof loader>();
...
}
const loaderData: SerializeObject<{
a: string;
}> | SerializeObject<{
a: number;
}> | SerializeObject<{
c: string;
}>
b
is never there.
Changing the type of a
in the x === 3
case fixes the type
export function loader() {
const x = Math.random();
if (x === 1) {
return json({ a: "stringA" });
}
if (x === 2) {
return json({ a: 2});
}
if (x === 3) {
return json({ a: [2], b: 2 });
}
return json({ c: "stringC" });
}
export default function AppTest() {
const loaderData = useLoaderData<typeof loader>();
...
}
```ts
const loaderData: SerializeObject<{
a: string;
}> | SerializeObject<{
a: number;
}> | SerializeObject<{
a: number[];
b: number;
}> | SerializeObject<{
c: string;
}>
The conclusion I was able to draw thus far is if an object is returned that has a property of the same type, but an extra property added as well, it doesn't get picked up.
If I take the first example and completely remove the x === 2
case, the b
property is picked up and the type if as follows:
export function loader() {
const x = Math.random();
if (x === 1) {
return json({ a: "stringA" });
}
// if (x === 2) {
// return json({ a: 2});
// }
if (x === 3) {
return json({ a: 2, b: 2 });
}
return json({ c: "stringC" });
}
export default function AppTest() {
const loaderData = useLoaderData<typeof loader>();
...
}
const loaderData: SerializeObject<{
a: string;
}> | SerializeObject<{
a: number;
b: number;
}> | SerializeObject<{
c: string;
}>
Unfortunately I think we're going to continue to get a lot of these weird edge cases. The issue is that Remix is trying to convert your actual data type, with all its various forms, and convert that to what the type would look like if you replaced any non-JSON types with their serialized versions.
They are using a home-grown function to do this, but plan to use Jsonify
from type-fest
. Even then, I'm not sure it it will every be 100% perfect.
I side-stepped this with remix-typedjson
by simply using whatever type that TypeScript thinks the loader is returning. In order to do this, I also wrote my own JSON serialize/deserialize function (similar to superjson
) to ensure that non-JSON values like Date
, BigInt
, etc. are converted back to their native values after parsing.
Thank you @kiliman, we started using remix-typedjson
right away when I saw this.
Can this be included in remix, types returned from remix are unusable. When using prisma,
Remix Json: SerializeObject<UndefinedToOptional<Person>>[]
Typed Json: Person[]
@kiliman Thanks!
Sadly, even with remix-typedjson it still doesn't work for me.
export interface LinksGroupProps {
icon: TablerIcon;
label: string;
initiallyOpened?: boolean;
links?: { label: string; link: string }[];
}
export const loader = async ({request}: LoaderArgs) => {
const data = {
main: [
{
link: "/taskapp",
label: "Task App",
links: [
{
link: "/tasks",
label: "Tasks"
},
{
link: "/processes",
label: "Processes"
},
{
link: "/cases",
label: "Cases"
}
]
},
{
link: "/modeler",
label: "Modeler",
},
{
link: "/admin",
label: "Admin"
},
{
link: "/idm",
label: "IDM"
}
],
side: [
{ icon: IconGauge, label: 'Dashboard' },
{
label: 'Market news',
initiallyOpened: true,
links: [
{ label: 'Overview', link: '/' },
{ label: 'Forecasts', link: '/' },
{ label: 'Outlook', link: '/' },
{ label: 'Real time', link: '/' },
],
icon: IconNotes,
},
{
label: 'Releases',
links: [
{ label: 'Upcoming releases', link: '/' },
{ label: 'Previous releases', link: '/' },
{ label: 'Releases schedule', link: '/' },
],
icon: IconCalendarStats,
},
{ label: 'Analytics', icon: IconPresentationAnalytics },
{ label: 'Contracts', icon: IconFileAnalytics },
{ label: 'Settings', icon: IconAdjustments },
{
label: 'Security',
links: [
{ label: 'Enable 2FA', link: '/' },
{ label: 'Change password', link: '/' },
{ label: 'Recovery codes', link: '/' },
],
icon: IconLock,
}
]
}
// return json(data)
return typedjson(data)
}
export default function Tasks() {
//const {main, side} = useLoaderData() as LoaderData
const {main, side} = useTypedLoaderData<typeof loader>()
console.log("side", side)
return (
<>
<HeaderComponent links= {main} />
<NavbarNested links= {side} />
<Outlet/>
</>
);
}
It seems like complex type like TablerIcon doesn't get serialized and send correctly across the wire.
Yes, as explained in remix-typedjson
, it only supports a subset of types: Date
, BigInt
, etc. I'm not sure what your custom types look like, so hard to say if it should serialize or not.
You could also try remix-superjson
. It uses superjson
to serialize and supports more complex types.
Thanks for the clarification.
For the remix tutorial, adding 'unknown' converstion seemed to make TS happy
const { posts } = useLoaderData() as unknown as LoaderData;
I’m having trouble with data coming straight from a GraphQL API, i.e. data that should already be serialised. Would it be a good idea to be able to allow the user to “opt out” of type serialisation (e.g. via a type argument)?
I don’t know enough about TypeScript to know if the serialisation problem is easily solvable — but if not, this suggestion seems to be the easiest fix.
Edit: please ignore, my issue was elsewhere and has been fixed in the meantime.
For the remix tutorial, adding 'unknown' converstion seemed to make TS happy
const { posts } = useLoaderData() as unknown as LoaderData;
For other people's reference, this is force casting: https://www.w3schools.com/typescript/typescript_casting.php
What would be the logical thing to do if done correctly without force casting? Should LoaderData
type be updated or useLoaderData()
method be updated. Ideally useLoaderData() should not have to go through unknown
casting, so curious.
Don't want to sound rude, but a feedback to the Remix Team - Facing the same issue and this has been frustrating! I had heard a lot about Remix but these kinds of errors while trying out its first tutorial makes me not want to use it for projects :(
useLoaderData<LoaderData>()
+1 this is definitely confusing and not clear how best to handle. I would fully expect to be able to do this:
const { user, notes}: { user: User | undefined, notes: Note[]} = useLoaderData<typeof loader>();
// user is type User | undefined
// notes is type Note[]
given a loader
function akin to,
export async function loader({ request }: LoaderArgs) {
const user = await useOptionalUser();
const notes = user ? await getNotes({ userId: user.id }) : ([] as Note[]);
return json({ user, notes });
}
quick edit: thinking about this some more, perhaps a prisma-client
model could automatically provide a toJSON() serializer ...perhaps it's not json
's job to handle this, but having it in the indie-stack example makes it confusing when slight adjustments leads to the SerializeObject<UndefinedToOptional<Note>>[]
issue.
Yeah, I'm not a big fan of the SerializeObject
type. That's why I wrote remix-typedjson
. It will automatically convert non-JSON types back into their native types, so you can use the actual types instead of the JSON-converted ones.
Is there any update on whether a solution akin to remix-typedjson
will be introduced within Remix?
So a lot has passed since I opened this issue and I read all your comments.
The snippet I shared at the start was a simplified version of a loader
I actually have in a project for Shopify, in this time I enhanced the project, updating also remix to 1.14.1
, so I would like to share with you some of my thoughts.
For this, take the following loader
for an embedded Shopify app:
export const loader = async ({ request }: LoaderArgs) => {
const url = new URL(request.url);
const shopifyDomain = url.searchParams.get("shop");
const host = url.searchParams.get("host");
if (shopifyDomain && host) {
const session = await shopSession.getSession(request.headers.get("Cookie"));
if (session.get("shopifyDomain") === shopifyDomain) {
const shop = await findShopInAuth({ shopifyDomain });
if (shop) {
const apiKey = Env.get("SHOPIFY_API_KEY");
return {
success: true,
apiKey,
host,
};
}
}
throw redirect(`/api/auth?shop=${shopifyDomain}&host=${host}`);
}
return {
success: false,
};
};
The return type of this loader
is
Promise<{
success: boolean;
apiKey: string;
host: string;
} | {
success: boolean;
apiKey?: undefined;
host?: undefined;
}>
Please, keep in mind that this is just how typescript infer the return type.
So you can't correctly discriminate this union:
export default function EmbeddedLayout() {
const loaderData = useLoaderData<typeof loader>();
const locale = useLocale();
const { ready } = useTranslation("translation", { useSuspense: false });
if (ready) {
return loaderData.success ? (
<EmbeddedApp {...loaderData}> // error
<Outlet />
</EmbeddedApp>
) : (
<AppProvider i18n={locale} linkComponent={Link}>
<Outlet />
</AppProvider>
);
}
return <></>;
}
type EmbeddedAppProps = React.PropsWithChildren<{
apiKey: string;
host: string;
}>;
declare function EmbeddedApp({ children, apiKey, host }: EmbeddedAppProps);
This raise an error since loaderData
is of type:
SerializeObject<UndefinedToOptional<{
success: boolean;
apiKey: string;
host: string;
}>> | SerializeObject<UndefinedToOptional<{
success: boolean;
apiKey?: undefined;
host?: undefined;
}>>
So, should we use a LoaderData
type? Well, you could:
type LoaderData =
| { success: true; apiKey: string; host: string }
| { success: false };
export default function EmbeddedLayout() {
const loaderData = useLoaderData<LoaderData>();
const locale = useLocale();
const { ready } = useTranslation("translation", { useSuspense: false });
if (ready) {
return loaderData.success ? (
<EmbeddedApp {...loaderData}> // all good
<Outlet />
</EmbeddedApp>
) : (
<AppProvider i18n={locale} linkComponent={Link}>
<Outlet />
</AppProvider>
);
}
return <></>;
}
but from my little experience I don't see this approach scalable, since if the loader
has to return another object, you should also update the LoaderData
type, otherwise you will get some errors.
I don't like this approach, so instead I started to use as const
on returns:
return {
success: true,
apiKey,
host,
} as const;
...
return {
success: false,
} as const;
From this typescript infer the following type:
Promise<{
readonly success: true;
readonly apiKey: string;
readonly host: string;
} | {
readonly success: false;
readonly apiKey?: undefined;
readonly host?: undefined;
}>
That is a truly discriminated union, and now loaderData
is
SerializeObject<UndefinedToOptional<{
readonly success: true;
readonly apiKey: string;
readonly host: string;
}>> | SerializeObject<UndefinedToOptional<{
readonly success: false;
readonly apiKey?: undefined;
readonly host?: undefined;
}>>
This works great, no force casting with unknown
and no LoaderData
type.
A little enhancement from Remix would be to remove from the type keys with undefined
as value, since they won't exists in the returned json, but I think it is ok now.
@miniplus this could also fix your issue, however I don't know why you never see b
, I use typescript 4.9.5, and the following typescript works:
function loader() {
const x = Math.random();
if (x === 1) {
return { a: "stringA" } as const;
}
if (x === 2) {
return { a: 2 } as const;
}
if (x === 3) {
return { a: 2, b: 2 } as const;
}
return { c: "stringC" } as const;
}
export type Prettify<T> = { [K in keyof T]: T[K] } & {};
type RemoveUndefined<T> = Prettify<{ -readonly [K in keyof T as T[K] extends undefined ? never : K]: T[K]}>
const bar: RemoveUndefined<ReturnType<typeof loader>> = loader();
The type of bar
is:
{
a: "stringA";
} | {
a: 2;
} | {
a: 2;
b: 2;
} | {
c: "stringC";
}
Here is a playground
@uhrohraggy from the type you posted, user
should be User | null
, and not User | undefined
, null
will be correctly serialized into the json, undefined
won't
I have Remix 1.14.3
and Typescript 4.9.5
in my project but this approach doesn't work for me in case I return nested structures from loader:
return {
foo: {
bar: "it's a string"
}
}
For flat structures it does work also without as const
.
Hey @kwiat1990, could you provide more details for your problem please?
I think I didn't quite get the last part of your example in which you call loader()
. How it should be using along with useLoaderData
?
@kwiat1990 oh sorry, that was a simple typescript example, here is how you should translate it to remix:
export const loader = () => {
const x = Math.random();
if (x === 1) {
return { a: "stringA" } as const;
}
if (x === 2) {
return { a: 2 } as const;
}
if (x === 3) {
return { a: 2, b: 2 } as const;
}
return { c: "stringC" } as const;
};
export default function Index() {
const loaderData = useLoaderData<typeof loader>();
if ("a" in loaderData) {
loaderData.a; // "stringA" | 2
}
if ("b" in loaderData) {
loaderData; // { a: 2; b: 2; }
}
if ("c" in loaderData) {
loaderData; // { c: "stringC"; }
}
return <div></div>;
}
As I said, the loaderData
type will encapsulate the actual data in SerializeObject<UndefinedToOptional<...>>
, so if you want to remove undefined keys you have to type it like the following:
type Prettify<T> = { [K in keyof T]: T[K] } & {};
type RemoveUndefined<T> = Prettify<{
-readonly [K in keyof T as T[K] extends undefined ? never : K]: T[K];
}>;
const loaderData: RemoveUndefined<Awaited<ReturnType<typeof loader>>> =
useLoaderData<typeof loader>();
Or better, write your own useLoaderData
hook like this:
function useLoaderData<T extends () => any>(): RemoveUndefined<
Awaited<ReturnType<T>>
> {
return useRemixLoaderData<T>();
}
OK then, it seems I have made everything correctly but like I said before, with this Typescript throws following error:
Type 'SerializeObject<UndefinedToOptional<{ readonly article: { readonly content: string; readonly readTime: string; readonly author: Author; readonly category: Category; readonly createdAt: string; readonly id: string; ... 6 more ...; readonly cover?: Image | undefined; }; readonly comments: Comment[]; readonly commentsC...' is not assignable to type '{ article: { readonly content: string; readonly readTime: string; readonly author: Author; readonly category: Category; readonly createdAt: string; readonly id: string; ... 6 more ...; readonly cover?: Image | undefined; }; comments: Comment[]; commentsCount: number; pagination: Pagination; relatedArticles: Article[...'.
The types of 'article.author.avatar' are incompatible between these types.
Type 'SerializeObject<UndefinedToOptional<Image>> | undefined' is not assignable to type 'Image | undefined'.
Type 'SerializeObject<UndefinedToOptional<Image>>' is not assignable to type 'Image'.
Types of property 'createdAt' are incompatible.
Type 'string | undefined' is not assignable to type 'Date | undefined'.
Type 'string' is not assignable to type 'Date'.ts(2322)
One of the things being returned from the loader and what's causing Typescript error is article
, which looks like this:
export interface Article {
author: Author;
category: Category;
content: string;
createdAt: string;
id: string;
lead: string;
publishedAt: string;
slug: string;
tags: Tag[];
title: string;
updatedAt: string;
cover?: Image;
readTime?: string;
}
@kwiat1990 from the error Typescript raised the problem is in the type of article.author.avatar.createdAt
that is serialized correctly as string
since initially it is of type Date
, in order to fix this issue you have to recreate a Date
object from it.
A possible solution (although not elegant) could be the following:
export default function() {
const loaderData = useLoaderData<typeof loader>();
return <Component
article={
{
...loaderData,
author: {
...loaderData.author,
avatar: {
...loaderData.author.avatar,
createdAt: new Date(loaderData.author.avatar.createdAt)
}
}
}
}/>
}
In any place or component I expect a Date
object. For that I have a helper function, which formats a date from a Date
object or a string
. In data from loader everything date-related is a string
.
I get mentioned error at this place in my code:
const loaderData: RemoveUndefined<Awaited<ReturnType<typeof loader>>> =
useLoaderData<typeof loader>();
The bottom line is that with the changes how Remix types loader data a lot of things got broken and currently I don't see any easy way in order to fix it. The typedJson
seems to cover only simply types.
@kwiat1990 oh sorry, the problem here is that the type from useLoaderData
can not be assigned to loaderData
since it is of type RemoveUndefined<Awaited<ReturnType<typeof loader>>>
.
For this, you should also pass ReturnType<typeof loader>
to SerializeObject<UndefinedToOptional<...>>
Thanks for clarification. It feels a bit counterproductive, sort of, to fight against a framework of your choice. I would rather stick to my own interfaces/types for loader data, which I need to write by myself than to use more and more wrappers to get rid of Remix types.
The issue here is that Remix is trying NOT to lie to you by specifying the actual result type you're getting, which may differ from what you returned from your loader due to JSON conversion.
And yes, it can be annoying, especially for nested objects. I'm sure it's difficult to have some automated way to infer this result type for countless return values that will work well in all cases.
That's why I punted, and instead of returning a JSON value, I returned my typed JSON, which automatically converts back to its native form. Then I can simply use the actual type from the loader inferred by TypeScript.
My issue with this behavior is that I need to manage to write some Typescript wrapper for this to work or install yet another dev dependency and hope it works with my custom types. In both cases I'm not the happiest guy in the world. I would really prefer that Remix says: hey, you need type those things on your own. We can't.
I think the solution here is using SerializeFrom
from @remix-run/server-runtime
.
e.g.
export default function PostList({ posts }: { posts: SerializeFrom<Post[]>; }) {
return (
<div>
Posts!
</div>
);
}
@graysonhicks sadly this method is not scalable
For now typedJson
seems to be the best alternative. But I'm still not sold on the idea it's not handled by Remix itself.
I heard there was a PR to fix this, right?
Yeah the serialized types have some challenges if you need to maintain the same type across the stack.
Would rather useLoaderData<typeof loader>
enforce the response is serializable, rather than convert the type to SerializeObject<UndefinedToOptional<T>>
.
Basically what @kwiat1990 said, https://github.com/remix-run/remix/issues/3931#issuecomment-1489848103
I would really prefer that Remix says: hey, you need type those things on your own. We can't.
Personally encountered issues where it violates the contract with my graphql-codegen
types and fragment masking
Such as here, representing id
as an optional field when it's actually a required field of the underlying type 🤷♂️
Type 'SerializeObject<UndefinedToOptional<{ __typename?: "Board" | undefined; id: any; }>>' is not assignable to type 'Board'.
Property 'id' is optional in type 'SerializeObject<UndefinedToOptional<{ __typename?: "Board" | undefined; id: any; }>>' but required in type 'Board'.ts(2322)
Anyways, im unenthusiastically working around this by unwrapping the underlying types
type UnwrapLoaderData<T extends LoaderFunction> = Awaited<
ReturnType<Awaited<Awaited<ReturnType<T>>['json']>>
>;
const data = useLoaderData() as UnwrapLoaderData<typeof loader>()
I still don't understand why SerializedObject
is at all required.
I hear people say network serialization, conversion to string and stuff, but it still doesn't make any sense, this is typescript types, why can't it just return the valid JSON type coming from the data layer?
@sslgeorge the loader
function can return only JSON values, remix needs to correctly convert non-JSON types in JSON compatible types, let me show you a simple example:
export const loader = () => {
return {
foo: new Date(),
};
}
export default function App() {
const { foo } = useLoaderData();
return <>{foo.getFullYear()}</>;
}
This code is wrong, because the method getFullYear()
does not exists on the String
object.
But isn't foo
supposed to be a Date
object? Yes, but you can't send objects like Date
, BigInt
or classes over the wire, you need first to convert them in a format that can go over the wire. This format is JSON. The process to convert non json values to json values in this scenario is called network serialization.
This is why the SerializedObject
type is required, is converts non-JSON types to JSON types.
Before addressing comments, I want to clarify that Remix's serialization types are meant to emulate let y = JSON.parse(JSON.stringify(x))
. In other words, for a value x
on the server that is serialized and sent over the network, what value y
will the client receive after it deserializes/parses that value?
Seems simple, but there are tons of little details and nuances to get right. Remix is committed to not mislead you about what the data will actually look like on the client, as mentioned by @nullndr in this comment.
My recommendation is to default to use Remix's built-in serialization types(e.g. useLoaderData<typeof loader>
). If you want stronger guarantees or you want to manually define simpler types than those inferred by Remix, you can use something like zod.
Additionally, if you know the data types your are using are supported by @kiliman 's remix-typedjson that can also be a great choice, but know that remix-typedjson
does not attempt to account for any serialization/deserialization outside of its supported types unless you explicitly register them.
The original issue makes this claim:
// expected
{
domain: string;
status: string;
reason?: string;
}
// actual
{
domain: string;
status: string;
reason?: undefined;
} |
{
domain: string;
status: string;
reason: string;
}
but in fact, Typescript doesn't agree. Here's a TS playground with no Remix code that demonstrates that TS doesn't think the type for reason
should be reduced to reason?: string
As for the behavior highlighted by @miniplus , this is also Typescript standard behavior, not a Remix issue. See this playground with no Remix code that shows the same behavior.
Additionally, if you know the data types your are using are supported by @kiliman 's remix-typedjson that can also be a great choice, but know that remix-typedjson does not attempt to account for any serialization/deserialization outside of its supported types
As of v0.2.0, remix-typedjson
lets you register a custom type handler, so you can handle any type your app uses.
https://github.com/kiliman/remix-typedjson#registercustomtype
Additionally, if you know the data types your are using are supported by @kiliman 's remix-typedjson that can also be a great choice, but know that remix-typedjson does not attempt to account for any serialization/deserialization outside of its supported types
As of v0.2.0,
remix-typedjson
lets you register a custom type handler, so you can handle any type your app uses.https://github.com/kiliman/remix-typedjson#registercustomtype
Nice! 💪
That still requires a manual step to register, which if you are using 3rd party types might be onerous. I think its a good tradeoff and the right design choice for remix-typedjson
, but just mentioning it for others who might want some further nuance on tradeoffs.
I don't quite understand your post in practical terms @pcattori so was how it was working before wrong?
Can you give an example by using Remix only, without 3rd party dependencies on how to use useLoaderData
types correctly?
export let loader = () => {
// the server will serialize this and send it over the network
// basically, `sendPayload(JSON.stringify(x))`
return json(x)
}
export default Component() {
// now, the client will read the data as json
// basically, `JSON.parse(receivePayload())`
let y = useLoaderData<typeof loader>()
}
In 1.19, Remix did a pretty good job of providing types to match JSON.parse(JSON.stringify(x))
's behavior. In 2.0, Remix switched to using type-fest
under-the-hood for those serialization/deserialization types, but that caused some new type errors due to some implementation details of type-fest
's Jsonify
type. In #7605 , Remix sheds the type-fest
types and goes back to owning those types like it did in 1.19, but the new implementation in that PR improves the SerializeFrom
types (which is the type helper that powers useLoaderData<typeof loader>
, useActionData<typeof action>
, etc.).
All reported issues should be fixed by https://github.com/remix-run/remix/pull/7605 . You can try it out with the next nightly release (which will get published in a few hours as of the time writing this comment).
If any new issues emerge, feel free to open a new GitHub issue for that specific case.
🤖 Hello there,
We just published version 2.1.0-pre.0
which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!
Thanks!
🤖 Hello there,
We just published version 2.1.0
which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!
Thanks!
What version of Remix are you using?
1.6.7
Steps to Reproduce
Expected Behavior
the type of
requestedDomains
should beActual Behavior
the type of
requestedDomains
is:See also this discussion where I try some solutions.