remix-run / remix

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

useLoaderData type inference broken when returned data has a `data` key #5211

Closed dmarkow closed 1 year ago

dmarkow commented 1 year ago

What version of Remix are you using?

1.11.1

Steps to Reproduce

If the object returned from the loader includes a data key, the type inference returns only the items nested under that key. So in this case, typescript thinks useLoaderData returns just {item: number}. The code works just fine - item and other are being destructured properly and displaying on the page, it's just the types that are wrong. If I rename the data key to something else, it works fine and typescript returns the full type, e.g. {myData: {item: number}, other: number}. This started when defer was released.

export const loader = async () => {
  return json({
    data: { item: 1 },
    other: 2
  })
}

export default function Index() {
  const { data: { item }, other } = useLoaderData<typeof loader>();

  return <div>Item: {item}, Other: {other}</div>;
}
> yarn tsc
yarn run v1.22.19
$ /Users/dylan/dev/data-issue/node_modules/.bin/tsc
app/routes/index.tsx:12:11 - error TS2339: Property 'data' does not exist on type 'SerializeDeferred<{ item: number; }>'.

12   const { data: { item }, other } = useLoaderData<typeof loader>();
             ~~~~

app/routes/index.tsx:12:27 - error TS2339: Property 'other' does not exist on type 'SerializeDeferred<{ item: number; }>'.

12   const { data: { item }, other } = useLoaderData<typeof loader>();
                             ~~~~~

Expected Behavior

Type inference should work as it did before, or if data is a reserved name, the documentation should say as much.

Actual Behavior

Typescript infers the wrong types, but the code works fine.

emilbryggare commented 1 year ago

I've this problem as well with the latest version of Remix. I just wanted to add that it also happens when using useFetcher in actions.

Reproduced this in a minimal repository for you: https://github.com/emilbryggare/remix-types/

import type { LoaderArgs } from "@remix-run/node";
import { json } from "@remix-run/node";
import { useLoaderData } from "@remix-run/react";

export const loader = async ({ request }: LoaderArgs) => {
  // Generate correct types.
  // return json({ myData: { message: "Hello World!" } });

  // Does not generate correct types, e.g. SerializeDeferred
  return json({ data: { message: "Hello World!" } });
};
export default function Index() {
  let loaderData = useLoaderData<typeof loader>();
  // Type is when data key is present in return object:
  //   SerializeDeferred<{
  //     message: string;
  // }>
  return (
    <div style={{ fontFamily: "system-ui, sans-serif", lineHeight: "1.4" }}>
      <h1>Welcome to Remix</h1>
    </div>
  );
}
dmarkow commented 1 year ago

Looks like this is the problem. https://github.com/remix-run/remix/blob/006b58bfec02622f9881c1e37444baef60584eda/packages/remix-server-runtime/serialize.ts#L21-L31

https://github.com/remix-run/remix/blob/006b58bfec02622f9881c1e37444baef60584eda/packages/remix-server-runtime/responses.ts#L9-L14

TypedDeferredData<infer U> matches on our result because there is a data key, instead of moving down to SerializeObject which it should since I'm not actually using defer.

CapitaineToinon commented 1 year ago

Can confirm this is also a problem with actions, returning POJOs or using the json helper.

moishinetzer commented 1 year ago

This is still an issue with actions, typed loaders are working for me.

Meaning, useActionData<typeof action>() is inferred completely wrong.

balzdur commented 1 year ago

To help solve the issue, it comes from this :

declare type Serialize<T> = IsAny<T> extends true ? any : T extends TypedDeferredData<infer U> ? SerializeDeferred<U> : ...

export declare type TypedDeferredData<Data extends Record<string, unknown>> = Pick<DeferredData, "init"> & {
    data: Data;
};

Type narrowing tends to consider every payload with {data: ..} to be a "defer" value.

ronnylt commented 1 year ago

Same issue here:

image

raminrez commented 1 year ago

This problem still exists ! Any idea how to fix it?

michaeldebetaz commented 1 year ago

Thanks for reporting the issue! I posted it on the Discord a couple of weeks ago and was late to report it here :)

For debbuging without setting up a whole environment, I would recommend the typescript sandbox. Here is a gist of the issue on TS Playground.

As you can see, the type inference takes what is in the "data" property instead of considering the whole object.

Another example:

export async function action({ request }: ActionArgs) {
  // Other returns are possible
  return json({ data: { foo: "bar" }, errors: { foo: "baz" })
}

export default function Something() {
  const actionData = useActionData<typeof action>()
  //    ^ actionData has type { foo: "bar" } šŸ¤·ā€ā™‚ļø
  const data = actionData && "data" in actionData ? actionData.data : null
  //    ^ āŒ data is of type string, but should be { foo: string }
  //          But it works if the property is named something else
  //          than "data", like "formData" šŸ¤·ā€ā™‚ļø
}
okalil commented 1 year ago

That also happened to me using Remix 1.14.0 (cloudflare runtime).

huyphamfc commented 1 year ago

This is still an issue with actions, typed loaders are working for me.

Meaning, useActionData<typeof action>() is inferred completely wrong.

To solve this issue, I implement return json(data) in the "action" function.

mikeybinns commented 1 year ago

This still occurs on Remix 1.15, and it also occurs on the SerializeFrom type helper, and the useFetcher<Type>() generic type.

I also have a typescript playground for testing.

bryce-pearce commented 1 year ago

here is a code sandbox replication as well

https://codesandbox.io/p/sandbox/confident-feather-x0oe5q?file=%2Fapp%2Froot.tsx&selection=%5B%7B%22endColumn%22%3A21%2C%22endLineNumber%22%3A31%2C%22startColumn%22%3A21%2C%22startLineNumber%22%3A31%7D%5D

MichaelDeBoey commented 1 year ago

Closed by #5516

github-actions[bot] commented 1 year ago

šŸ¤– Hello there,

We just published version v0.0.0-nightly-9af8868-20230412 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

jcloutier-indeed commented 1 year ago

It looks like it resolved the issue for us from what I can see.

machour commented 1 year ago

1.16.0 is out šŸŽ‰

Darshan562 commented 10 months ago

What version of Remix are you using?

1.11.1

Steps to Reproduce

If the object returned from the loader includes a data key, the type inference returns only the items nested under that key. So in this case, typescript thinks useLoaderData returns just {item: number}. The code works just fine - item and other are being destructured properly and displaying on the page, it's just the types that are wrong. If I rename the data key to something else, it works fine and typescript returns the full type, e.g. {myData: {item: number}, other: number}. This started when defer was released.

export const loader = async () => {
  return json({
    data: { item: 1 },
    other: 2
  })
}

export default function Index() {
  const { data: { item }, other } = useLoaderData<typeof loader>();

  return <div>Item: {item}, Other: {other}</div>;
}
> yarn tsc
yarn run v1.22.19
$ /Users/dylan/dev/data-issue/node_modules/.bin/tsc
app/routes/index.tsx:12:11 - error TS2339: Property 'data' does not exist on type 'SerializeDeferred<{ item: number; }>'.

12   const { data: { item }, other } = useLoaderData<typeof loader>();
             ~~~~

app/routes/index.tsx:12:27 - error TS2339: Property 'other' does not exist on type 'SerializeDeferred<{ item: number; }>'.

12   const { data: { item }, other } = useLoaderData<typeof loader>();
                             ~~~~~

Expected Behavior

Type inference should work as it did before, or if data is a reserved name, the documentation should say as much.

Actual Behavior

Typescript infers the wrong types, but the code works fine.

I face same issue but in my project I use JavaScript instead of TypeScript

I use Remix 2.0.0 version Error: Property 'filterData' does not exist on type 'unknown' Image (https://github.com/remix-run/remix/assets/96987506/f7753cfe-d0bb-4d2a-93c7-504188133d1c)

pcattori commented 10 months ago

@Darshan562 a bunch of types were improved in 2.1-2.4. If you still have issues after upgrading to those versions, please file a separate issue.