ChilliCream / graphql-platform

Welcome to the home of the Hot Chocolate GraphQL server for .NET, the Strawberry Shake GraphQL client for .NET and Banana Cake Pop the awesome Monaco based GraphQL IDE.
https://chillicream.com
MIT License
5.03k stars 724 forks source link

Require non-null `first` when using `RequirePagingBoundaries=true` and `AllowBackwardPagination=false` #6926

Open cmeeren opened 3 months ago

cmeeren commented 3 months ago

Product

Hot Chocolate

Is your feature request related to a problem?

I am using UsePaging. I am returning a big IEnumerable and want to require the client to paginate the request, hence RequirePagingBoundaries=true. At the same time I want to be able to move to offset-based DB pagination later without breaking the client, which means I must disallow backward pagination, hence AllowBackwardPagination=false.

Using this combination of parameters, the query has a first parameter and an after parameter. However, both of these are nullable/optional in the schema.

The solution you'd like

There is the possibility of an improvement here: With this combination of parameters, first is required and must be non-null, otherwise HotChocolate returns an error. This should be reflected in the type system, changing first from Int to Int!.

(My assumption is that this is not normally done for RequirePagingBoundaries=true because this requires either first or last, but in combination with AllowBackwardPagination=false, the first parameter is required.)

cmeeren commented 2 weeks ago

I see this was labeled Area: F#, but this is not F#-specific. Here is C# code to repro:

var builder = WebApplication.CreateBuilder(args);
builder.Services.AddGraphQLServer().AddQueryType<Query>();
var app = builder.Build();
app.MapGraphQL();
app.Run();

public class Query
{
    [UsePaging(RequirePagingBoundaries = true, AllowBackwardPagination = false)]
    public IEnumerable<string> Test() => new[] { "a" };
}

Here is part of the schema this produces:

type Query {
  test(
    """
    Returns the first _n_ elements from the list.
    """
    first: Int

    """
    Returns the elements in the list that come after the specified cursor.
    """
    after: String
  ): TestConnection
}
cmeeren commented 2 weeks ago

I just discovered that there's no workaround: Applying [<GraphQLNonNullType>] to the first parameter in this case has no effect. It's still nullable.