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

Support Entities With id: Int? #938

Closed
rjhilgefort opened this issue Aug 28, 2024 · 9 comments
Closed

Support Entities With id: Int? #938

rjhilgefort opened this issue Aug 28, 2024 · 9 comments
Labels

Comments

@rjhilgefort
Copy link

Hello! Thanks for this package. I'm implementing it in a code base to transition from prisma1 -> prisma5. I ran into an issue where the types don't appear to be flowing and I'm unable to call findManyCursorConnection. It's a pretty simple call site but findManyCursorConnection doesn't appear to be inferring the correct types.

  findManyCursorConnection(
    args => prisma.creatorAnalytics.findMany(args),
    () => prisma.creatorAnalytics.count(),
    {},
  );
image

The error is:
CleanShot 2024-08-28 at 15 03 39@2x

My schema looks like this. Is it because of the id: Int in my Prisma schema?
image

@queicherius
Copy link
Member

This supports number IDs (see tests). Your error talks about compKey which comes from your own codebase as far as I can tell.

@rjhilgefort
Copy link
Author

It looks like it's not happy with the types flowing through to the Cursor generic

CleanShot 2024-09-02 at 15 20 29@2x

@queicherius
Copy link
Member

This is also supported (see here) and without more surrounding code it's impossible to debug your issue.

@rjhilgefort
Copy link
Author

Appreciate the replies and understand it can be tough without a reproduction repo. That said, it seems to be a pretty simple problem with types not flowing when the id is number instead of string. When I mess with the type defs and change the generics assignment to be {id: number}, things work well for my problematic call site (but breaks all my other entities that are {id: string}).

image

In your previous comment, are you recommending that I use a custom cursor to coerce the number to be a string? Otherwise, my call site looks like the "type safe" example you linked. To be clear, types flow just fine as long as my entity has {id: string}.

@rjhilgefort
Copy link
Author

I see this original issue and followed it through to the merged PR, but the code looks to have changed since the original issue.

#1

I also noticed the tests and the apparent proof that it can work. When I tried it, my includes didn't flow through to the results and I couldn't access the related entities.

https://github.com/devoxa/prisma-relay-cursor-connection/blob/master/tests/index.spec.ts#L173

@queicherius
Copy link
Member

I just added a regression test for your use-case just to make sure it works.

It does, but you do have to manually pass the generic: https://github.com/devoxa/prisma-relay-cursor-connection/blob/master/tests/index.spec.ts#L194

//                                            v Your model type
const result = await findManyCursorConnection<User, { id: number }>(
//                                                    ^ Your cursor generic

I added a follow up-issue to make this easier: #944

@rjhilgefort
Copy link
Author

rjhilgefort commented Sep 5, 2024

Thanks again for the reply and additional tests, but I don't think this issue should be closed just yet. The problem remains that the results type of the findMany doesn't flow through out of the wrapper naturally. Providing the generics does "bandaid" the situation, but the types break down when you use includes or select in your findMany arguments.

Here you can see that Category exists on the results of the raw Prisma findMany call
image

Because of the manually provided generics, the findMany resulting types don't come out on the edges/nodes (if you don't provide generics for an {id: string} entity, the types flow fine).
image

Ideally, we would have the default assignment of the generics support both {id: number} and {id: string} without having to provide generics so the types can be inferred correctly. In the mean time, I've dupicated findManyCursorConnection to create an additional function that only handles {id: number}. Not ideal from a maintainability standpoint but it's the only thing I can figure so I can keep moving. Any further help would be greatly appreciated!

image

@queicherius
Copy link
Member

Providing the generics does "bandaid" the situation, but the types break down when you use includes or select in your findMany arguments.

Ah yeah. I added that to #944.

Ideally, we would have the default assignment of the generics support both {id: number} and {id: string} without having to provide generics so the types can be inferred correctly.

I agree with this, I added a branch that adds some (failing) type tests for this case, but I could not figure out how to make this work. PRs welcome!

@rjhilgefort
Copy link
Author

@queicherius Thanks! I couldn't figure out the types either in all my tinkering. If I figure it out, I'll contribute back here.

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

No branches or pull requests

2 participants