graphql-platform icon indicating copy to clipboard operation
graphql-platform copied to clipboard

DataLoader source generation could be improved when used in business layer

Open mvestergaard opened this issue 2 years ago • 0 comments

Product

Hot Chocolate

Is your feature request related to a problem?

I'm trying out the source generator for data loaders in our project, where our data loaders live in the business layer and are used in MediatR requests.

I've found a few issues.

  1. The DataLoaderAttribute and DataLoaderDefaultsAttribute live in HotChocolate.Abstractions and not GreenDonut which requires adding a dependency to more HotChocolate specific things from a layer that doesn't really know anything about graphql.
  2. Even though GenerateRegistrationCode is set to false, a using statement for using HotChocolate.Execution.Configuration; is added to the top of the generated file, which requires another package reference to work.
  3. Again, even though GenerateRegistrationCode is set to false, it generates the HotChocolateTypeModule.g.cs file with registrations, also with a using HotChocolate.Execution.Configuration; statement
  4. Data loader method signatures require the use of IReadOnlyList<T>, otherwise it becomes a CacheDataLoader. In our codebase we generally use the less specific IEnumerable<T>, so it's very easy to end up with a different data loader than you expect. Similarly, it requires the return type to be IReadOnlyDictionary<,> and can't handle the less specific IDictionary<,>
  5. Nullability doesn't seem to flow through correctly. If the return type of the method is IReadOnlyDictionary<int, SomeType?> it generates it as IReadOnlyDictionary<int, SomeType>. This results in calls to .LoadAsync not having the potential to be null, which doesn't reflect the real world.

The solution you'd like

The ability to use the DataLoader source generator more independent from anything directly HotChocolate related (In projects that only reference GreenDonut). If possible it would also be great if the source generator lived in a GreenDonut.Analyzers package instead.

mvestergaard avatar Aug 03 '23 13:08 mvestergaard