Add generic type safety for useLocation state data

[boylett]

Currently, useLocation is non-generic, and the state data returned by it is simply typed as any.

The Location type already supports state typing, so it should be trivial to implement a generic for useLocation.

// react-router/lib/hooks.tsx - 133

-export function useLocation(): Location {
+export function useLocation<State = any>(): Location<State> {

[A-EbrahimRSA]

I think this is a potentially great value add. This will mean you don't need to do any location.state as Type.
I don't think this change will cause any adverse effects since it should primarily be a change to types, which will primarily improve DX for implementers 😄

[rossipedia]

I'm not sure I see any benefit when moving from:

const { state } = useLocation() as Location<State>;

versus:

const { state } = useLocation<State>();

Is it just about removing some extra keystrokes? They'd be functionally identical.

The main thing I worry about would be the implication that there's some kind of guarantee that useLocation<T>() would in fact return a Location<T>, since there's nothing of the kind (nor can there be without some kind of runtime validation of location.state).

I think something with stronger guarantees could be useful here:

function useLocation<T = unknown>(validateState?: (state: unknown) => T): T;

or possibly even leveraging Standard Schema to allow for something like:

import { z } from 'zod';

const StateSchema = z.object({ id: z.number() })
	.or(z.undefined());

const { state } = useLocation(StateSchema);
//      ^? { id: number } | undefined

[A-EbrahimRSA]

I agree that as per your examples it is ultimately syntactic sugar . However, my personal preference is to try and avoid as type assertions as much as possible. The reason being that I've seen in practice it often gets misused and reduces some of the pains of Typescript but also introduces some issue mainly being as you say it doesn't guarentee any type considerations at runtime. However, I am aware that it can be a useful in some scenarios.

I also understand that in order to fully guarentee the state without resorting to something at runtime would require typing all cases where we set the location.state for e.g. in things like <Link/> which opens up another can of worms.

I'd also maybe challenge the idea that any guarentee is given by useLocation<T>(). From my perspective it gives the same guarentee that useState<T>() does. For e.g. the following is valid React code (assuming youre allowing any's)

const [counter, setCounter] = useState<number>(0);

const exampleOutput = 10 * counter;

// A placeholder for some external logic such as <Link/> updating the location.state, which we are unaware of
setCounter(false as any);

The setCounter(false as any) seems unintuitive, but its an example to show that React doesn't stop you from doing anything bad here in the setCounter call. Nor does it provide any guarentees when using the counter value that it is a valid number. It just provides the type inference and you as the implementer can do bad things if you're not following proper conventions.

[sergiodxa]

There's a difference between useState and useLocation, the first one is a getter and setter, and the setter will only accept T as value to set, so the getter will always return T

But useLocation is only a getter, you set it somewhere else (in a Link, Form, or navigate), so there's no way to enforce the setter will use the correct type the getter expects

At the end, useLocation will do as T internally anyway, so the only benefit of supporting this is that you won't see the type casting, but it will still be there.

[A-EbrahimRSA]

I'm not sure you're understanding my meaning

In my above example I have setCounter(false as any). This is a contrived example but I have added this to show that useState does not provide any runtime guarentees as to the contents of the getter. It will try and prevent you from doing bad things by typing the setter as well. The setCounter(false as any) can be interpreted as some external <Link/> setter which is passing an arbitratry value to the location.state.

This means that useState provides no runtime guarentees, these are just compile time inferences. I am suggesting the same for useLocation. I understand that having the compile time inferences on the setter as well as the getter is what provides the good type safety but in this situation, as we both agree on, we can't neccessarily assume that without a runtime type checker for location.state.

Nevertheless, this is a small QOL issue in the sea of everything else 👍

[rossipedia]

If I have this is one route:

navigate("/foo", { state: "bar" });

and then in the route that handles /foo I have this:

const { state } = useLocation<number>();

Then typescript is going to tell me that I have a number, when in fact the value is a string. That's not a very strong type guarantee, and exactly the kind of "casting with extra steps" that RR has been moving away from in general (eg: useLoaderData<T>())

I can see the argument about it being a QoL improvement, in much the same way useLoaderData<T>() was over useLoaderData() as T, but I don't think that clears the bar for type-safety guarantees in the way that React Router is going these days.

There might be an opportunity for a proposal here for some kind of LocationState type export from route modules that could be leveraged in Framework Mode, which could then be used somehow with generated types to ensure that when navigating to a particular route you're using the state type that route specifically accepts. Ultimately, I think I'd like to get @pcattori's take on this, he has a lot more context on RR's vision for type-safety.

However, my personal preference is to try and avoid as type assertions as much as possible

I'm curious about your reasoning here. Type assertions are a tool that have their uses.

[A-EbrahimRSA]

Yes, we're in agreement that without applying strong typing to both setters & getters in this context then we're ultimately applying a classification for some unknown object and hoping it fits, which has the potential to break things.

I can sortof see your suggestion for a type export, but I worry that may have the potential to create more complexity/confusion. So I'd prefer this wasn't implemented rather than implemented in a complicated/unintuitive way. But maybe your suggestion is simple enough to use.

I'm curious about your reasoning here. Type assertions are a tool that have their uses.

Exactly as you've outlined at the first response, type assertions do not prevent any runtime issues. So, in general where possible I tend to enjoy the challenge that comes with Typescript generics and things like Conditional Generics. Its provided me some really great compile time checking especially with some fancier behaviour. However, as you point out they are a tool and have their uses, so I do use them in cases where the Type system reaches its ceiling / I've lost the battle with the type system.

Once again this is a minor QoL change, so I'm happy if we're going to be more conservative and not add this change. But glad we could discuss and challenge the idea 😄