Skip to content
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

Question: support for @include(if: $foo) directive #302

Open
laynor opened this issue Nov 16, 2021 · 8 comments
Open

Question: support for @include(if: $foo) directive #302

laynor opened this issue Nov 16, 2021 · 8 comments

Comments

@laynor
Copy link

laynor commented Nov 16, 2021

Does ferry support queries like the one below?

query Foo($shouldFetchBar: Boolean = true) {
  moo {
    baz {
      bar @include(if: $shouldFetchPaymentSound) {
         foobar
      }
    }
  }
}

My query works correctly with the Apollo client app, while ferry returns a null response.data field.
I wonder if I'm doing something wrong - I noticed ferry silently returns a null data field even in the case of union fields if the query is not specifying the __typename field, e.g.

query Foobar { 
   foo { 
      ... on Bar { 
              moo
      }
      ... on Baz { 
              boo 
     }
}

won't work (returning a null data field), while

query Foobar { 
  foo { 
  ... on Bar { 
    __typename
    moo
  }
  ... on Baz { 
    __typename
    boo 
  }
}

will work correctly.

@smkhalsa
Copy link
Member

Are these two different questions?

Do you get any errors (either graphql errors or link errors)?

@laynor
Copy link
Author

laynor commented Nov 25, 2021

No errors, just null responses. The question is one (releated to the @include directive) - I was just pointing out that silently returning null also happens in other cases where Ferry doesn't know how to type the response. I don't know if it's a type issue in the case at hand - I just get a null.

@TanguyChevillardNovade
Copy link

When using the @include directive, the data objects seems to always be built, but only populated if they should be included (otherwise no request is sent to the backend).
Ferry doesn't return null for me, but if the query has non nullable fields, I get an error when it shouldn't be included.

Example of query:

query getData ($shouldGetData: Boolean) {
	getDataRequest() @include(if: $shouldGetData) {
		totalDataCount
		dataElements {
			name
		}
	}
}

Types in the schema:

type Query {
	  forms(): FormConnection!
}

type FormConnection {

  """Array of elements."""
  dataElements: [FormEdge!]!

  """Fetch total count of records"""
  totalDataCount: Int!
}

@eranganovade
Copy link

eranganovade commented Dec 15, 2023

@laynor, @smkhalsa, @knaeckeKami Any update on this? We would also like to see @include and @skip directives work on ferry.

e.g.: We would like to see authors property as a nullable field in the generated classes for posts (i.e.: GPostsData).

Query:

query Posts ($includeAuthors: Boolean!) {
  posts {
    id
    title
    text
  }
  authors @include(if: $includeAuthors) {
    id
    name
  }
}

GraphQL schema

type Query {
  posts: [Post!]!
  authors: [Author!]!
}

Expected class

abstract class GPostsData implements Built<GPostsData, GPostsDataBuilder> {
  GPostsData._();

  factory GPostsData([Function(GPostsDataBuilder b) updates]) = _$GPostsData;

  static void _initializeBuilder(GPostsDataBuilder b) =>
      b..G__typename = 'Query';
  @BuiltValueField(wireName: '__typename')
  String get G__typename;
  GPostsData_posts get posts;
  GPostsData_authors? get authors;
  static Serializer<GPostsData> get serializer => _$GPostsDataSerializer;
  Map<String, dynamic> toJson() => (_i1.serializers.serializeWith(
        GPostsData.serializer,
        this,
      ) as Map<String, dynamic>);
  static GPostsData? fromJson(Map<String, dynamic> json) =>
      _i1.serializers.deserializeWith(
        GPostsData.serializer,
        json,
      );
}

But we get GPostsData_authors? get authors as GPostsData_authors get authors here.


If this is something we need to add to the ferry, we would be happy to work on it given some guidance on where to look.

@knaeckeKami
Copy link
Collaborator

Hi!

There is an open draft PR where this feature has been started:

gql-dart/gql#357

It's in gql_code_builder though, a dependency of ferry_generator.

I never got to finish finish it, there are some merge conflicts and debug prints still there, and maybe some edge cases missing, but I don't fully remember.

@eranganovade
Copy link

eranganovade commented Dec 15, 2023

@knaeckeKami Nice, I will try to work on this when I have some free time. If you remember the edge cases, please comment here as well.

@knaeckeKami
Copy link
Collaborator

I took some time to fix the merge conflicts.

I also remembered what the issue was, the current state only works on fields, not on fragments, and to make this work, a bigger refactor is needed. (instead of just collecting the selectionNodes, we also need to track if it's in a context that has a skip or include directive to know whether to force the type to be nullable)

@eranganovade
Copy link

I took some time to fix the merge conflicts.

I also remembered what the issue was, the current state only works on fields, not on fragments, and to make this work, a bigger refactor is needed. (instead of just collecting the selectionNodes, we also need to track if it's in a context that has a skip or include directive to know whether to force the type to be nullable)

Great, thank you for the update. 🥂

I have put this issue in our backlog as something we should implement. Once we add that into one of our sprints, I'll update here. Since this issue has been here for more than 2 years, I guess it will remain open until we revisit it again :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants