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

DataFetchingFieldSelectionSet look ahead with aliases #1545

Closed
elxnis opened this issue May 20, 2019 · 9 comments
Closed

DataFetchingFieldSelectionSet look ahead with aliases #1545

elxnis opened this issue May 20, 2019 · 9 comments
Milestone

Comments

@elxnis
Copy link

elxnis commented May 20, 2019

Hi,

It looks like qualifiedName values for fields on DataFetchingFieldSelectionSet are using aliased names instead of schema/type/field names and hence it is hard/impossible to do look ahead to resolve nested fields, boolean contains(String fieldGlobPattern) also matches only aliases and not actual field names. Currently I am using QueryTraversal with my QueryVisitor to build these fields. Would it be possible to add another field or method on fields to have correct path or is there easier way to translate them, so i don't need to do full traversal again?

Thanks,
Reinis

@bbakerman
Copy link
Member

Thanks for reporting. I think field aliases got lost in the design wash here.

We should use the canonical field name for server side fetcher code.

We will look into this in the next release

@bbakerman bbakerman added this to the 14.0 milestone Jun 19, 2019
@andimarek
Copy link
Member

@bbakerman another issue to keep in mind when we look at using the query execution tree for the field selection.

@andimarek andimarek removed this from the 14.0 milestone Jan 16, 2020
@elxnis
Copy link
Author

elxnis commented Mar 27, 2020

Any suggestions if I want to contribute for a fix for this? Where should I start looking?

@hameno
Copy link

hameno commented Apr 22, 2020

Just stumbled upon this issue. Makes it hard for us to optimize our fethers as the queries could use arbitrary aliases

@aoudiamoncef
Copy link
Contributor

Hi @hameno,

    public boolean isFieldRequested(final DataFetchingFieldSelectionSet fetchingFieldSelectionSet) {
        return fetchingFieldSelectionSet.getFields().stream()
                .filter(selectedField -> "parent".equals(selectedField.getName()))
                .map(selectedField -> selectedField.getSelectionSet().getFields())
                .flatMap(List::stream)
                .filter(graphQLFieldDefinition -> "child".equals(graphQLFieldDefinition.getName()))
                .map(selectedField -> selectedField.getSelectionSet().getFields())
                .flatMap(List::stream)
                .anyMatch(selectedField -> "name".equals(selectedField.getName()));
    }

@glasser
Copy link
Contributor

glasser commented Aug 21, 2020

As a stop-gap, it would at least be nice if the docs on DataFetchingFieldSelectionSet explained that all the field names in its API are response field names (possibly aliased). Would such a PR be accepted, or is the idea that the semantics may actually change?

@aoudiamoncef
Copy link
Contributor

Hi @bbakerman @andimarek,
Any news about this issue ?
I could help by working on it, but i need some help.
Thanks

@bbakerman
Copy link
Member

We are working right now on a better sub selection mechanism.

If you look at master right now there is

public class NormalizedField {
    private final String alias;
    private final Map<String, Object> arguments;
    private final GraphQLObjectType objectType;
    private final GraphQLFieldDefinition fieldDefinition;
    private final List<NormalizedField> children;
    private final boolean isConditional;
    private final int level;
    private NormalizedField parent;

actually we are NOT going to make this API but rather fold it into the graphql.schema.SelectedField that exists today

It will include the alias in its data. But it wont be used in the glob matching

@bbakerman bbakerman added this to the 16.0 milestone Oct 13, 2020
@andimarek
Copy link
Member

done

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

6 participants