Made LoaderFunction, MetaFunction and ActionFunction generic

[mattpocock]

No description provided.

[joeflateau]

@mattpocock I came to the PRs looking for a PR like this one. But seems you need to add yourself to the contributors.yml to "sign the CLA" per https://remix.run/docs/en/v1/pages/contributing (which is worded confusingly, but I see other PRs marked "CLA: signed" when they also add themselves to contributors.yml in the PR)

[ryanflorence]

Action and loader can't be generic. They return new Response(), which itself is not generic. The fact we let you return any should generally be ignored in these conversations (and maybe deprecated since this comes up so often) because it's simply a convenience API where we end up wrapping your value in new Request(JSON.stringify(loaderValue)). The actual return value is semantically still a Response.

People are usually wanting to do this so they can get type inference between loader and useLoaderData(). You have to remember that there's a network request in between what you returned from loader on the server and when you access it in the component after Remix fetched it in the browser. Serialized data over the wire is invisible to static analysis, and therefore invisible to a types system. Type systems can only know the types for code that your code calls. Your code never calls loader(). The browser fetches it. You can't infer the response of a network request.

let res = await fetch("https://example.com/api");
let json = await res.json(); // <-- this can't be generic
// the function lives on the server, it's not called by this code
// so this code can't infer or know anything about it.

new Response() and response.json() aren't generic for a reason: you can't have type safety over the network. Since loader and action return responses, they can't be generic either.

To maybe help with this idea a little more (sorry this comes up a lot so I'm trying to answer for the last time here 😅), let's pretend ...

await readFileToJSON("/some/file.json");

What does that function look like?

async function readFileToJSON(path) {
  let file = await fs.readFile(path);
  return JSON.parse(file);
}

TypeScript can't have any idea what is in that file. That's essentially what useLoaderData is. The value went over the network, our generic in useLoaderData<ThisThing> just let's you feel like you're not potentially lying to yourself by moving the lie to Remix's source code instead of your app, but it's a potential lie all the same. In fact, I could easily be convinced we should deprecate it (though Error Boundaries keep me from being a purist here).

Again, consider instead of server responses and fetches from the browser, we built a framework for writing and reading files to the disk:

// pretend remix loaders actually wrote JSON files to disk
export function writeFileToJSON() {
  return { lol: true }
}

// and pretend useLoaderData read those files:
export default function Thing() {
  let data = useFileData();
}

Because they're in the same file they feel like TypeScript can infer stuff ... and maybe we can ... but that inference is still just a convenient guess, it's not real type safety. If you read from disk, even if you just barely wrote the file to disk in the same module, you can't really know what's in there when you read it again. It's invisible to static analysis. Type safety comes from static analysis.

Generics like the ones in this PR (and our own questionable generic to useLoaderData) are just guesses to get the editor to stop yelling you 🙈

Additionally, you often need to return headers in your response or different status codes and the generics can't participate in that at all:

export function loader() {
  return json(data, {
    status: 401,
    { headers: { "Server-Timing": serverTiming } }
  });
}

Let's say you somehow solve all of this, now you've got the serialization problem. Sending a new Date() the loader is not a Date object after the network serialization.

export function loader() {
  return json({ date: new Date() });
}

let { date } = useLoaderData();
// it's a string!

It's not just dates either, stuff like FaunaDB return objects with custom methods on them from queries. So then you have custom parse/serialization needs, too.

And then after you solve that, now you've got React context and TypeScript's limitations on module APIs in the way.

// typescript doesn't let you type a module (afaik) so we can't infer
// this return value and put it somewhere for ...
export function loader() {
  return something
}

export default function Route() {
  // ... this hook to infer. Now if you solve the TypeScript problem,
  // every call to `useLoaderData` will have different return types
  // depending on the route it was called in. Now mix in calling it
  // from a different file (in an abstraction) and there's just no way
  // to know what each useLoaderData call should infer.
  let data = useLoaderData();
}

Our Recommendation:

Our recommendation for type safety is to use the json generic (where static analysis can actually help) and then make a good guess with the useLoaderData generic (even though it could be a lie if the network falls apart):

export let loader: LoaderFunction = async () => {
  // this will give you type safety that you returned the right
  // shape of data, while still having control over status/headers, etc.
  return json<YourType>(data, {
    status: 401,
    { headers: { "Server-Timing": serverTiming } }
  });
}

export default function RouteComp() {
  // correct 99.999% of the time, but possibly a lie if the network
  // falls apart or your reconfigure your CDN and it botches the
  // response from the loader, but it's okay because Error Boundary will
  // render if this object is the wrong shape
  let data = useLoaderData<YourType>();
}

[joeflateau]

@mattpocock you may find this helpful https://www.npmjs.com/package/remix-superloader#advanced-loaderfunction-type

[colinhacks]

Creator of Zod/tRPC here :) I'd love to see something like this PR happen in Remix, and I'm not sure I understand the reasoning against it, I'm afraid. I'll try to discuss some of the main points in order.

