refactor(react-router): never return type for notFound

[RobinDaugherty]

Currently, when you call notFound({ throw: true }) Typescript does not identify it as never returning.

A discussion of this issue: #2168

Here is an example:

export const Route = createFileRoute('/$tenantId')({
  loader: async ({ context, params: { tenantId } }) => {
    const result = await context.getTenantInfo()
    if (!result) {
      throw new Error('Got no result')
    }
    if (result.error) {
      throw result.error
    }
    if (!result.data.tenant) {
      notFound({ throw: true })
    }
    return result.data.tenant
  },
  component: TenantLayout,
})

function TenantLayout() {
  const tenant = Route.useLoaderData() // tenant is possibly null or undefined
}

Typescript infers the return type of the loader function in this case to be either a data structure or undefined or null. Since the type being returned is a subset of another object, defining an explicit type is not very efficient.

Fortunately Typescript can be given a hint that the notFound function will never return, and in that case will infer a non-nullable return type.

Because the notFound function only raises an exception when the throws option is true, this is expressed as an overload of the function. The overloads tell Typescript the concrete return types depending on options passed, returning never when throws is true, and returning a NotFoundError in other cases.

With this in place, the result of Route.useLoaderData() is a non-nullable data structure.

(The throws: true implementation is desired when using eslint, since it will complain about throwing a NotFoundError because it does not inherit from Error. By asking the function to throw instead, we can bypass the eslint complaint. The complaint is otherwise desirable, but the internal implementation of the router throws and catches these specific NotFoundError objects, and our application should not catch one, so the eslint complaint is not applicable.)

[schiller-manuel]

returning notFound is deprecated, throwing is the way to use it.
same goes for throw: true, this was just addded because of at that time the loader type inference could not handle functions that would only throw and never return any "real" value.
we need to mark this as deprecated in code and documentation.

cc @chorobin

[lo1tuma]

returning notFound is deprecated, throwing is the way to use it.

😢 oh no that is really sad. Throwing non-exceptions is IMHO a big code smell. There is not much difference to a goto and it impossible to type the thrown value properly, so it always requires runtime validation when catching those errors. I know a lot of other frameworks do it in the same way, but I don’t think that makes it better.

[schiller-manuel]

it impossible to type the thrown value properly, so it always requires runtime validation when catching those errors.

you as a library user do not need to handle those exceptions, it's handled by router. no need to add any "runtime validation" on your side.

[lo1tuma]

you as a library user do not need to handle those exceptions

Well, that is not quite true. As a user of the library I’m in charge of the code that throws this weird value. Since I want to unit test the code that I’m in control of I need to somehow catch this value and verify that it is the expected value.