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

@pothos/core@3.31.0 introduced some type errors in complex recursive types #1111

Closed
Cauen opened this issue Dec 19, 2023 · 5 comments
Closed

Comments

@Cauen
Copy link
Contributor

Cauen commented Dec 19, 2023

After 3.31.0 or specifically this change, the typings began to be "unraveled"

Before:
image

After:

image

This generated some problems if the typings are very deep and recursive
I think its not exactly an error. Because Graphql can send null when sometimes nulls are not accepted.

image

But at this level of depth, it is simply impossible to remove nulls at the typing level.

image

image

Since the idea of inputRef is to enable this "infinite" level of recursive typing. I think this change brings unexpected behavior to the functionality. And the expected behavior is what already existed before this RecursivelyNormalizeNullableFields.

What do you think? @hayes

@hayes
Copy link
Owner

hayes commented Dec 19, 2023

I am definitely open to reconsidering this, or finding alternative ways of handling these APIs.

The behavior you are seeing is exactly what was intended and is technically correct. Defining your prisma inputs they way you did, opens you up to runtime errors if anyone passes null values anywhere in their inputs, because prisma will reject them. Pothos has always prioritized having things be type-safe and correct, even if it comes at the cost of making the APIs a little less ergonomic.

That being said, I want the dev experience to be as good as possible for this. I think we could update the signature to something like inputRef<T extends object, Normalize = true> so you can do builder.inputRef<Prisma.ProfileWhereInput, false>. I think the current behavior is still the right default, but I understand wanting to opt out of it.

For prisma specifically, there is the prisma-utils plugin which handles stripping nulls at runtime, which can be a little safer/easier just realized that your maintaining a codegen tool that for this 😅

@hayes
Copy link
Owner

hayes commented Dec 19, 2023

I know I mentioned the 4.0 release a long time ago. I've been a little stuck on changes around nullability that have prevented me from actually getting that done. I have a lot of more ambitious changes I wanted to make, and trying to balance that with making upgrades easy has lead to it stagnating a bit. I'm hoping to have some time over the holidays to pick up some of that work again.

@Cauen
Copy link
Contributor Author

Cauen commented Dec 20, 2023

Thanks for the quick response @hayes

I think your proposed solution makes all sense in these cases:

builder.inputRef<Prisma.ProfileWhereInput, false>
or
builder.inputRef<Prisma.ProfileWhereInput, { normalize: false }>

I also agree that the standard solution makes sense

About the prisma-tools: I thought twice about using it because of the message "This package is highly experimental and not recommended for production use". Do you think it’s ready for production today?

About 4.0, I totally understand, you have a big project and full of plugins to handle. I'm very happy that you still think about improving it a lot, because I think Pothos is unbeatable at the moment

@hayes
Copy link
Owner

hayes commented Dec 21, 2023

Published a new version with the above change. Note that this will allow you do things that are technically not type-safe

Cauen added a commit to Cauen/prisma-generator-pothos-codegen that referenced this issue Dec 22, 2023
…o disable input normalization: hayes/pothos#1111. It fixes: #57.

Chore/Example-Update: Update Pothos/Prisma/Others to latest versions
Chore/Peer-deps: Remove @pothos/core exact version restriction
@Cauen
Copy link
Contributor Author

Cauen commented Dec 22, 2023

You're awesome @hayes

@Cauen Cauen closed this as completed Dec 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants