Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
dmarkow opened this issue Jan 22, 2023 · 18 comments
Closed

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

dmarkow opened this issue Jan 22, 2023 · 18 comments
Assignees
Labels
bug Something isn't working feat:typescript

Comments

@dmarkow
Copy link
Contributor

dmarkow commented Jan 22, 2023

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.

@machour machour added bug Something isn't working feat:typescript and removed bug:unverified labels Jan 22, 2023
@emilbryggare
Copy link
Contributor

emilbryggare commented Jan 25, 2023

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
Copy link
Contributor Author

dmarkow commented Jan 25, 2023

Looks like this is the problem.

type Serialize<T> =
IsAny<T> extends true ? any :
T extends TypedDeferredData<infer U> ? SerializeDeferred<U> :
T extends JsonPrimitive ? T :
T extends NonJsonPrimitive ? never :
T extends { toJSON(): infer U } ? U :
T extends [] ? [] :
T extends [unknown, ...unknown[]] ? SerializeTuple<T> :
T extends ReadonlyArray<infer U> ? (U extends NonJsonPrimitive ? null : Serialize<U>)[] :
T extends object ? SerializeObject<UndefinedToOptional<T>> :
never;

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

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
Copy link

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

@moishinetzer
Copy link

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

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

@balzdur
Copy link

balzdur commented Feb 14, 2023

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
Copy link
Contributor

ronnylt commented Feb 16, 2023

Same issue here:

image

@raminrez

This comment was marked as duplicate.

@michaeldebetaz
Copy link
Contributor

michaeldebetaz commented Feb 22, 2023

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
Copy link

okalil commented Mar 18, 2023

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

@huyphamfc
Copy link

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
Copy link
Contributor

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.

@MichaelDeBoey
Copy link
Member

Closed by #5516

@github-actions
Copy link
Contributor

🤖 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
Copy link

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

@machour machour added the awaiting release This issue has been fixed and will be released soon label Apr 29, 2023
@machour machour reopened this Apr 29, 2023
@machour machour removed the awaiting release This issue has been fixed and will be released soon label May 1, 2023
@machour
Copy link
Collaborator

machour commented May 1, 2023

1.16.0 is out 🎉

@machour machour closed this as completed May 1, 2023
@Darshan562
Copy link

Darshan562 commented Dec 20, 2023

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
Copy link
Contributor

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working feat:typescript
Projects
None yet
Development

No branches or pull requests