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

Parameterized fragments #204

Open
rmosolgo opened this issue Aug 23, 2016 · 23 comments
Open

Parameterized fragments #204

rmosolgo opened this issue Aug 23, 2016 · 23 comments
Labels
💭 Strawman (RFC 0) RFC Stage 0 (See CONTRIBUTING.md)

Comments

@rmosolgo
Copy link

rmosolgo commented Aug 23, 2016

A recent talk about Relay 2 suggested adding arguments to query fragments via directives.

I think first-class support for arguments in fragments would give a few benefits:

  • Smaller variable scope: instead of checking each usage of a fragment (which may be deeply nested) to determine what variables are available, a fragment would be its own scope. You could determine available variables by the fragment definition alone.
  • More flexible reuse: since a fragment wouldn't depend on its query's variables, fragments could be applied to queries whose variables had different names. They could also receive literal values when the query has no variables.

For example, a fragment might be defined with variables:

fragment avatarFields($thumbSize: Int = 40) on Image {
  thumbURL(size: $thumbSize)
  uploadedAt
}

The spread with different values:

{
  person(id: 1) {
    avatar {
      ... avatarFields(thumbSize: 80)
    }
  }
}
query findPerson($personId: Int!, $avatarSize: Int!) {
  person(id: $personId) {
    avatar {
      ... avatarFields(thumbSize: $avatarSize)
    }
  }
}

To be honest, I don't have a use case. But this seems like a really solid feature, and as a library maintainer, I'd rather add this language feature than implement some new directives! (Or, would those directives be "compiled out"?)

Technically, this could even be backwards compatible. Fragments could also use variables from their outer scope.

What do you think, would this be an improvement? Have I overlooked any downsides or complexities?

@dschafer
Copy link
Contributor

dschafer commented Aug 23, 2016

This is a great discussion point; we talked about it for months before we open-sourced, and even now I think there are good arguments either way. To spark the discussion, I've dug up a document that @leebyron wrote up to summarize the options around variables in general (since there were a bunch of things we considered).

Idea

  • Fragments should be able to be validated in isolation. Therefore any variables it uses it must describe their types.
  • Queries should describe all the variables they require in order to execute.
  • Fragments should not be significantly more difficult to use as a result of declaring args.
  • Support Relay query/fragment variables, variadic fragments.

Explicit Declaration

Fragments define args, and those args must be provided by the reference.

query Foo(arg: Number) {
   ...A(arg: $arg)
}

fragment A(arg: Number) {
  field,
  ...B(arg: $arg)
}
``
fragment B(arg: Number) {
  field(arg: $arg)
}

Pros:

  • Each fragment can be validated in isolation. Only changing the argument definitions of a fragment would require finding all of it's references and re-validating them.

Cons:

  • This is very annoying when you have very deeply nested fragments and a leaf fragment needs a variable. That variable must be explicitly threaded through every fragment level. For some variables, this may be preferable, but for some (ex. device characteristics like scale) it's very disruptive.

Global Declaration

All arguments defined in a fragment are implicitly global (passing down is automatic). Query must describe all global variables.

query Foo(arg: Number) {
   ...A
}

fragment A {
  field,
  ...B
}

fragment B(arg: Number) {
  field(arg: $arg)
}

Pros:

  • No longer have the annoying threading through deep fragment references. Middle fragments can remain isolated and unaware of variable usage.

Cons:

  • Changing a fragment argument definition requires climbing all the way to all root queries to ensure the variable of the same name is defined with the correct type.
  • Fragment arguments will need fairly unique names (ex. NOT “size”) to avoid conflict.
  • Unclear if a query argument can ever be safely removed, because it's laborious to find all fragment references which reference it. Tooling can assist here.

Explicit Globals

All arguments are defined in a fragment, but must explicitly declare they are put into and come from a global scope.

query Foo(global arg: Number) {
   ...A
}

fragment A {
  field,
  ...B
}

fragment B(global arg: Number) {
  field(arg: $arg)
}

Pros:

  • Explicit that some variables defined in a query are globally accessible to all child fragments
  • No threading of these global variables.
  • Leaf fragments limit the args which require climbing to the root query to validate

Cons:

  • “global” likely to become the default, which just adds more shit to type.
  • “global” keyword, ugh. Trying to minimize keywords.
  • Still must climb to root query in order to validate.

Implicit Globals

All arguments are defined in a fragment, but the root query defines nothing.

query Foo {
   ...A
}

fragment A {
  field,
  ...B
}

fragment B(arg: Number) {
  field(arg: $arg)
}

Pros:

  • No threading of variables.
  • No explicit work of writing requirements at Query level.

