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

Feature: Local dev server to return __typename (similar to AppSync) #115

Closed
maoosi opened this issue Feb 17, 2023 Discussed in #113 · 12 comments
Closed

Feature: Local dev server to return __typename (similar to AppSync) #115

maoosi opened this issue Feb 17, 2023 Discussed in #113 · 12 comments
Labels
is: feature request New feature request package: server Server package
Milestone

Comments

@maoosi
Copy link
Owner

maoosi commented Feb 17, 2023

Discussed in #113

Originally posted by tomschut February 17, 2023
Hi,

Perhaps a weird question, but our front-end sometimes uses __typename to determine types returned from appsync. Appsync supplies these types, and to have it work locally we'd need them there too.

So my question is: would it be possible to have yoga server return the typenames as well?

Kind regards,

Tom

@maoosi maoosi added is: feature request New feature request package: server Server package labels Feb 17, 2023
@maoosi maoosi added this to the 1.0.0-rc.7 milestone Feb 17, 2023
@tomschut
Copy link

tomschut commented Mar 30, 2023

Hi,

I'd like to fix this issue. Is there anything I should know besides the contributing guide?

Kind regards,
Tom

@maoosi
Copy link
Owner Author

maoosi commented Mar 30, 2023

@tomschut sounds good, happy for you to give it a try! The only thing I would recommend is to work from the dev branch which is the most up-to-date and include lots of changes as part of the 1.0.0-rc.6 milestone.

@maoosi
Copy link
Owner Author

maoosi commented Mar 30, 2023

What you are wanting to fix is part of the server package.

Following the contribution guide - you should be able to setup a local dev environment up that will make changes easy to test in real-time. Let me know if you have questions!

@tomschut
Copy link

tomschut commented Apr 7, 2023

hi @maoosi ,
I've been playing around in a local dev environment trying to figure out how to add __typename to the response, but I ran into some issues.

Unlike in appsync, in yoga the lambda resolver is only triggered once no matter how nested the queries. Which means that if I want to add __typename in the server package I'd have to visit the entire lambda result and translate the entity names to graphql types. I suppose the typeFromAST function could be leveraged for that, but am not sure yet on whether there's enough information to make that work.

I've been thinking of solving the problem differently by either trying to add it in the generator package (which is undesirable as appsync doesn't need this code) or using the resolver system that yoga offers instead of the plugin that is used now, but that seems like too big a change.

So I thought it I'd circle back here and see if you could point me in the right direction.

ps. the dev environment was very easy to set up, so kudos for that

@maoosi
Copy link
Owner Author

maoosi commented Apr 9, 2023

@tomschut Unless your setup is different that what is provided with Prisma-AppSync, your Lambda resolver will also be triggered only once no matter how nested the queries. Prisma Client will directly handle all nested queries as opposed to calling new resolvers for each nested element. This technique also solves the n+1 problem.

The main difference is that AppSync will automatically add __typename to the response object, using the underlying GraphQL Types. Doing the same on the local server requires manually mapping the Types based on the incoming query and the Schema AST.

It took me a bit of time to figure out the right approach and it ended up being easier for me to push the code directly rather than just providing a direction. I’ve pushed a first iteration that seems to be working okay (see 6090eb1). I've only tested it with basic queries, so please give it a try and let me know if working fine on your end!

@maoosi maoosi modified the milestones: 1.0.0-rc.7, 1.0.0-rc.6 Apr 9, 2023
@tomschut
Copy link

tomschut commented Apr 11, 2023

HI @maoosi , thanks for your solution. I was a bit shocked that I hadn't realized that each resolver will try to resolve the entire query, so thanks for pointing that out. On our end we'll need to figure out how this will impact caching and whether it's desirable.

The solution works for top level resources, but not yet for the sub resources. I'll have a look into that.

ps. have you noticed that awsdate outputs are no longer working in the dev branch?

@tomschut
Copy link

I think this solves it on my end, but am unsure if there's more types to be handled.

            for (let index = 1; index < pathsWithoutArrays.length - 1; index++) {
                if (fields?.type.constructor.name === GraphQLObjectType.name)
                    fields = (fields?.type as GraphQLObjectType)?.getFields()?.[pathsWithoutArrays[index]]
                else if (fields?.type.constructor.name === GraphQLList.name)
                    fields = (fields?.type as GraphQLList<any>)?.ofType?.getFields()?.[pathsWithoutArrays[index]]
            }

@maoosi
Copy link
Owner Author

maoosi commented Apr 14, 2023

Thanks @tomschut - do you want to open a PR? Or do you prefer me to directly insert the new piece of code?

@tomschut
Copy link

I don't mind you adding the lines, so if you want to, go ahead.
I've just tested with a larger query, and can't find anything wrong. There's one type: GraphQLUnionType, which I'm not sure if it needs to be added in the conditions of the loop as well.

let me know when the code's availabe for testing in an rc. and if you have an idea why aws-dates stopped working

@maoosi
Copy link
Owner Author

maoosi commented Apr 15, 2023

@tomschut the issue was due to the new traverseNodes function. It should work better with c5a0a93.

@maoosi
Copy link
Owner Author

maoosi commented Apr 15, 2023

@all-contributors please add @tomschut for code and ideas!

@allcontributors
Copy link
Contributor

@maoosi

I've put up a pull request to add @tomschut! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is: feature request New feature request package: server Server package
Projects
Status: Released
Development

No branches or pull requests

2 participants