sorry this comes up a lot so I'm trying to answer for the last time here 😅),

lol sorry 😅

People are usually wanting to do this so they can get type inference between loader and useLoaderData() ...

By making direct inference from loader hard, users have to either

a) manually write types for all request payloads and pass into return json<MyType>
b) use the Awaited<ReturnType<typeof getLoaderData>> pattern that's in the docs.

The first option isn't DRY. I'd have to change both MyType and the loader implementation to make a change. This is especially unfortunate if I'm using a typed ORM/query builder, in which case my handwritten MyType declaration is fully redundant. The second is just boilerplatey/verbose.

our generic in useLoaderData just let's you feel like you're not potentially lying to yourself by moving the lie to Remix's source code instead of your app, but it's a potential lie all the same...Serialized data over the wire is invisible to static analysis, and therefore invisible to a types system.

Every TypeScript type is a potential lie! There's nothing special about sending data over the wire that makes these types especially untrustable. Anything can happen to your in-memory data structures too, and TypeScript can't do anything to stop it.

let object = {
  name: "frodo",
};
(object as any).name = 5;

object.name; // => 5

Types only exist as a development-time convenience and are essentially "made up". But it doesn't matter, because the lies are useful and we can generally trust that they'll be true. Part of the appeal of a fullstack TypeScript framework is that its easy to share our made-up types across the client-server divide.

In fact, I could easily be convinced we should deprecate it (though Error Boundaries keep me from being a purist here).

This is another line in the sand that confuses me. You've already provided an escape hatch by making useLoaderData generic (thank you btw!). Most TypeScript users are relying on it. And anyone with a strong predilection for DRYness can use one of the suboptimal approaches I described to infer the return type of loader and pass it into useLoaderData. (I'd argue inference from loader is a best practice; if I was reviewing code that didn't use this approach, I would flag it.)

So isn't this battle already lost, in some sense? You've provided the ingredients needed to make inference possible, but its still unnecessarily difficult. There's already a desire path here, so it really seems like the right move to streamline this DX.

API

there's just no way to know what each useLoaderData call should infer

Yeah the API you're critiquing there is impossible I think.

Here's my proposal from twitter

import {json, makeLoader} from "@remix-run/node";
import {useLoaderData} from "@remix-run/react";

export const loader = makeLoader(async ctx => {
  return json({name: "Colin"});
});

export default function Page() {
  const person = useLoaderData<typeof loader>();
  return <p>Hello {person.name}</p>;
}

Nitty gritty

new Response() and response.json() aren't generic for a reason: you can't have type safety over the network. Since loader and action return responses, they can't be generic either

True, generics are like a virus. You'd have to genericify things all the way down.

Additionally, you often need to return headers in your response or different status codes and the generics can't participate in that at all

The "raw" return type of loader is the union of whatever gets returned along all code paths. You can do post-processing on this type (say, strip out everything with a 4xx status code). As for headers, I certainly wouldn't expect inference on this; just fall back to {[k: string]: string} as a type signature.

Sending a new Date() the loader is not a Date object after the network serialization. It's not just dates either, stuff like FaunaDB return objects with custom methods on them from queries. So then you have custom parse/serialization needs, too.

I'd only expect to be able to return JSON-serializable stuff from a loader personally. I think this is expected and decently easy to explain to users. Next.js users will also be accustomed to this already.

In fact, you could statically prevent users from trying to return non-serializable data with something like this (regardless of whether you change your mind on the generics stuff):

type Scalar = string | number | boolean | null | undefined;
type JSON = Scalar | { [key: string]: JSON } | JSON[]

function json(arg: JSON) { ... }

[ryanflorence]

I think my trouble with all of this is that loaders are sometimes called directly, especially in unit tests. As I understand it, what you're proposing only works when coupled with useLoaderData and Remix's behavior of unwrapping the response.

What happens when I make component loader functions and call them in my route loaders?

// components/user-profile.tsx
export let loader: LoaderFunction = async () => {
  // ...
  return json(...);
}

// routes/users/$userId.tsx
import { UserProfile, userLoader as loader } from "~/user-profile"

export let loader: LoaderFunction = async (ctx) => {
  // What is the type of `res`? It's not unwrapped like in useLoaderData
  let res = await userLoader(ctx);

  // This is a TS error right? The type thinks it's unwrapped?
  let user = await res.json();
  return json(user);
};

export default function User() {
  let user = userLoaderData<typeof loader>();
  // this isn't really the `typeof loader` it's an unwrapped version of
  // a loader because of Remix's specific behavior of unwrapping it
}

Additionally, it's common to unit test loaders by calling them directly, what happens to the types there?

import { loader } from "~/routes/some-route.tsx"

it("...", async () => {
  let request = new Request("...");
  let res = await loader({ request, params: {}, context: {} });
  // this is a TS error right? The type thinks it's unwrapped?
  let data = await res.json();
  expect(data).toEqual("...");
});

My TypeScript skills are very basic and there's a good chance I'm missing something you're saying. Let me know if I'm missing something here.

I do recognize that Remix isn't completely bound to "well, Response isn't generic so loaders can't be either". Remix has specific behaviors we can plan on between loader and useLoaderData, and I'm happy to take advantage of that. Unlike a normal fetch to anywhere, we have the loader function itself available for static analysis.

It seems like what we're really after is a way to get the unwrapped version of a loader for useLoaderData, rather than make loader types expect to be unwrapped always:

let data = useLoaderData(Unwrapped<loader>);

[mattpocock]

@ryanflorence @colinhacks A proposal to solve this:

https://www.loom.com/share/13109c44002e4061b75a91e994d3d8bf

TL;DW:

You can add a generic to Response without actually changing the nature of the Response itself, and use a conditional type to extract that generic and assign it to the data returned from useLoaderData. Minimum changes required within Remix, maximum type safety and utility.

TS playground:

https://www.typescriptlang.org/play?#code/JYWwDg9gTgLgBAbwCIEMYoGIFcB2BjGYCHAQSgHMBnAGjgCtLi4VK4oBTEYADwClGcAXzgAzKBBBwA5AAEOXbgFoouAPQ4IAE3ZSAULuA4Y7KCJR52cAOLscJ4HgBK7SpByV2AHgAqqdAD44dm5jHE1WZ1diD0RBfRFcAiIcegEfPxR-AApNNBQALjhfPIBKQps7KAdItw90vMCEXTgWuFVVOABJEBRyQxQoAE84UDAAG05bdEJiZtaOGCwoFIRhFmYcQd043RhBsEtOnBETABkIFG0oDJ9zy5NAgF4iu6ugkNtwuCyAOj+BqiFFCbADaAF0SnBHoEKvYnC5al5DCcoEUMoEAPxovJwQp2ABuJn0eGi8CwHleJgyUO+kOhiDmLQWSxWa1YwK2OxJ7ngYwub2eWQBlEKGWw+BmpAolDpjUZbHYi2WqWIWSarQ1bQ6AEZCgBhAAWwPIlhgBtN+0s5o4fx+8o15A4aAyhVW8sEJW2+l07TgACYgWE4GaLQc4HgjTgTaxrexbbpuZR4Ll0IUjijKdc8p49gcICI4Hz7lAnnByexMxksiUANw+jqtAB6GP0QA

[ryanflorence]

Interesting. I think I'm after all of this

// note it's `async`
const loader: LoaderFunction = async ({ request }) => {
  return json({ greatData: {} })
}

// nice for route components
const data = useLoaderData<InferLoaderData<typeof loader>>();
data.greatData;

// json returns normal responses, no "remixisms"
const res = json({ beef: "lol"})
const data = await res.json();

// can call loaders directly from other loaders or tests
const res = await loader({ params: {}, context: {}, request: new Request("/fake")})
const data = res.json();

[mattpocock]

All of that is certainly possible. Away from computer watching football, will post a POC tomorrow.

Do you want the res.json to be strongly typed in the returned response when reusing a loader?

[mattpocock]

@ryanflorence

Here you go:

