Don't require ToString to return a valid enum value name in IEnumTypeDescriptor.Value
cmeeren opened this issue · 2 comments
Product
Hot Chocolate
Is your feature request related to a problem?
When configuring an enum type using a type deriving from EnumType<_>
, we call IEnumTypeDescriptor.Value(...)
in the Configure
override to add values. The Value
method calls INamingConventions.GetEnumValueName
, which in the default naming conventions calls ToString()
on the enum value.
Now consider a non-enum type that we want to surface as an enum in GraphQL. My use-case is surfacing F# union types as enums in HotChocolate.FSharp, but the principle is the same for any non-enum type, so for your convenience I'll give a C# example:
public class Component
{
public static readonly Component SystemA = new Component("System.A");
public static readonly Component SubsystemA = new Component("Subsystem.A");
private string Value { get; }
private Component(string value)
{
Value = value;
}
public override string ToString()
{
return Value;
}
}
public class ComponentDescriptor : EnumType<Component>
{
protected override void Configure(IEnumTypeDescriptor<Component> descriptor)
{
descriptor.Value(Component.SystemA).Name("SYSTEM_A");
descriptor.Value(Component.SubsystemA).Name("SUBSYSTEM_A");
}
}
What happens above is that when I call e.g. Value(Component.SystemA)
, the naming conventions end up getting the value "System.A"
from the ToString
call it does. Since this contains a dot, it's not a valid GraphQL enum name, and HotChocolate immediately throws at this point (in HotChocolate.Utilities.NameUtils.EnsureGraphQLName
, via the setter of DefinitionBase.Name
), not reaching the Name
call which specifies a proper GraphQL enum name.
The solution you'd like
I'm open to suggestions, but one simple, low-maintenance and backwards compatible solution would be to add an overload of IEnumTypeDescriptor<_>.Value
that accepts the name as the second parameter, and just set DefinitionBase.Name
to this name. I would then call:
descriptor.Value(Component.SystemA, "SYSTEM_A");
I have considered adding a custom naming convention in FSharp.HotChocolate, but that is not something I'd like to do, because either FSharp.HotChocolate would override the user's custom naming conventions if set, or the user's would override the library's, negating the fix.
This has wider implications. The API needs to be consistent across all types. I do not want to introduce a new overload here. A better solution is to not set these things until the type is created. But this is quite a large refactoring with implications across a lot of extensions. We will have a look at it but this is nothing that will be done in the V14 or V15 cycle. We have a larger work at the moment focused on schema primitives to get a new abstraction in that will make schema building more flexible with source generators. Until this is done we will not attempt any larger changes to the core.
A better solution is to not set these things until the type is created.
I wholeheartedly agree.
We have a larger work at the moment ... Until this is done we will not attempt any larger changes to the core.
I understand. It is of course unfortunate, since there is no workaround apart from changing a type's ToString
implementation, which may not always be possible, but in my specific case I can live with it.