maoosi/prisma-appsync

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

maoosi opened this issue · 12 comments

maoosi commented

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

Hi,

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

Kind regards,
Tom

maoosi commented

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

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!

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 commented

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

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?

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 commented

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

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 commented

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

maoosi commented

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

@maoosi

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