https://www.typescriptlang.org/play?#code/PTAEFpNAZBJAhUBhA8gEQKIUgKBwSwDsAXAUwCcAzAQwGNTQBxUwi-WgJVIGcAHAe0LdSAHgAqaasWoA+UKQAeZQgBNuoFAFt8xEVz6DhAGlAByAFbdBpuQG9QOUE9CXBALlAAKAJSgAvHIACuT82sLiktIyOAC+eMQAnrwMALLUCQBGpMGh+OFicn6gOWGiBaAAPqBi8UkMsISUFND81CoUkdTiLW0UhdU97eTySixqXgB0U9TkAObcHtSECQDaALq+AaBpmdkhpSLMrOTs+gJCokRNwxJSsnIA-NWdoB6sAG4UeJQAroS0xHwghcVkIETuMk8KjuHlu0m8HiObE4PHO+U6dkczhAoFgmmosyIMwSoHwml4ABtSJoWNJAYIsU5yKRiD9yIRQLYYqBqOolglYnhaIZiKAfsJBh07v4vJs7KBmaz2ZzubyectQHEcDjIOBQABVADKGA4yHQWF1eDJAnIovsnQAYn8AUDCABBObqbmUfZmAACzO0CnA5D+wEI-HapjwOIjZFJxFM6gABryEv9kzhhUJRRTWkMZWn-l57MyAI4-HiimIeR3O+nuz1yzmMhUstkc1yETz2WbMqSdDxczXeQXasCEdgMSj8YYhH7x4XkwS07hZkXIULnWkynz+THOUDZ7ii6HSDwNa6S8idESJZL8SigPO9cj9cWka+dHwAblbZ+oCY+1IAc7j-LVxxBYFFQ7dQI3IfEKTbAwLm4EwI1AAAiQN8AUPJNG4TD1xzUBfmLIoi1oWV9xbQ9j1FZl1CKLse1ALJSEoDxMLzClMJHVt6NAADCwAd2oHRkImFjvD-Q8cWcAA9B4x0EsjaAAJkLbh0yovctlsPA5LAWgliPagKSQl8hnUFR8GZAEKRJH1QlAfhiAACwoZ98wodRZ1AMgTzXOiN0Y0TxNzHzyFY3gZmoAihxiExszIJREpMctKxPN5SBE0AuArKtPEw4AaAAa1ITDfBiUcQpI4SKLEiTGKk0Ff1beSnCUwUgA

json only, not new Response

This only ensures that json responses are strongly typed. I imagine it's possible to return a Response directly from a loader. If you wanted that to be strongly typed, you'd need to provide a TypedResponse class from Remix to let them do it, or petition Node to change the types of @types/node.

loader declaration

Note the way I've declared the loader:

const loader = async ({ request }: DataFunctionArgs) => {
    return json({ greatData: {} })
}

I've not declared it like this:

const loader: LoaderFunction = async ({ request }) => {
    return json({ greatData: {} })
}

That's because the LoaderFunction in this position declares what the return type is, so you can't do any clever inference on the top of that. I'm comfortable with this constraint - it's the same one present in InferGetServerSidePropsType from next - but it would need to be documented.

[MichaelDeBoey]

@mattpocock Wouldn't it be possible to somehow still have a LoaderFunction type, so we could enforce the return type to always be of type Request for instance?

[mattpocock]

@MichaelDeBoey Sure, but you wouldn't be able to do any inference here based on the return type, you'd need to provide it up front:

const loader: LoaderFunction<{ id: string }> = () => {
  // Must return a `Response` with `json({ id: '' })`
}

Which is cool in itself, but perhaps a separate discussion.

[MichaelDeBoey]

@mattpocock So there's no solution to both infer the type + make it strict? 🤔
I thought there was, but can't remember to do it, hence my question 🙈

[mattpocock]

If you're using the InferLoaderType discussed above, that will revert to never (which will throw a type error on usage) if you don't return the correct thing from your loader.

TS is proposing a satisfies keyword to do what you're describing.

microsoft/TypeScript#47920

[colinhacks]

@ryanflorence thanks for the response. Sorry for the delay here - been at a conference all week.

---

As @mattpocock mentioned, making Response and json generic would provide a solid loader inference DX. Because of how interface merging works, I believe its as simple as including the following code in a .d.ts file anywhere in the build version of remix-server-runtime. These declarations will be merged with the globals ones from lib.dom.d.ts.

// declarations.d.ts
interface Body<T> {
  readonly body: ReadableStream<Uint8Array> | null;
  readonly bodyUsed: boolean;
  arrayBuffer(): Promise<ArrayBuffer>;
  blob(): Promise<Blob>;
  formData(): Promise<FormData>;
  json(): Promise<T>;
  text(): Promise<string>;
}

interface Response<T> extends Body<T> {
  readonly headers: Headers;
  readonly ok: boolean;
  readonly redirected: boolean;
  readonly status: number;
  readonly statusText: string;
  readonly type: ResponseType;
  readonly url: string;
  clone(): Response<T>;
}

Then you can make json generic in remix-server-runtime/responses.ts:

// make `json` generic
export declare type JsonFunction = <T>(
  data: T,
  init?: number | ResponseInit
) => Response<T>;

With this change, the json function now returns a typed, generic Response object. This can either be inferred (the static type is inferred from the implementation) or "enforced" (the static type is passed explicitly, and TypeScript will guarantee that the implementation matches).

// inferred
json({hello: 'world'}); // Response<{hello: string}>;

// enforced
json<string>(5); // TypeError: number is not assignable to type `string`

You can call loaders from other loaders, and they'll return Response<T> (unwrapped).

// route.ts
export async function loader(args: DataFunctionArgs) {
  return json({hello: 'world'});
}

// outerroute.ts
import {loader} from './route';

export async function loader(args: DataFunctionArgs) {
  // unwrapping Request
  const res = await userLoader(args);
  const user = await res.json();
  // {hello: string}

  return json(user);

  // directly returning result of another loader
  return userLoader(ctx);
}

As Matt suggested, Remix could also provide a utility called InferLoaderType to extract the datatype returned by the loader.

type ExtractResponseType<T> = T extends Response<infer U> ? U : T;
type InferLoaderType = ExtractResponseType<Awaited<ReturnType<typeof loader>>>;

This is the bare minimum that I'd recommend in terms of paving the way for type inference, but I still think there room for improvement in the DX. Most of this was in my original API proposal but I didn't justify those decisions explicitly, so here goes.

Alias for DataFunctionArgs

TypeScript users will be importing/using the DataFunctionArgs interface quite a bit more frequently once type inference from loaders is possible. I don't think this name is particularly descriptive or easy to remember, so I'd recommend aliasing/renaming it to LoaderArgs or LoaderInput.

Accept loader type in useLoaderData

I think the InferLoaderType approach is a bit clunky. I always thought the same about Next.js's InferGetServerSidePropsType too. It's a long name that you need to remember, it requires an extra import, and it just looks clunky:

import type {InferLoaderType} from "@remix-run/node";

export const loader = ()=>{ ... }

type LoaderData = InferLoaderType<typeof loader>;

export default App(){
  const data = useLoaderData<LoaderData>();
  return <div>sup</div>
}

IMO it makes more sense for useLoaderData to be able to accept a loader type as its generic parameter:

export const loader = ()=>{ ... }

export default App(){
  const data = useLoaderData<typeof loader>();
  return <div>sup</div>
}

This avoids the need to import a special alias or declare an intermediate LoaderData type and looks clean.

Currently useLoaderData expects to receive the return type of the loader's response ({hello: string}). I'm proposing that it be extended to also accept a loader type itself ((args: DataFunctionArgs)=>Response<{hello: string}>). This can be done in a backwards compatible way with conditional types.

function useLoaderData<T>(): T extends Function ? InferLoaderType<T> : T {
  // ...
}

Pass loader into useLoaderData?

It was suggested on Twitter that useLoaderData could accept the loader itself as an input. It would avoid the need for any type annotation whatsoever, which would be nice.

const loader = ...

export default App(){
  const data = useLoaderData(loader);
  return <div>sup</div>
}

Though I assume this is impossible for bundling reasons.

makeLoader

I think that providing a makeLoader utility provides the best DX and maybe could solve some other problems. Here's the proposed API:

export const loader = makeLoader((args) => {
  args.request; // args is automatically typed
  return {hello: 'world'};
});

This API is nice for a lot of reasons.

1. No input annotation

You don't need to add a type annotation to the function input. The args variable is automatically typed.

2. Support "type enforcement"

If you don't pass an explicit type hint to makeLoader, it will infer your return type. However, in some cases it's useful to have things work the other way, where the user specifies a "goal" return type and TypeScript makes sure the implementation agrees.

