Loader data generics

[siric]

What is the new or updated feature that you are suggesting?

Instead of returning unknown by default and then typecasting (as seen in one of the examples), prefer generics:

Bad

const data = useLoaderData() as Data

Good

const data = useLoaderData<Data>()

The same should be applied to useRouteLoaderData and useActionData and it should account for defer as well (see #9641 and #9642).

Why should this feature be included?

Generics over typecasting.

[devpeerapong]

This will be decided in 6.5

[brophdawg11]

I'm going to convert this to a discussion so it can go through our new Open Development process. Please upvote the new Proposal if you'd like to see this considered!

[brophdawg11]

Related PR to ensure we have typings on json and redirect: #9669

[brophdawg11]

Related PR: #9670

[brophdawg11]

Related Issue and PR for defer types as well: #9641 #9642

[lennerd]

@brophdawg11 thanks for linking the PRs. I'm a bit confused though. Thesse PRs are all either closed or merged.

Maybe I'm wrong but there is still no support for generics in useLoaderData or useActionData, right? What is the state of the discussion here?

[brophdawg11]

Yeah those are closed PRs we may refer to if we implement this. There are no generics at the moment - we may still do them but are also considering some alternative ways to provide stronger type safety/inference. See #9687 and #9768.

[jamesopstad]

I've been working on a typesafe wrapper library for React Router (https://github.com/jamesopstad/typesafe-router) and would like to build in better support for the various data types that can be returned by loaders. Currently, defer, redirect and json are not generic so type inference is lost when using these functions (this isn't strictly necessary for redirect). If we take defer as an example then the data parameter would be generic in the defer function and DeferredData class. Ideally, defer, redirect and json would return branded types so that they can be differentiated and the correct type can be extracted for each.

I'd be happy to put together a PR if you are in favour of this approach. It would also benefit regular React Router users.

[danielnixon]

Bad

const data = useLoaderData() as Data

Good

const data = useLoaderData<Data>()

Doesn't this just hide the as type assertion in the implementation of useLoaderData? There's still the same number of type assertions overall (and the level of type safety hasn't increased or decreased), it's just that we've disguised the type assertion.

As they say in https://github.com/DefinitelyTyped/DefinitelyTyped#common-mistakes

getMeAT(): T: If a type parameter does not appear in the types of any parameters, you don't really have a generic function, you just have a disguised type assertion. Prefer to use a real type assertion, e.g. getMeAT() as number. Example where a type parameter is acceptable: function id(value: T): T;. Example where it is not acceptable: function parseJson(json: string): T;. Exception: new Map<string, number>() is OK.

Overall you might even argue that we're worse off, because disguising the type assertion in this way renders it invisible from tooling such as https://github.com/plantain-00/type-coverage that would otherwise be able to warn us about the as in the AST.

[brophdawg11]

Doesn't this just hide the as type assertion

Yes, absolutely. We argued this repeatedly over in Remix and eventually gave in. We don't love it though and now that we've got a better grasp on our patterns (and TS in general!) we're focusing on better ways to do this in future APIs. That's part of why we've avoided carrying this over from Remix to React Router.

[danielnixon]

Thanks for holding the line. Using the same approach as useContext is perhaps the answer. It's not completely type-unsafe and it comes from React itself, so...

[JoepKockelkorn]

@brophdawg11 Do you have any update on this? I've looked at the Roadmap but it doesn't seem completely up-to-date and can't find this issue on it.

[silvenon]

I was quite surprised to not find useLoaderData<typeof loader>() in React Router. Are there plans to port it over from Remix?

[tykhan]

Generics should also work with useAsyncValue()

[joernroeder]

any updates on this? seems like a strange gap in the toolkit to have…