-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Next major version feature branch #10218
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
Conversation
🦋 Changeset detectedLatest commit: 8874b85 The changes in this PR will be included in the next version bump. This PR includes changesets to release 18 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
💻 Website PreviewThe latest changes are available as preview in: https://pr-10218.graphql-code-generator.pages.dev |
b3a8241 to
f5116a7
Compare
🚀 Snapshot Release (
|
| Package | Version | Info |
|---|---|---|
@graphql-codegen/cli |
6.0.0-alpha-20250907025311-8874b85f4c8e523c94bf7f20a7544c353bc1414a |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/core |
5.0.0-alpha-20250907025311-8874b85f4c8e523c94bf7f20a7544c353bc1414a |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/add |
6.0.0-alpha-20250907025311-8874b85f4c8e523c94bf7f20a7544c353bc1414a |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/fragment-matcher |
6.0.0-alpha-20250907025311-8874b85f4c8e523c94bf7f20a7544c353bc1414a |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/introspection |
5.0.0-alpha-20250907025311-8874b85f4c8e523c94bf7f20a7544c353bc1414a |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/schema-ast |
5.0.0-alpha-20250907025311-8874b85f4c8e523c94bf7f20a7544c353bc1414a |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/time |
6.0.0-alpha-20250907025311-8874b85f4c8e523c94bf7f20a7544c353bc1414a |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/visitor-plugin-common |
6.0.0-alpha-20250907025311-8874b85f4c8e523c94bf7f20a7544c353bc1414a |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/typescript-document-nodes |
5.0.0-alpha-20250907025311-8874b85f4c8e523c94bf7f20a7544c353bc1414a |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/gql-tag-operations |
5.0.0-alpha-20250907025311-8874b85f4c8e523c94bf7f20a7544c353bc1414a |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/typescript-operations |
5.0.0-alpha-20250907025311-8874b85f4c8e523c94bf7f20a7544c353bc1414a |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/typescript-resolvers |
5.0.0-alpha-20250907025311-8874b85f4c8e523c94bf7f20a7544c353bc1414a |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/typed-document-node |
6.0.0-alpha-20250907025311-8874b85f4c8e523c94bf7f20a7544c353bc1414a |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/typescript |
5.0.0-alpha-20250907025311-8874b85f4c8e523c94bf7f20a7544c353bc1414a |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/client-preset |
5.0.0-alpha-20250907025311-8874b85f4c8e523c94bf7f20a7544c353bc1414a |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/graphql-modules-preset |
5.0.0-alpha-20250907025311-8874b85f4c8e523c94bf7f20a7544c353bc1414a |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/testing |
4.0.0-alpha-20250907025311-8874b85f4c8e523c94bf7f20a7544c353bc1414a |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/plugin-helpers |
6.0.0-alpha-20250907025311-8874b85f4c8e523c94bf7f20a7544c353bc1414a |
npm ↗︎ unpkg ↗︎ |
aa191e7 to
dd2b0bd
Compare
fd51b95 to
56fea45
Compare
b0b7e86 to
0b0c9bf
Compare
1a64f6d to
6a2bca7
Compare
6a2bca7 to
30ac893
Compare
4ac5762 to
8da92a5
Compare
33a469f to
85a76ab
Compare
|
@eddeee888 What a huge set of changes – I appreciate the hard work that went into this over months! I'm confused about one change here:
The confusion is for 3 reasons: First of all, it seems to not have been deprecated but actually removed – so unless I'm missing something, there doesn't seem to be a way anymore to have anything except for I'm also curious why a "majority of use cases cannot implement resolvers in Interfaces". I get that it's an age-old discussion whether it should be possible to share only structure or also code via interfaces; but it certainly is convenient, e.g. when you have different user types that might even be managed in the same exact data source (think: import type { UserInterfaceResolvers } from "./types"
export const UserInterface: UserInterfaceResolvers = {
__resolveType: ({ __typename }) => __typename,
isConfirmed: ({ user }) => user.confirmedAt !== null,
// …and any other fields that might be shared between multiple user types
}import type { UserResolvers } from "./types"
export const User: UserResolvers = {
__resolveType: ({ __typename }) => __typename,
}import type { AdminResolvers } from "./types"
export const Admin: AdminResolvers = {
__resolveType: ({ __typename }) => __typename,
}With the changes in this release, unless I'm overlooking something, I'd either have to copy & paste And it only gets worse as number of types and shared behaviors increase; and then maybe the complexity increases as well (e.g. certain interfaces only apply to certain user types) and all of a sudden it turns into a mess. Last but not least, I'm wondering how this is related to federation. As far as I can see, the types on the GraphQL side of things don't change: The Could you give an example where this is an issue in the context of federation? |
|
Hi @clemens, Thanks for the question and feedback!
Apologies about the confusion in the term. In this context, it is meant to say the config is removed, as you suspect. I'll clarify this intention better in the future.
I believe this is how shared field resolvers on an Interface are supposed to work. I've tested this many times and never observed field resolvers in Interface work: import type { UserInterfaceResolvers } from "./types"
export const UserInterface: UserInterfaceResolvers = {
__resolveType: ({ __typename }) => __typename,
isConfirmed: ({ user }) => {
console.log("Never called: " , { user });
return user.confirmedAt !== null,
}
}
import type { UserResolvers } from "./types"
export const User: UserResolvers = {
isConfirmed: ({ user }) => {
console.log("Called when user is User: " , { user });
return user.confirmedAt !== null,
}
}
export const Admin: AdminResolvers = {
isConfirmed: ({ user }) => {
console.log("Called when user is Admin: " , { user });
return user.confirmedAt !== null,
}
}In this case, I've never seen the
You are right, it is not 🙂. This major release includes a mix of type changes for both normal and Federation use cases, and this change is applicable to both. |
|
With the code as you've shown it, I agree: Resolvers higher up in the chain don't get called, if a lower resolver already resolves a field. But that wasn't my point: My point was that if a higher resolver defines it and a lower resolver doesn't, then the method of the higher resolver gets called. But this doesn't seem to be possible anymore with the changes in this PR. I'll try and replicate our code as best as I can while including as little code as possible to not overwhelm you with our business logic… ;-) We have the following GraphQL types: """
Interface to implement the base user type.
"""
interface UserInterface {
id: ID!
email: Email!
"""
etc.
"""
}
"""
This interface carries all fields for a user that has verified their email and access to the app.
"""
interface VerifiedUserInterface implements UserInterface {
id: ID!
email: Email!
"""
etc.
"""
}
"""
This interface carries all fields that are required for a user who has at least one policy.
"""
interface UserWithPolicyInterface implements UserInterface & VerifiedUserInterface {
id: ID!
email: Email!
"""
etc.
"""
}
"""
The user has at least one confirmed policy and no unconfirmed policies.
"""
type User_WithPolicy implements UserInterface & VerifiedUserInterface & UserWithPolicyInterface {
id: ID!
}Note: The interfaces have further fields, the type has no further fields except the ID and the ones it gets from the interfaces. Then, there are these resolvers: import type { UserInterfaceResolvers } from "./types"
export const UserInterface: UserInterfaceResolvers = {
__resolveType: ({ __typename }) => __typename!,
id: ({ user }) => user.id,
email: ({ user }) => {
console.log("user.email in UserInterface", user.email)
return user.email
},
}import type { VerifiedUserInterfaceResolvers } from "./types"
export const VerifiedUserInterface: VerifiedUserInterfaceResolvers = {
__resolveType: ({ __typename }) => __typename,
email: ({ user }) => {
console.log("user.email in VerifiedUserInterface", user.email)
return user.email
},
}import type { UserWithPolicyInterfaceResolvers } from "./types"
export const UserWithPolicyInterface: UserWithPolicyInterfaceResolvers = {
__resolveType: ({ __typename }) => __typename,
}import type { User_WithPolicyResolvers } from "./types"
export const User_WithPolicy: User_WithPolicyResolvers = {
__resolveType: ({ __typename }) => __typename,
}In a Jest test, I'm then executing a query like this: query GetPayments {
meV2 {
__typename
... on UserWithPolicyInterface {
email
}
}
}Which logs When I add/remove the
Does that help with illustrating the problem? Happy to dive deeper or provide additional info. |
|
Hi @clemens , I'm unable to reprod using the provided materials 😞 . Maybe I'm missing the Could you help create an issue with minimal reprod using GitHub, Stackblitz or CodeSandbox please? This will help me track and get on top of this 🙏 |
|
I need to apologize: While trying to reproduce this in a Code Sandbox, I realized that we had a script in place that I wasn't aware of that does the magic. We're basically parsing all of our type definitions and then using our own little codegen to create extensions for every type that implements interfaces. In other words: For the example above, we'd actually codegen files like this: extend type User_WithPolicy implements UserInterface & VerifiedUserInterface & UserWithPolicyInterface {
email: Email!
"""
etc.
"""
}That then gets loaded together as part of the codegen where we're using your CLI to generate the final outputs. So your CLI would e.g. get this: type User_WithPolicy implements UserInterface & VerifiedUserInterface & UserWithPolicyInterface {
id: ID!
}
"""
etc.
"""
extend type User_WithPolicy implements UserInterface & VerifiedUserInterface & UserWithPolicyInterface {
email: Email!
"""
etc.
"""
}Long story short: It's probably not an issue with your changes and we might have to adapt that script to account for the changes you made. I'll give that a try and report back to you in case it turns out to be an issue after all. (And if it is, I'd try to create a proper setup to reproduce.) In the meantime, thanks for caring! <3 |
|
Thank you for collaborating with me on this @clemens! I'm really appreciate you getting back and explaining your use case, to make sure we don't miss anything! 🙏 |
|
@eddeee888 So I think I've run into a problem after all and I've managed to reproduce it in this CodeSandbox: https://codesandbox.io/p/devbox/objective-field-ssvjcd?workspaceId=ws_PEDGHUswTsoCvZUnDz9SSL The situation occurs when you have an interface and a type that implements this interface: interface UserInterface {
id: ID!
username: String!
email: String!
}
type ConfirmedUser implements UserInterface {
id: ID!
username: String!
email: String!
confirmedAt: String!
}When I'm then using the // old version (types-old.ts):
export type UserInterfaceR[esolvers<ContextType = any, ParentType extends ResolversParentTypes['UserInterface'] = ResolversParentTypes['UserInterface']> = {
__resolveType: TypeResolveFn<'ConfirmedUser', ParentType, ContextType>;
email?: Resolver<ResolversTypes['String'], ParentType, ContextType>;
id?: Resolver<ResolversTypes['ID'], ParentType, ContextType>;
username?: Resolver<ResolversTypes['String'], ParentType, ContextType>;
};
// new version (types-new.ts):
export type UserInterfaceResolvers<ContextType = any, ParentType extends ResolversParentTypes['UserInterface'] = ResolversParentTypes['UserInterface']> = {
__resolveType: TypeResolveFn<'ConfirmedUser', ParentType, ContextType>;
};Note the absence of the 3 interface fields ( The difference here seems to stem from For for the new version (as well as the old version with export const UserInterface: UserInterfaceResolvers = {
__resolveType: ({ __typename }) => __typename!,
id: ({ user }) => user.id,
// …
}I'm getting errors like I still don't fully understand the rationale behind this change. From your comment in #10221 and the referenced issue (#5648), am I correct to understand that this code is indeed never called? And thus the types in the old version were "wrong" in the sense that they required us to write what's effectively dead code? Note that we're indeed using |
Yes, this is correct, purely from how GraphQL resolver works.
This is interesting, I didn't know that |
|
A new option was added I've updated the doc to clarify this behaviour and its relationship with |
|
I can confirm that everything seems to work in our app by adding Thanks for getting this fixed and thanks for the quick turnaround! |
Related: #10413, #7020, #10385, #9701
typescript-resolvers & Federation Changes
Related: #10206
__resolveReferenceto applicableInterfaceentities, fix Interface types having non-meta resolver fields #10221__isTypeofis only generated for implementing types (of Interfaces) or Union members #10283__isTypeOfresolver to output meta #10417Breaking Changes
UnwrappedObjectutility type, as this was used to support the wrong previously generated type.onlyResolveTypeForInterfacesbecause majority of use cases cannot implement resolvers in Interfaces.generateInternalResolversIfNeeded.__resolveReferencebecause types do not have__resolveReferenceif they are not Federation entities or are not resolvable. Users should not have to manually set this option. This option was put in to wait for this major version.__isTypeOffor non-implementing-types or non-union-members@external,@providesand@key @keyscenarios easierRefactors
watchConfigdedupeFragmentsnoGraphQLTagBreaking changes
Record<PropertyKey, never>is used instead of{}for empty object type in@graphql-codegen/typescriptand@graphql-codegen/typescript-resolversClient Preset
- [ ] Improve default and forwarded configWill happen after major version update