export const loader = makeLoader<{hello: string}>((args) => {
  return {
    hello: 5,
    //     ^ TypeError: number not assignable to type string
  };
});

So there's no solution to both infer the type + make it strict

This is actually possible with makeLoader. If you pass a type T into makeLoader<T>(), TypeScript can both enforce that the return type extends T and infer the exact return type simultaneously.

function makeLoader<T, LoaderFunc extends (arg: DataFunctionArgs)=>T>(arg: LoaderFunc){
  ...
};

const loader = makeLoader<string | number>(args => {
  return json('sup')
});
typeof loader;
// (args: DataFunctionArgs)=>Response<"sup">

3. No json necessary

Currently users can already return plain data from loaders. Internally, Remix autowraps the result in a Response as needed.

return isResponse(result) ? result : json(result);

But if this loader is called from a test or another loader, it returns the raw data directly, not a response. I agree that this is a bit unsatisfying, since it breaks the "loaders return Responses" mental model.

A makeLoader helper would solve this. It can return a wrapped version of the function you pass into it and automatically do some post-processing on the returned value.

async function makeLoader<T>(
  loader: (args: DataFunctionArgs) => T
): (args: DataFunctionArgs) => T extends Response ? T : Response<T> {
  const result = await loader();
  return isResponse(result) ? result : json(result);
}

const loader = makeLoader((args) => {
  return {hello: 'world'};
});
type loader = typeof loader;
// (args: DataContextArgs)=>Response<{ hello: string }>

This way, users don't need to wrap the result in json(...) and the loader function itself always returns a Response object. Best of all worlds.

---

Aight, that's all I got for now. I'll be at Remix Conf next week so we could discuss this further in person!

[mattpocock]

I'll also be at Remix Conf! We should have a summit.

[kiliman]

I think that providing a makeLoader utility provides the best DX and maybe could solve some other problems. Here's the proposed API:

Unfortunately your makeLoader hof will break tree-shaking since it's a side-effect.

[colinhacks]

Unfortunately your makeLoader hof will break tree-shaking since it's a side-effect

True. But since this would be a first-party HOF, we could consider using the emptyModulesPlugin to always shake @remix-run/node (and cloudflare, etc) as if they were *.server.ts files.

[itsMapleLeaf]

I think that providing a makeLoader utility provides the best DX and maybe could solve some other problems. Here's the proposed API:

Unfortunately your makeLoader hof will break tree-shaking since it's a side-effect.

Can this be fixed with a //@__PURE__ comment?

[mattpocock]

I think that the Response with generic approach described above by myself and Colin is the most idiomatic. I think that having makeLoader muddies the waters on what gets omitted and what doesn't, so I think a type-only solution is preferable.

[colinhacks]

I think that the Response with generic approach described above by myself and Colin is the most idiomatic. I think that having makeLoader muddies the waters on what gets omitted and what doesn't, so I think a type-only solution is preferable.

I agree, I shouldn’t have coupled makeLoader into this discussion. The proposals should be considered independently.

Note that a straight genericification of Response appears to be impossible - I tried every version in my power but it’s not possible to override json without incorrectly implementing the Body interface. My linked PR implements an alternative that works though.

[kentcdodds]

@colinhacks do you mind writing out the idea you shared with me at Remix Conf? I'm mostly interested in what the API looks like. We can talk about implementation later. Here are the things that are important to me:

1. No higher-order functions (so makeLoader is definitely a no).
2. Don't allow non-serializable types (like Date).

I think those are the most important bits. If we can handle that then I feel much better about this.

[mattpocock]

From my understanding, serializable types should be allowed - but they should be deserialized at the type level so that Date becomes string. This reflects Remix's behaviour.

[kiliman]

@mattpocock I'm not sure what that's supposed to look like. I believe @kentcdodds wants to ensure that

return json({ date: new Date() }) // this should complain, as type is not compatible with useLoaderData

const { date } = useLoaderData<typeof loader>()
console.log(date.toLocaleDateString()) // TS is lying as date is really a string, not Date type

If json helper restricted object to JSON values only, then this will complain in IDE, not at runtime.

[itsMapleLeaf]

@kiliman This is what I think he means:

type Serialized<T> = {/* ts magic */}

type Input = { date: Date }
type Output = Serialized<Input> // { date: string }

This would allow passing other certain types where their serialized forms are known, and they would show up as that type in the useLoaderData generic.

---

Although I don't think it's a huge ask for users to manually do .toISOString() or whatever their intent was

[kentcdodds]

From my understanding, serializable types should be allowed - but they should be deserialized at the type level so that Date becomes string. This reflects Remix's behaviour.

Agreed. Here's my adjusted requirements:

1. No higher-order functions (so makeLoader is definitely a no).
2. useLoaderData should not return anything that JSON.parse can't produce.

[kiliman]

Ah, ok. Although I find it odd that the return type of the loader wouldn't match the return type of useLoaderData, since that seems to be what everyone is asking for.

[joeflateau]

@kiliman the types would have to be a lie because when the date is sent over fetch it’s serialized to a string and remix doesn’t convert those back to date objects. That’s why I wrote https://www.npmjs.com/package/remix-superloader which basically does everything folks in this thread are asking for BUT adds a bit of overhead.

Edit: just reread the thread and I see you already know that. I’ll see myself out

[mattpocock]

Ah, ok. Although I find it odd that the return type of the loader wouldn't match the return type of useLoaderData, since that seems to be what everyone is asking for.

@kiliman I agree, but it's important for the types not to lie to users.

[kiliman]

Yes and that's the reason I suggest restricting json parameter be JSON values so there would be no chance to lie. If they want to send a date they should convert it to a string explicitly instead of relying on JSON.stringify.

[mattpocock]

@kiliman I disagree, the types should reflect what is actually allowed by the library. If the library threw a runtime error (like Next does in this situation) then your solution would be appropriate.

[edwin177]

The first time I ran into the Date issue, I was surprised Remix didn't warn me that I can't return Dates.

Making people explicitly convert to strings before returning from loaders has my vote.

[mattpocock]

@edwin177 Could you raise a separate issue about that?

[kiliman]

I’ve been around Remix long enough to know that everyone that’s been asking for inferred types are going to be highly confused and disappointed that the types will be different going in and coming out.

I’m surprised you’re considering this a separate issue.

I prefer explicit types so not an issue for me.

The loader API makes it seem like you put data in on one end and get the same data out at the other. The fact that they are 2 separate functions is kind of immaterial to those asking for implicit types.

The use of JSON is explicit since that’s the name of the function, so I don’t think it’s a stretch to ensure only JSON values go in since that’s what comes out.

[mattpocock]

@kiliman Since that's a change to existing behaviour, could you put that in a separate issue? I want to keep this issue only about adding TS inference to the current behaviour.

[kentcdodds]

@kiliman, I disagree with you on this. I prefer my types to describe what is possible. The fact is that it's possible for me to call json with a Date, but it's not possible to expect a Date out of useLoaderData. So our types should enforce that.

In practice, people will see that their date type in their loader is a string and not a date and they'll probably be a bit confused about that. This is an issue we can deal with in the future. But the fact is this is the reality right now with or without typescript so I think our first approach at improving things is to just get TypeScript to help people with the reality of the situation and reduce the boilerplate necessary to get there.

So again, here are my requirements of whatever we do:

1. No higher-order functions (so makeLoader is definitely a no).
2. useLoaderData should not return anything that JSON.parse can't produce.

[itsMapleLeaf]

I'm still leaning on the side of disallowing types that "transform" on the other end, and here's why. I feel like relying on the default Date serialization is going to be a mistake in more cases than not, and similar for any other non-primitive.

Also because there's apparently a whole rabbit hole when it comes to stringify. So for me it's a practical thing. I think it makes more practical sense to make the user specify their intent (how it should turn into a string, not implicitly), and it's easier that way in the typings too.

[kentcdodds]

I'm not sure I understand what you're suggesting, @itsMapleLeaf. All I'm saying is that the types should allow you to do what can be done. Currently the only serialization/deserialization that happens within Remix is basically JSON.stringify and JSON.parse. So here's the adjusted requirements that will hopefully make it more clear what I'm looking for:

1. No module side-effects (so higher-order functions like makeLoader is definitely a no).
2. json should allow anything that JSON.stringify allows.
3. useLoaderData should not return anything that JSON.parse can't return.

