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

Single Fetch: TypeScript implementation of defineLoader doesn't support records with arrays #9514

Open
KubaJastrz opened this issue May 30, 2024 · 8 comments

Comments

@KubaJastrz
Copy link

KubaJastrz commented May 30, 2024

Reproduction

I copied the implementation of Serializable into TypeScript playground. Let me know if you want me to submit an actual reproduction with loader code.

https://www.typescriptlang.org/play/?#code/C4TwDgpgBAyhBOBLAhgG0QL2QI1dAvFAK4B2AJhAGaIkRlQA+UJRqqjU2A9l3siRwDOwJCQDmQkAFtu7JixkIOAQXjxkIADxwkaTDjwA+DgG8AUFCgBtANYQQALigAFeF0jxQAaXsBdJzoo6Fi4EADcZgC+HNiIYjTAHAAiyMDQTACqAEoAMhxZEGIAogAeYBxFalzwHACyyGDaCEH6oQA0sM16IUYccMBNusEGEMZMrlxSiIIQgy09oxFmZgkIlMgAxtApwMhQ5pbowk4508AAkmlSVr4RkSskafDrW1CnwpcQUvsWUIhkTmEojEd2WFA2qGQ8GgGy4JGEUDIqWQTh2yGWAHoMVBlIJBHESDQJGQuBBBCQAOSJADu1RsUAcGLMsPhiWwyABnSGrTwUEISN2SyxUHqYDARKgtPgNkEUAA-HLmXCEWIeJzAt0Rnyfoczk4BcgAHRHYCGqQNAAUFsQVwAlHzjBaDpY-pybV9Df82r9Irbbd7IkA

type Serializable = undefined | null | boolean | string | symbol | number | Array<Serializable> | {
  [key: PropertyKey]: Serializable;
} | bigint | Date | URL | RegExp | Error | Map<Serializable, Serializable> | Set<Serializable> | Promise<Serializable>;


interface Data {
  list: ListItem[];
}
interface ListItem {
  id: string;
}

declare const data: Data

// Assigning doesn't work :/
const bad: Serializable = data;

// Mapping works ??
const good: Serializable = {
  list: data.list.map((item) => ({
    id: item.id,
  })),
}

System Info

npmPackages:
    @remix-run/dev: ^2.9.2 => 2.9.2 
    @remix-run/node: ^2.9.2 => 2.9.2 
    @remix-run/react: ^2.9.2 => 2.9.2 
    @remix-run/serve: ^2.9.2 => 2.9.2 
    vite: ^5.2.12 => 5.2.12 
    typescript: ~5.4.5 => 5.4.5

Used Package Manager

pnpm

Expected Behavior

No TypeScript errors

Actual Behavior

TypeScript error:

Type 'Data' is not assignable to type 'Serializable'.
  Type 'Data' is not assignable to type '{ [key: string]: Serializable; [key: number]: Serializable; [key: symbol]: Serializable; }'.
    Index signature for type 'string' is missing in type 'Data'.(2322)
@tomiwaajayi
Copy link

I have a similar issue.
I am using the unstable_defineLoader fn which is meant to improve type safety (according to the docs)

my loader returns this

return { store, cart, ENV: getEnv(), userPrefs: userPrefsCookie, transactionRef, }

And I get errors that saying cart is not of type Serializable

@KubaJastrz
Copy link
Author

KubaJastrz commented May 30, 2024

I entered a little rabbit hole and all roads lead to this: microsoft/TypeScript#50087 (comment)

It seems like this is an intended mechanic in TS. Workaround would be to use type over interface, although that is not always possible (my original issue is caused by an interface imported from 3rd party package 😢).

More cases in the > playground <.

EDIT: I added more mixed cases, it seems like it has to be all type: > playground <

@brookslybrand
Copy link
Contributor

Yep, I ran into this too, and @pcattori confirmed exactly what you said

Thanks for adding the types. Definitely a problem for 3rd party packages like you mentioned

@tomiwaajayi
Copy link

tomiwaajayi commented May 31, 2024

@brookslybrand
Is there any temp fix for this ?

Or we'd have to wait for some update from the remix team.

For context:
We have a types repo shared by backend team and frontend team.
So it's basically impossible for us to change all interfaces to type.

@kiliman
Copy link
Collaborator

kiliman commented May 31, 2024

@tomiwaajayi either stop using defineLoader or override it to support your requirements.

This function is there just to help with types. It's not a requirement.

@brookslybrand
Copy link
Contributor

@tomiwaajayi we're currently looking into good options/tradeoffs

A temporary fix would be to rework your type in way to where it undoes it's definition as an interface. You can do this through type utilities or by rewriting the type

For example, if you have an interface like

interface BlogPost {
  title: string;
  authors: { name: string }[];
}
Pick<BlogPost, 'title' | 'authors'>

will get it to work.

That's not a perfect solution by any means, nor the officially recommended one, but remember that Single Fetch is still an unstable API, so right now any suggestions I have are going to be a hack around the final solution

@KubaJastrz
Copy link
Author

KubaJastrz commented May 31, 2024

Here's a couple of implementations found in type-fest discussions:

> playground <

But beware, none of them happen to be exported from type-fest/index.d.ts at the moment. Don't know if by mistake or by design, because I found ConditionalSimplifyDeep in their source code.

@tomiwaajayi
Copy link

Thanks @kiliman @brookslybrand @KubaJastrz

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants