How to define field of type [string!] with expression

[gao-artur]

I'm trying to use expressions instead of verbose syntax to describe the fields. How do I convert this definition to get [string!] type?

Field<ListGraphType<NonNullGraphType<StringGraphType>>>("Names").Resolve(x => x.Source.Names);

This leads to [string]!

Field(x => x.Names);

This to [string]

Field(x => x.Names, nullable: true);

I want a nullable list with non-nullable elements. The only option I found is defining the type parameter:

Field(x => x.Names, type: typeof(ListGraphType<NonNullGraphType<StringGraphType>>));

But it kind of misses the goal because typing-wise, I only save the "Names" string.

[gao-artur]

Using NRT doesn't help either.

[Shane32]

I don't have an answer here. I run into similar issues using the ID graph type. Usually I create an extension method within my application to facilitate the goal. So for example:

this.IdField(x => x.Id);   // uses custom extension method

[gao-artur]

Yep, same problem with ID. I think analyzing NRT should be a good solution for handling collections. And maybe adding IdField as a build-in method? It looks very useful.

[Shane32]

We could make this change for v8, so that by default all members directly referenced are analyzed for NRT. But analyzing NRT isn't possible for computed fields or anything that's not a direct member access, as the generic type in the Field<T> will not carry any NRT data. So the only way is to, like we do for name detection, look for directly referenced members (e.g. x => x.Name) and pull NRT data from the member.

Note that I expect this would be a very major breaking change. One example:

class MyClass
{
    public string? Name { get; set; }
}

Field(x => x.Name);  // v7: non-null .... v8: nullable

// how to fix???   'nullable' argument already defaults to 'false'

We may need to change method signatures (e.g. bool? nullable), and we may need to design a global switch to revert behavior.

Keep in mind also that GraphQL encourages all fields to be nullable, so that errors do not bubble. I don't design my graphs this way, but it certainly some developers do so, especially if they rely on errors not-bubbling in their client applications.

[gao-artur]

Yes, I thought about all these problems as well. But they look solvable. As you have said, the global switch can be introduced to revert the behavior. The default value from the nullable argument can be removed, and additional overload can be introduced without this argument. This overload will call the overload with nullable depending on NRT analysis + global switch value. Also I don't care about generic usages, only the lambda that returns property. Whoever uses a generic solution probably passes the type parameter anyway. I can try adding code fixes to remove type parameter where it will produce a similar result based on NRT. So it will be possible to remove the global switch in v9. Maybe the "local" switch can be added to allow granular migration type-by-type.

[Shane32]

Well, I think I'd need to see a PR / POC before I would commit to approving such a PR. It sounds good in theory, I agree...

If you want to work on it, I can show you where our existing NRT analysis code exists so we are not duplicating any of that logic. Then we can review once we have a POC.

[Shane32]

Maybe the "local" switch can be added to allow granular migration type-by-type.

Currently this probably wouldn't be so easy to implement. I don't think this feature is specifically needed.

[Shane32]

Another idea: implement this feature for v8, with a global switch, but leave the global switch in 'legacy mode' for now. And switch it for v9. I don't know that this really is any better, however, because if someone upgrades without thinking about migration changes, then they will be just as surprised when they upgrade to v9 later as they would be switching to v8 now.

In the past, I've done this with minor versions prior to a major release. So like if it was possible to implement these changes now in v7, then we could do so, with the global switch in 'legacy' mode, letting people prep for the change immediately. And change the switch for v8. But I don't know if it's possible to implement now in v7 without a breaking change. Something to look at.

[gao-artur]

I'll prepare the POC