In practice someone will notice that the Date they sent to json will appear as a string in useLoaderData and they'll figure out what's going on and either format it manually on the server, or parse it to a date on the client. The point is that this becomes an obvious issue they have to deal with on their end (currently it's a non-obvious issue which they still have to deal with on their end). If they don't want to deal with it, using @joeflateau's package is a great solution and maybe one day we'll handle something like that for you or at least improve the DX of that.

But my whole objective here is to simply make the over-the-network typing experience more true-to-what's-happening and I welcome a proposal that handles the requirements I've laid out :)

(Also, thank you everyone who's contributed to this conversation, I think we'll come up with something really great out of this).

[kiliman]

Again I’m not even arguing for a feature that I’m going to use. I’m just saying that what you guys are proposing is NOT what everyone that has asked for type inference is asking for.

I’ve answered this question hundreds of times on Discord, Twitter and here on GitHub. It’s probably the #2 question, where the first is “can parent loaders pass data to child loaders?”

As always it’s a documentation/training issue but I can guarantee you that it will trip up pretty much everyone as it doesn’t match their expectation.

Anyway it’s not a hill for me to die on.

[kentcdodds]

And all I'm saying is this is not what we're trying to deal with in this conversation. Right now we're just trying to make the typing better than it is already. This is why I think Matt suggested making a separate GitHub discussion, because that's another topic. All I want to do is take the existing behavior and model the TypeScript types after that and reduce boilerplate while we're at it.

[colinhacks]

A few points of discussion relating to the technical approach. All of this is reflected in my PR: #3276

1. Restrict json input

The goal is to restricting the input of json to only values that can be JSON.stringified. That's fairly broad, since JS will allow many non-JSON primitives, per https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify. Also any object can provide a custom .toJSON method that will be used to convert the object into a serializable structure. Here's what I came up with:

type SerializablePrimitives =
  | string
  | number
  | boolean
  | null
  | { toJSON(): any }
  | undefined
  | Function
  | symbol;
type Serializable =
  | SerializablePrimitives
  | { [key: string | number | symbol]: Serializable }
  | Serializable[];

2. useLoaderData output

To guarentee that the result of useLoaderData only returns types that can be produced by JSON.parse will require a recursive mapped type. This will convert any serializable-but-not-parsable type to it's post-serialization type, for instance, convert all instances of Date to string. It must also read the ReturnType of toJSON method for objects on which it is defined. Refer to my implementation here: https://github.com/remix-run/remix/pull/3276/files#diff-594d4b6ed5bd4abff7e9319566b5505b6a85753e665ff4d2113e888384a98377R1309

3. TypedResponse

The return type of Response is implemented with a generic TypedResponse type:

type TypedResponse<T> = Response & { json(): Promise<T> }

I initially tried to extend the existing lib.dom.d.ts Response interface like so:

declare global {
  interface Response<T = any> {
    json(): Promise<T>
  }
}

This fails compilation with an "Incorrectly implements interface" error (see CI failure), since Response implements Body which contains the following declaration:

interface Body {
  // ...
  json(): any;
}

To the best of my knowledge it's not possible to "override" this via interface merging, since interface merging can only be used to add additional overrides, not make the return type more specific. You can see a minimal reproduction of the issue here:

export interface Test {
  json(): any;
}
export interface Test<T = any> {
  json(): T;
}

const infer = <T>(arg: T): Test<T> => {
  return arg as any;
};
const arg = infer("asdf");
const out = arg.json(); // any

4. useActionData

I extended the PR to include useActionData as suggested by @itsMapleLeaf. Naturally both hooks should be consistent w.r.t. typing/inference.

[colinhacks]

Also: if anyone is looking to stress test the type conversion logic, here's my pass at it. This is a type-level implementation of JavaScript's built-in JSON.stringify behavior: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#description

type JsonPrimitives = string | number | boolean | null;
type NonJsonPrimitives = undefined | Function | symbol;

type SerializeType<T> = T extends JsonPrimitives
  ? T
  : T extends undefined
  ? undefined
  : T extends { toJSON(): infer U }
  ? U
  : T extends []
  ? []
  : T extends [any, ...any[]]
  ? {
      [k in keyof T]: T[k] extends NonJsonPrimitives
        ? null
        : SerializeType<T[k]>;
    }
  : T extends (infer U)[]
  ? SerializeType<U>[]
  : {
      [k in keyof T as T[k] extends NonJsonPrimitives
        ? never
        : k]: SerializeType<T[k]>;
    };

[kentcdodds]

@colinhacks, thank you so much for taking the time to implement these requirements. I'm going to move conversation over to your PR.