Skip to content

[RFC] Spread field names #484

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

Closed
Wazner opened this issue Jul 27, 2018 · 4 comments
Closed

[RFC] Spread field names #484

Wazner opened this issue Jul 27, 2018 · 4 comments
Labels
🗑 Rejected (RFC X) RFC Stage X (See CONTRIBUTING.md)

Comments

@Wazner
Copy link

Wazner commented Jul 27, 2018

I'm experimenting with GraphQL and I've run into a scenario which currently isn't possible. This could be solved in libraries, but I think language support would allow for better adoption.

My goal is to allow the user to select what fields they want to see in the user interface. This could be solved using dynamic queries, but that won't work with persisted queries for example. My proposal is to allow array variables to be used as fragments.

query MyQuery($fields: [String!]) {
  hero {
    ...$fields
  }
}

When fields is for example: ["id", "name"] the following response would be returned:

{
  "data": {
    "hero": {
      "id": 5,
      "name": "My hero"
    }
  }
}

I'm not entirely sure how we could incorporate nested objects into this, or if we would even want something like that.

@mjmahone
Copy link
Contributor

There are some issues with allowing people to spread fields as input values. The most obvious to me is that your query cannot be statically verified: I have no idea when I'm sending my query whether the server actually supports the fields I'm spreading in.

If we were to enable something like this, we'd probably want to make it either much simpler or much more detailed. The simpler solution would be to pass in the name of a fragment. This way you could at least statically verify that the fields you might ask for exist on the server. Doing that, though, we run into problems: what happens if the fragment you want to include has variables used inside it? You now need a way to associate variables to a fragment (which GraphQL does not currently have), or you need to ensure no variables are used in any of your "input spread-able" fragments. This also runs into problems for people who are persisting queries: how do you know which fragment to execute, purely by its name, when you've persisted 100 different versions of that fragment?

The much more complex alternative would be to somehow change the language to add a new input type that is "dynamically accessible fields". I'd encourage you to read through the "Relay Modern changes" to get an idea for why we used to allow dynamic field spreading in Relay, but chose to remove this ability in the modern version.

@Wazner
Copy link
Author

Wazner commented Jul 30, 2018

I agree that this can get complex quite fast. And I prefer any overhead that comes from a feature like this to not occur when not using it. But the issue remains and I can imagine many data-intensive applications will require field-level dynamic queries.

An alternative way is to make something like the following work:

type HeroFields {
  field1: Boolean
  field2: Boolean
  field3: Boolean
}

and then:

query HeroQuery($fields: HeroFields!) {
  heroes {
    field1 @include(if: $fields.field1)
    field2 @include(if: $fields.field2)
    field3 @include(if: $fields.field3)
  }
}

This will bloat the query, but that is less of a problem when using persisted queries.
Application developers could make this work with some tooling to generate fragments that include these directives.

The language could assist in this by supporting types based on existing types and/or a directive on the parent.

query HeroQuery($fields: FieldsOf<Hero>!) {
  heroes @include(fields: $fields) {
    field1
    field2
    field3 
  }
}

The nice thing about this is that is uses the existing infrastructure and is probably easier to integrate with other (potential) GraphQL features, like #377 for example.

@mjmahone
Copy link
Contributor

That proposal, to have client-side tooling auto-generate a query with a bunch of @include(if:) directives, seems like a very promising path forward! It also will be substantially less bloated over-the-wire compared to including some string with the field names, as boolean variables can be compressed really well.

I would make a new directive for yourself (like @dynamic_fields), which would then transform into your query with many if values. You'd still probably want to end up with one variable per generated @include, as most GraphQL servers should be able to handle variables-as-indexed-arrays instead of uploading a JSON map, and that would be quite bytes-over-the-wire efficient.

All of the above can be implemented purely using directives and client-side, pre-query-persistence transformation. So I'd highly encourage you to build this feature out! But it probably doesn't belong in the spec.

@leebyron leebyron added the 🗑 Rejected (RFC X) RFC Stage X (See CONTRIBUTING.md) label Oct 2, 2018
@leebyron
Copy link
Collaborator

leebyron commented Oct 2, 2018

I'm going to close this issue as rejected after this helpful discussion. I think "...is that your query cannot be statically verified" is unfortunately the nail in the coffin for this proposal since that would break so many of GraphQL's tools.

@leebyron leebyron closed this as completed Oct 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🗑 Rejected (RFC X) RFC Stage X (See CONTRIBUTING.md)
Projects
None yet
Development

No branches or pull requests

3 participants