Cons:

  • Changing a fragment argument definition still requires climbing to all root queries to ensure variables are unique and types do not collide.
  • It may not be obvious if two fragments use an argument of the same name with the same type but the two arguments have different semantic uses.
  • Server executor cannot early-exit if params are not known before execution without first walking a query.

Always Globals, Always defined (this is what GraphQL today does)

All params are defined at the query level and can be used by any fragment.

query Foo($arg: Number) {
   ...A
}

fragment A {
  ...B
}

fragment B {
  field(arg: $arg)
}

Pros:

  • Per query it is explicitly clear to a human reader what params are necessary to provide and of what types.
  • No threading of params necessary.
  • Server-side param coercion and validation can occur before any querying begins.
  • Client-side param validation error messages can be more clear.

Cons:

  • Fragments are no longer variadic.
  • When seeing a param in use, it may be unclear what type it will be as that information is defined at the query level.
  • Must write out all params for each query. (Anecdotally, an exercise to do this for news feed was not difficult).
  • The addition of new params for very common fragments would result in needing to update every referenced query. (Anecdotally, an uncommon operation)

Always Globals, Implicit (This is what the original GraphQL at FB did)

All params are global, never defined, and can be used by any fragment.

query Foo {
   ...A
}

fragment A {
   ...B
}

fragment B {
   field(arg: $arg)
}

Pros:

  • Least amount of query text to type.
  • New params can be added at a fragment level in isolation of the queries they affect.

Cons:

  • Fragments are no longer variadic.
  • When seeing a param in use, it may be unclear what type it will be as that information is not defined anywhere.
  • Unclear what params must be sent alongside any given query without tooling.
  • Server-side param coercion occurs during execution, errors may result after some parts of the query have already executed.
  • Client-side param validation error messages could be confusing - in a type conflict it's unclear which type is intended and which is accidental.
  • Accidental misspellings of a param go undetected at static-time.

@dschafer
Copy link
Contributor

dschafer commented Aug 23, 2016

As I understand it, a syntactic version of what Relay2 does would be something like this:

Globals, Explicit, fragment arguments

All params are defined at the query level and can be used by any fragment. Additionally, fragment arguments can be defined and a caller can provide them.

query Foo($arg: Number) {
   ...A
}

fragment A {
   ...B(another: 128)
}

fragment B(another: Number = 64) {
   field(arg: $arg, another: $another)
}

@josephsavona or @wincent, correct me if I'm wrong here.

@dschafer
Copy link
Contributor

As I recall the conclusion of the discussion prior to the launch, we basically thought that parameterized fragments sounded like a good concept, but that we didn't have a sense of how commonly they would be used, and because of that, the tradeoff of taking on the additional complexity in the core didn't seem worth it.

The Relay2 approach seems promising, though, and one of the main reasons we wanted to have directives in the core was to enable experimentation like this. Once we see more use of Relay2 (or other approaches to parameterized fragments), I definitely think we should revisit this decision with the additional knowledge we've gained and see if there's a better option.

@stubailo
Copy link
Contributor

The only issue with Relay2-style experimentation, compared to having stuff like this in the spec, is that it breaks the GraphiQL experience completely. You can no longer debug your queries in an interactive environment and then simply paste them in if all of your queries rely on custom extensions.

@josephsavona
Copy link
Contributor

josephsavona commented Aug 23, 2016

The only issue with Relay2-style experimentation, compared to having stuff like this in the spec, is that it breaks the GraphiQL experience completely.

@stubailo The same is true of ES6 code pasted into an older browser. If as a community we limit our language experimentation to only what works in current GraphiQL, we won't be able to experiment much at all.

The question is whether we can continue to provide interactive editing environments with new tools. And we can - we could easily build a Relay2 extension to GraphiQL so that people using Relay2 can debug queries. When these features are more established and make their way into the spec, then the need for the custom graphiql would go away. Since GraphiQL can be delivered entirely client-side (querying an existing GraphQL endpoint) and Relay2 compiles down to plain GraphQL, this doesn't even require anything special on the server.

@rmosolgo
Copy link
Author

Relay2 compiles down to plain GraphQL, this doesn't even require anything special on the server

👍 very cool

@stubailo
Copy link
Contributor

Perhaps we need some kind of tool like Babel, so that people can plug in different GraphQL transformations and use them together? Otherwise we might rapidly run into a world where everyone is using a different flavor of GraphQL, which makes it a lot harder to build great tools, share code between different environments, etc.

@stubailo
Copy link
Contributor

To put it a different way, I think custom extensions to GraphQL are great. IMO having stuff in the spec is even better, but I agree that the spec is not the place to experiment/innovate.

But, to take your metaphor about old browsers and JS a different direction:

I think it's fine that we all experiment with new features in JavaScript that browsers don't support, and figure out how to make them work. But it would be very frustrating if we had one flavor of JavaScript for our frontend code, one for backend code, one for test code, etc. The greatest benefit of something like Babel for JS is that you can transpile all of your code and it's tool-independent.

I'm worried about a situation where there is a Relay-specific flavor of GraphQL that can't be easily consumed by other tools like linters, in-browser auto-complete tools, interactive editors, and more. So perhaps the ideal world would be where the Relay query transformation was open-sourced as a separate tool so that the whole ecosystem of possible tools, including GraphiQL, could use it.

@josephsavona
Copy link
Contributor

So perhaps the ideal world would be where the Relay query transformation was open-sourced as a separate tool so that the whole ecosystem of possible tools, including GraphiQL, could use it.

@stubailo We totally agree, and will open-source the Relay2 compiler as soon as we can!

@josephsavona
Copy link
Contributor

josephsavona commented Aug 23, 2016

@dschafer Already covered the main variations of this, but I'll add a bit more detail about how we're thinking about variables in Relay2.

We've found that the encapsulation provided by React components is incredibly useful because it lets developers reason about components in isolation. Developers are able to look at a component and understand how it works without having to read the rest of the codebase - you can reason locally. We feel that it's important to maintain this property with Relay containers, which means that it should be possible to understand container fragments in isolation. This means all variables referenced in a fragment should be defined by that fragment.

In current Relay this is achieved by initialVariables definitions and inlining: container fragment variables end up as literal argument values (at runtime). When designing Relay2 we examined a variety of real use cases for initialVariables and the dynamic equivalentprepareVariables, and found the following:

  • Most variables are constants (initialVariables: {count: 5}) or local constants (initialVariables: {count: DEFAULT_COUNT}, where the const is defined in the same file).
  • Most other variables are dynamic based on the environment, such as initialVariables: {pixelDensity: Device.getPixelDensity()}.
  • A few edge cases in prepareVariables perform logic/math, such as calculating different picture sizes based on the device width and other attributes.

Based on this we're currently planning to support two main options for fragment argument in Relay2:

  • Literal constants
  • Runtime values

Here's an example that demonstrates both:

query ViewerQuery {
  viewer {
    actor {
      ...UserProfile
    }
  }
}
fragment UserProfile on User @argumentDefinitions(
  count: {type: "Int", defaultValue: 10},
  pixelDensity: {type: "Int", provider: "PixelDensity.get"},
) {
  friends(first: $count) { edges { node { ... } } }
  photo(pixelDensity: $pixelDensity) { uri }
}

Note that the pixelDensity variable references a global provider "PixelDensity.get". This allows us to express the fact that we want to reference a runtime value, but we reference that value by name instead of by value. The Relay2 compiler generates a unique variable for each distinct "provider", and synthesizes a query variable for every "provider" that appears in a query. So the above compiles down to:

query ViewerQuery($__PixelDensity_get: Int) {
  viewer {
    actor {
      ...UserProfile
    }
  }
}
fragment UserProfile on User {
  friends(first: 10) { edges { node { ... } } }
  photo(pixelDensity: $__PixelDensity_get) { uri }
}

To run this query, developers provide a resolver that, given a provider name such as "PixelDensity.get", can return the runtime value (provider: string) => mixed.

@mike-marcacci
Copy link
Contributor

I'm curious if there has been any more internal discussion about this. Using Relay modern, I've found it necessary to give globally unique names to my fragment variables, because of the following case.

fragment ReactComponentA_friends on User {
	id
	sessions(first: $ReactComponentA_friends_first, after: $ReactComponentA_friends_after) @connection {
		pageInfo {
			endCursor
			hasNextPage
		}
		edges {
			node {
				...something
			}
		}
	}
}


fragment ReactComponentB_pages on User {
	id
	sessions(first: $ReactComponentB_pages_first, after: $ReactComponentB_pages_after) @connection {
		pageInfo {
			endCursor
			hasNextPage
		}
		edges {
			node {
				...something
			}
		}
	}
}

query UserSummary_user () {
	user {

		# I want the top 100 friends
		...ReactComponentA_friends

		# I want the top 5 pages
		...ReactComponentB_pages		

	}
}

What I would rather do, is:

fragment ReactComponentA_friends on User {
	id
	sessions(first: $first, after: $after) @connection {
		pageInfo {
			endCursor
			hasNextPage
		}
		edges {
			node {
				...something
			}
		}
	}
}


fragment ReactComponentB_pages on User {
	id
	sessions(first: $first, after: $after) @connection {
		pageInfo {
			endCursor
			hasNextPage
		}
		edges {
			node {
				...something
			}
		}
	}
}

query UserSummary_user () {
	user {

		# I want the top 100 friends
		...ReactComponentA_friends(first: 100, after: null)

		# I want the top 5 pages
		...ReactComponentB_pages(first: 5, after: null)

	}
}

