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

Is isTypeOf required ? on builder.node - Relay Plugin #98

Closed
pixelmund opened this issue Jul 9, 2021 · 4 comments
Closed

Is isTypeOf required ? on builder.node - Relay Plugin #98

pixelmund opened this issue Jul 9, 2021 · 4 comments

Comments

@pixelmund
Copy link

Typescript is yelling at me because isTypeOf on builder.node is required, so i looked at the codebase https://github.com/hayes/giraphql/blob/8787140f51d82247eb8a9c1804dacb5c8a5bc525/packages/plugin-relay/src/schema-builder.ts#L155 and it seems like the types are wrong in this case? because there is a default way to handle it if no isTypeOf is given.

@hayes
Copy link
Owner

hayes commented Jul 9, 2021

The default isTypeOf only works if you use a class as the type param, or the shape associated with the type ref has a _type property that can be used to match again.

Something like:
builder.node(SomeClass, {... options}) or builder.node(builder.objectRef<{_type: 'SomeType'}>('SomeType'), {... options})

On a phone, so not sure how clear that is. If you are using one of these patterns, and still seeing type errors, some more context on how you are calling the node function would be helpful

@pixelmund
Copy link
Author

I'm at work right now, i can give a proper example in a few days. But i just tried your suggestion

builder.node(builder.objectRef<{ _type: 'SomeType'; id: string }>('SomeType'), {
	id: {
		resolve: (node) => node.id
	},
	fields: (t) => ({ id: t.exposeID('id') })
});

It's still yelling about isTypeOf is required.
If i create a class it goes away, but since my data usually is not represented in classes rather coming from prisma i don't know what the best way is to create nodes.

Also with the objectRef approach would i have to add _type to every object? Or am i misunderstanding something.

@hayes
Copy link
Owner

hayes commented Jul 9, 2021

Sorry, small typo there, it should be __type, and yes, you would need to add the type to each instance of the node you resolve.

The issue isTypeOf solves is that when there is a field that resolves to the Node interface, graphql needs be able to figure out which type is actually being resolved. If the node is being loaded through the default node or nodes queries we could determine what type the node is, but for custom fields that return nodes all we know is we got an object back and need to figure out what type it's supposed to be. If it's an instance of a class, or the object has a property that tells us the type, we can create a default isTypeOf method that can use an instanceof check, or read the type property. Lacking those, we don't have another way to figure out what the type should be, which is why it becomes required.

I've got a few ideas on how to make this better in the future, but currently you need to manually provide a way to resolve the type of a node.

In the mean time, I don't have a great solution for Prisma. I know some people actually just add the graphql type into their db. This is obviously not ideal. There is a feature request out in Prisma to include the tabel/model name in query results. This would make it trivial to write the isTypeOf functions. Adding on the __type property manually is not perfect, but it might be the

@pixelmund
Copy link
Author

@hayes That makes much more sense now, thank you! I will go ahead and close this issue for now.

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