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

Augment resolved types with internal properties #90

Open
lorefnon opened this issue Feb 6, 2024 · 13 comments
Open

Augment resolved types with internal properties #90

lorefnon opened this issue Feb 6, 2024 · 13 comments

Comments

@lorefnon
Copy link
Contributor

lorefnon commented Feb 6, 2024

So, not sure if this is a bit of esoteric use case, but I'd like to have an API that enables me to augment the object types defined through garph with some internal properties.

These properties are not exposed in the graphql schema, but will be enforced by typescript to be present in the values returned by the resolvers and will be expected to be available in the parent object passed to field resolvers.

The API I am thinking of is something like below, but obviously I will also be happy with any alternative approaches that effectively facilitate this.

export const BookT = g.type("Book", { 
    // Exposed properties:
    chapters: g.ref(ChapterT).list(),
}).augment<{ 
    // Internal properties:
    contentHash: string
}>()

type BookModel = Infer<typeof BookT, {
    internal: true
}>
// { chapters: Chapter[], contentHash: string } 
//^ This is what the resolvers will need to return and any field resolvers of book will receive 

type Book = Infer<typeof BookT>
// { chapters: Chapter[] }
//^ What is exposed to the outside world 

Just to be clear, this feature is primarily about enabling this in type-safe manner. I can already return an object with arbitrary additional properties and use them in field resolvers in plain js.

This is trivially easy to achieve in libraries like Microprofile GraphQL or TypeGraphQL where the DTO classes are authored by developer and fields are selectively exposed to the outside world through decorators/annotations.

Some other graphql implementations like java-graphql and dotnet chillicream library have a concept of subtree context or scoped context which is a slightly roundabout way to address this by selectively propagating some context to child resolvers. GraphQL.js also has an open issue for this.

However this approach is a bit harder to make type-safe and most common cases where we want to propagate some additional information to child resolvers is easily solved by having some additional fields in the parent object.

@mishushakov
Copy link
Member

Why not this instead:

export const BookT = g.type("Book", { 
    chapters: g.ref(ChapterT).list(),
    contentHash: g.internal<string>()
})
type BookModel = Infer<typeof BookT, {
    internal: true
}>

One question I have though: Do the internal properties have to be resolved?

@lorefnon
Copy link
Contributor Author

lorefnon commented Feb 8, 2024

I don't mind g.internal. But it may not be so obvious that this works only inside an object type and nowhere else. Eg. what happens if I use contentHash: g.internal<string>() inside an input object definition - do we make it throw at runtime?

Do the internal properties have to be resolved?

Yes, in the sense that any resolver that returns Book must return the associated internal properties too. And any field resolvers that receive Book as parent must receive them. I don't think separate field resolvers for internal properties would make sense though.

@lorefnon
Copy link
Contributor Author

lorefnon commented Feb 8, 2024

Actually it may not be so hard to statically disallow it. Let me fiddle around and see.

@mishushakov
Copy link
Member

mishushakov commented Feb 8, 2024

What's the use-case of internal then? Is it just to hide it from the schema definition (like private property?)

@lorefnon
Copy link
Contributor Author

lorefnon commented Feb 8, 2024

Yes, basically to propagate some internal details from parent object to field resolvers.

@mishushakov
Copy link
Member

Can you see my PR here: #92
Does it work the way you want?

@lorefnon
Copy link
Contributor Author

lorefnon commented Feb 8, 2024

Hi, yeah. This looks great.

Only caveat I can see is that we are now able to do something like this:

const addrFilterType = g.inputType('AddrFilter', {
  name: g.string(),
  id: g.internal<string>(),
})

If this is done, the args passed to resolver now has an id property but it is impossible to supply that id so it will result in a runtime error instead of a type error if the resolver uses it.

@mishushakov
Copy link
Member

Can't reproduce. Can you give me a full example, where the runtime error occurs?

@lorefnon
Copy link
Contributor Author

lorefnon commented Feb 8, 2024

Yes, this is a modification of your internal.ts example.

@mishushakov
Copy link
Member

I can make the internal fields undefined there, but if you want to completely exclude the property, then your approach was actually better, but the implementation would be different, like how implements and extend is now implemented (but without actually adding the properties to schema)

@lorefnon
Copy link
Contributor Author

lorefnon commented Feb 9, 2024

In #93, I updated your branch to allow internal properties only inside object types.

This retains g.internal but attempting to use it anywhere outside an object type will result in reasonably understandable type error:

image

@mishushakov
Copy link
Member

mishushakov commented Feb 13, 2024

Hey, looks nice, maybe a bit over-engineered, maybe 😁. I'm still thinking of use-cases or code examples where this feature could be really useful? I appreciate the community adding new things though 🙏

@lorefnon
Copy link
Contributor Author

lorefnon commented Feb 14, 2024

Thanks @mishushakov. For example/use-case, we can consider the following query:

query fetchUserBalance {
    user(email: "[email protected]") {
        accountBalance {
           total
        }
    }
}

To resolve this I lookup users table with email, the returned user row has a stripe_id col.
Then in accountBalance resolver, I make a request to stripe using this stripe_id and fetch the user's current balance.

To enable this the user object returned from user resolver should include the stripe_id column value, and the parent object that accountBalance resolver receives should have that too, but we may not want to expose the stripeId as a field in the exposed User type because use of stripe is an internal detail and exposing it can have security repurcussions.

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