@erickreutz
Copy link

Is any progress being made towards this?

Something along the following would be very useful:

query UserInfo {
  currentUser {
    id
    firstName
    ...UserAvatar(size: 100)
  }
}
fragment UserAvatar($size: Int!) on User {
  avatarUrl(size: $size)
}

The current experimental support of variable definitions in fragments simply being hoisted to the parent query leaves much to be desired.

@jlouis
Copy link

jlouis commented Jul 20, 2018

In my GraphQL system, I currently treat a fragment as a (lambda) function. The parameters are inferred through a standard type check inference algorithm. Your example just makes this into an explicit construction where the parameter passing can be made explicit as well.

However, this would mean you have to update the execution phase to include some kind of parameter passing and funcalls as well. Currently, you can get away with keeping the lambda constructions in the type checker only, because in execution the variable environment to draw vars from is static over the query.

I've said for a while that GraphQL is just syntactic sugar for a small total lambda calculus: operations and functions and so are fragments. It would be natural to extend this into the language proper at some point.

(Aside: This also suggests you can compile GraphQL queries into a small Core language which is functional (lambda calculus + primops) and then execute that in the execution phase. I have a strong hunch this is possible, but I have yet to try rewriting my execution phase to do this).

@iamchathu
Copy link

iamchathu commented Mar 4, 2020

We have a use case for a chart We need to get data with custom field name as Day name (Sunday,Monday) same fragment

fragment Sales($date:DateTime){
    sunday:salesByDate(date:$date){
         total
    }
}

We need to use date-fns function inside query to get the day name. How can we do that?

@valoricDe
Copy link

valoricDe commented May 2, 2020

I also got a usecase, but without query variables and rather with fixed fragment values.
Gatsby's image sharper plugin gives results in a query with a request like:

  myHeaderImage {
      url
      file {
        childImageSharp {
          id
          fixed(height: 37) {
            base64
            width
            height
            src
            srcSet
          }
        }
      }
    }

It would be nice if I could create a local value fragment which would pass the given value to a nested function.

  myHeaderImage {
      url
      ...ImageFixed(height: 37)
    }

fragment ImageFixed($params: ImageSharpFilterInput) {
    file {
        childImageSharp {
          id
          fixed($params) {
            base64
            width
            height
            src
            srcSet
          }
        }
      }
}

@gaplyk
Copy link

gaplyk commented Sep 24, 2020

any progress on this?

@acao
Copy link
Member

acao commented Sep 25, 2020

to move this RFC along, you can:

  • add it to the agenda for the monthly working group calls
  • attend the working group calls and promote it
  • provide more use cases
  • create a graphql-js PR to implement/demonstrate the proposal (most important!)

@westonAnchore
Copy link

I have a use case using gatsbyjs/react/graphql where we make countless image queries like this

export const imageFields = graphql`
  fragment imageFields on WpMediaItem {
    altText
    localFile {
      childImageSharp {
        gatsbyImageData
      }
      publicURL
    }
  }
`

in many cases we explicitely set the width a/o hight

export const imageFields = graphql`
  fragment imageFields on WpMediaItem {
    altText
    localFile {
      childImageSharp {
        gatsbyImageData(width: 500, height: 300)
      }
      publicURL
    }
  }
`

which can easily be written as

export const imageFields = graphql`
  fragment imageFields on WpMediaItem {
    altText
    localFile {
      childImageSharp {
        gatsbyImageData(width: $width, height: $height)
      }
      publicURL
    }
  }
`

but becomes problematic when used inside another fragment since there doesn't seem to be a way to pass the variable value from a fragment

export const resourceCardFields = graphql`
  fragment resourceCardFields on WpPost_Resource {
    heading
    description
    type
    ...imageFields
  }
`

it would be great to be able to simple do:

export const resourceCardFields = graphql`
  fragment resourceCardFields on WpPost_Resource {
    heading
    description
    type
    ...imageFields($width: 500, $height: 300)
  }
`

I may be wrong, but it seems like the proposed solution would enable this type of fragment nesting which is extremely useful when you have hundreds and hundreds of lines of graphql queries.

@async3619
Copy link

lol It's been 6 years since this issue was first raised, and it's surprising that there hasn't been any noticeable change yet.

@rajington
Copy link

rajington commented Jul 18, 2022

Although standardizing would be great, might be worth mentioning/re-mentioning:

@icetrust0212
Copy link

Oh... So it isn't supported yet?

@amansinghsom

This comment was marked as off-topic.

@DoisKoh

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💭 Strawman (RFC 0) RFC Stage 0 (See CONTRIBUTING.md)
Projects
None yet
Development

No branches or pull requests