-
Notifications
You must be signed in to change notification settings - Fork 17
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
Propagate interface fields #120
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for grats ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
} | ||
-- TypeScript -- | ||
import { iThingGreeting as doohickeyIThingGreetingResolver } from "./concreteTypeInheritsFunctionFieldFromClosestTransitiveInterfacets"; | ||
import { greeting as widgetGreetingResolver } from "./concreteTypeInheritsFunctionFieldFromClosestTransitiveInterfacets"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is incorrect. Widget should still be using iThingGreeting
.
name: "greeting", | ||
type: GraphQLString, | ||
resolve(source) { | ||
return doohickeyGreetingResolver(source); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no "correct" answer for what this resolver should do. IThing
and Entity
each define a greeting
field and their implementation's differ. Doohicky
implements both equally so which should it choose?
Note, I think some languages with multiple inheritance leave it to ordering. The ordering of the inheritance becomes meaningful and the first one wins.
We likely need this to become a compile time error and force the user to define an explicit resolver for the type.
Thinking aloud: Another choice would be to start supporting the ability to define interfaces using (abstract?) classes and then only allow those interfaces to have concrete implementations. That would allow us to piggy-back on TypeScript only allowing single inheritance to ensure we always have a single unambiguous implementation for each field. However, it does not play well with functional style, where you want to be able to define your types as object literals or perhaps interfaces themselves.
This PR changes how interface fields are handled. Previously, each interface implementor was responsible for adding
/** @gqlField */
to each field. Even those which were already annotated on one of the interfaces it implements. This was tedious.The one exception was fields defined with functions on interfaces. Here we would try to propagate the implementation to all implementors. However, there was a bug where we did not propagate the field to transitive implementors.
This PR addresses both issues by automatically propagating all fields to all implementors (including transitive implementors). If an implementor provides its own implementation of a field, the more specific implementation will be used.