[v6] concerning optional URL params

[Jazzmanpw]

Why does useParams returns a Partial object? On the one hand, it's a guard from putting a hook under a route that doesn't have this parameter. On the other hand, it forces us to write some guards with undefined checks that should never run. Maybe I missed something in the docs, but it's not clear why such a decision was made. What's the motivation for this change?

[chaance]

@Jazzmanpw You answered your own question, I believe 😄

it's a guard from putting a hook under a route that doesn't have this parameter

Hooks can be called from anywhere, and the library can't be smart enough to know for sure that your params will be defined. We chose to optimize for real type safety here, and that means acknowledging that a parameter might be undefined.

it forces us to write some guards with undefined checks that should never run

Not exactly. If you are certain your parameter will be defined, you can use type casting or the non-null assertion operator to avoid a runtime check.

let { param } = useParams() as { param: "definitely defined" }
// or
let { param } = useParams<{ param: "definitely defined" }>();
let str = param!.toLowerCase();

[ericbf]

Doesn’t that kind of defeat the point of the type parameter, to a degree? Generally speaking, passing type parameters is preferred over type casting, as type casting implies that the type is being changed. That’s not what’s going on here. The hook mutates the type parameter you pass, forcing you to use a type casting if you want your actual type to be utilized.

If the programmer is expressly saying that the type should be { param: "definitely defined" }, then that should be the type returned from the hook. If the hook is called somewhere where that value could be undefined, then the programmer can easily pass the type parameter as { param?: "maybe undefined" }. But, for example, in your own app where you only serve a component at a route where that param is always defined, the type should be exactly as expressed.

Should the library decide for the user that it’s being used somewhere that may return undefined?

In the same way that’s it’s not smart enough to know for sure that the parameter is defined, it’s also not smart enough to know for sure whether the parameter may be undefined.

IMO, it should just use the type that is passed in the type parameter as is, if such a type parameter is passed.

[Jazzmanpw]

@ericbf initially I thought that you're talking about two equally fair alternatives. It's not smart enough for both, so authors have to choose. And we can't say that one option is better than another. But after a little bit of thinking I came up with a native TS example that is similar to this situation

I.e. you have an array of entities with an ID and construct an object where you can get these objects by id. It will be

const entityMap: Record<number, {id: number; name: string}> = {
  1: {id: 1, name: 'First',},
  2: {id: 2, name: 'Second',},
};

There's definitely a chance that someone will try to entityMap[3] and will get undefined at the runtime. But TS thinks that we are smart enough to know that we expect a value for every key we intend to pass here. And if we're not sure, we'll do it Partial<Record<...>>, and TS is ok with that either

Because TS is not about truth of any extend, it's about "well, we can't really describe this JS entity, but we'll try...". It's kind of a point for using the type argument as-is. But one can also find a point for the current implementation, so I'm not sure if this discussion is worth it

[ericbf]

I appreciate the discussion, though some may consider it not worthwhile 😄.

For such a case as you described, TS introduced the option "noUncheckedIndexedAccess", which will always include undefined in the type returned from an indexed access. When the compiler can’t be sure that an index is defined, it makes sense to assume it may be undefined.

That’s a little different from the situation where you purposely describe the type though. To use your example, if you were to type entityMap expressly, TS doesn’t assume that it might be undefined, though technically speaking, it could be undefined at runtime.

const entityMap: {
	1: { id: 1; name: "First" }
	2: { id: 2; name: "Second" }
} = {
	1: { id: 1, name: "First" }
} as any

const three = entityMap[3] // Error
const two = entityMap[2] // Undefined is not included in type

two.name = "Second" // Runtime error

My primary point is, the compiler will allow you to make assumptions if you choose to do so, even if they may prove false in situations that the compiler cannot account for. It doesn’t assume that, because [2] might be undefined at runtime, we have to include undefined in its type.

The behavior of useParams feels off because instead of using the type that is passed to it, it tries to be smart and mutates it, resulting in unexpected behavior, whereas the compiler does not do that (unless you enable the noUncheckedIndexedAccess option, in the case of the Record).

Not sure how settled this already is, or whether this should really be a new issue, but in my humble opinion, it would make a lot more sense for the hook to merely use the type as passed and allow the user to decide whether it could be optional or not, by either passing Partial<...> if it might be used in a context where the params are not present, or not passing Partial<...> otherwise.