-
-
Notifications
You must be signed in to change notification settings - Fork 789
Use correct type name when referencing type with ID<Type> #8504
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
base: main
Are you sure you want to change the base?
Conversation
Related: #6264 @michaelstaib @glen-84 : Did I miss something here? I considered fluent type definition naming and the [GraphQLName] attribute by using the name of the referenced ObjectType after it was created. I couldn’t think of anything else at the moment... |
src/HotChocolate/Core/src/Types/Types/Relay/Extensions/NodeIdNameDefinitionUnion.cs
Outdated
Show resolved
Hide resolved
src/HotChocolate/Core/src/Types/Types/Relay/Extensions/NodeIdNameDefinitionUnion.cs
Outdated
Show resolved
Hide resolved
src/HotChocolate/Core/src/Types/Types/Relay/Extensions/RelayIdFieldExtensions.cs
Outdated
Show resolved
Hide resolved
src/HotChocolate/Core/src/Types/Types/Relay/Extensions/RelayIdFieldHelpers.cs
Show resolved
Hide resolved
src/HotChocolate/Core/src/Types/Types/Relay/NodeResolverTypeInterceptor.cs
Outdated
Show resolved
Hide resolved
src/HotChocolate/Core/test/Types.Tests/Types/Relay/IdDescriptorTests.cs
Outdated
Show resolved
Hide resolved
src/HotChocolate/Core/test/Types.Tests/Types/Relay/IdDescriptorTests.cs
Outdated
Show resolved
Hide resolved
src/HotChocolate/Core/test/Types.Tests/Types/Relay/IdDescriptorTests.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Glen <[email protected]>
@@ -195,5 +320,75 @@ public interface IFooPayload | |||
string AnotherId { get; set; } | |||
} | |||
|
|||
private class Another; | |||
public class Another |
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.
Just want to point out this change. Since we now depend on the type to resolve the correct ID–name mapping, the type must be a valid GraphQL type (which is not the case for a private class that won’t be detected by HC).
Probably not relevant for 99.9% of cases, but still breaking.
I applied the changes from the review, thanks 👍 Regarding the docs: Should this change be mentioned in the migration guide from v15 to v16? I could add it in another PR. The generic ID attribute isnt very popular in the current docs, so maybe that could be reworked as well. |
Summary of the changes (Less than 80 chars)
descriptor.Name("...")
and[GraphQlName("...")]
forID<Type>
Closes #6256