remix-run / remix

Build Better Websites. Create modern, resilient user experiences with web fundamentals.
https://remix.run
MIT License
29.82k stars 2.51k forks source link

`UseDataFunctionReturn<T>` omits properties of `T` defined as `never` #3747

Closed m5r closed 2 years ago

m5r commented 2 years ago

What version of Remix are you using?

1.6.5

Steps to Reproduce

I've put together a minimal reproduction case available here: https://codesandbox.io/s/heuristic-shockley-t1ks84?file=%2Fapp%2Froutes%2Findex.tsx

Expected Behavior

The useActionData<ActionData>() hook should contain all of the properties typed in ActionData

Actual Behavior

The useActionData<ActionData>() hook doesn't have the properties typed as never in ActionData

kentcdodds commented 2 years ago

Probably related: https://discord.com/channels/770287896669978684/771068344320786452/997372983008047154

rossipedia commented 2 years ago

This is how TypeScript works. never is used to indicate that this thing doesn't exist, and is specifically used to omit properties when using mapped types.

In your example, you're not actually narrowing the types by just checking for the existence of the property, but you can use the in operator to perform that kind of check:

{(actionData && 'one' in actionData) ? (
  <h3>You clicked button number {actionData.one}</h3>
) : (actionData && 'two' in actionData) ? (
  <h3>You clicked button number {actionData.two}</h3>
) : null}
kiliman commented 2 years ago

I think a better way to do this is to use discriminated union types.

type ActionData =
  | {
      button: 'one'
      one: 1
    }
  | {
      button: 'two'
      two: 2
    }
export async function action({ request }: ActionArgs) {
  const formData = await request.formData()
  if (formData.get('_type') === 'one') {
    return json<ActionData>({ button: 'one', one: 1 })
  } else {
    return json<ActionData>({ button: 'two', two: 2 })
  }
}
export default function Index() {
  const actionData = useActionData<typeof action>()
  return (
      {actionData?.button === 'one' ? (
        <h3>You clicked button number {actionData.one}</h3>
      ) : actionData?.button === 'two' ? (
        <h3>You clicked button number {actionData.two}</h3>
      ) : null}

https://codesandbox.io/s/remix-union-types-tsml0k?file=/app/routes/index.tsx

kwiat1990 commented 2 years ago

@kiliman I have used this approach before and after the latest update (1.6.5) I doesn't work anymore with fetcher. I still get something like Property 'message' does not exist on type 'never'., although inferred fetcher typing looks following:

// in component which make a call to search route
import type { loader } from "~/routes/api/search";
const fetcher = useFetcher<typeof loader>();

// it produces
(alias) useFetcher<({ request }: DataFunctionArgs) => Promise<TypedResponse<{
    message: string;
    statusCode: number;
}> | TypedResponse<SearchResponse>>>(): FetcherWithComponents<...>

// usage triggers error: Property 'message' does not exist on type 'never'.
{fetcher.data && "message" in fetcher.data && <p>{fetcher.data?.message}</p>}
rossipedia commented 2 years ago

It doesn't look like useFetcher supports inferred loader/action return types. Probably because useFetcher can be used with any URL, whereas useLoaderData is explicitly linked with a Remix route.

kiliman commented 2 years ago

@rossipedia here's wrapper function to get typed fetchers

type TypedFetcherWithComponents<T> = Omit<FetcherWithComponents<T>, "data"> & {
  data: UseDataFunctionReturn<T> | null;
};
export function useTypedFetcher<T>(): TypedFetcherWithComponents<T> {
  return useFetcher<T>() as TypedFetcherWithComponents<T>;
}
image
rphlmr commented 2 years ago

@rossipedia here's wrapper function to get typed fetchers

type TypedFetcherWithComponents<T> = Omit<FetcherWithComponents<T>, "data"> & {
  data: UseDataFunctionReturn<T> | null;
};
export function useTypedFetcher<T>(): TypedFetcherWithComponents<T> {
  return useFetcher<T>() as TypedFetcherWithComponents<T>;
}
image

Is it planned to be added in a future release ? Thanks for this snippet <3

kiliman commented 2 years ago

Not sure. But note: to get the Date type, you have to use the superjson helper. Otherwise, you'll get the JSON serialized type. So in that screenshot, it will be today: string if you're using standard json on v1.6.5+

pcattori commented 2 years ago

Narrowed this down to how TS handles mapped types:

type Same<T extends object> = {[k in keyof T]: T[k]} // just copying the object via mapped type
type Before = { one: 1, two?: never }
type After = Same<Before>
//   ^? { one: 1, two?: undefined }

@m5r : this might be a good thing to ask the TS folks about directly. Not sure if its intentional or not.

pcattori commented 2 years ago

Going to close this since it seems more like a TS issue and/or clarification than for Remix.

@m5r feel free to reopen if you get any additional info from the TS team.