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

Added experimentalFragmentVariables compatibility. #167

Merged
merged 3 commits into from
Apr 19, 2018

Conversation

lucasconstantino
Copy link
Contributor

I'm experimenting with fragments on react-apollo-fragments and have reached a point to deal with fragment variables. The folks at the core of graphql-js project have added some experimental parsing for this, and I would love to use it in my own library. But as I use graphql-tag, as as this library does not export the full parser API below it, I'm decided to pull-request.

You can better understand what I'm looking for here https://github.com/graphql/graphql-js/blob/master/src/language/parser.js#L99-L115

I tried to find a workaround, but I found no way other than replace graphql-tag usage.

  • feature
  • blocking
  • docs

@ghost ghost added the feature New addition or enhancement to existing solutions label Apr 2, 2018
@jnwng
Copy link
Contributor

jnwng commented Apr 6, 2018

thanks @lucasconstantino — i didnt know there were runtime flags for the graphql parser, it seems like something that we can patternize to make it easier to flag all of these.

one question i have before i merge: what's the use case for enabling / disabling this flag on the fly? was that mainly done for testing purposes? (just trying to see if that's generally a pattern for actual usage)

src/index.js Outdated
function parseDocument(doc) {
var cacheKey = normalize(doc);

if (docCache[cacheKey]) {
return docCache[cacheKey];
}

var parsed = parse(doc);
var parsed = parse(doc, { experimentalFragmentVariables });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unfortunately we're not compiling this file just yet, so im going to make this an explicit object

@lucasconstantino
Copy link
Contributor Author

lucasconstantino commented Apr 6, 2018

@jnwng I'm currently working on making a first-class component Fragment for react-apollo. It's a work in progress, and pretty much a proposition to react-apollo itself, but it will rely on parameterized fragments such as:

fragment NamedFragment ($arg: String!) on Type {
  field (arg: $arg)
}

You can see the work in progress here: https://github.com/lucasconstantino/react-apollo-fragments

@lucasconstantino
Copy link
Contributor Author

Just for reference: there is a long discussion happening on the community since 2016 here: graphql/graphql-spec#204

@erickreutz
Copy link

erickreutz commented Jul 19, 2018

Can someone help me better understand how to use these fragment variables? The documentation is pretty sparse and doesn't elude to how to use the feature entirely.

I'm under the assumption that I can do something like the following:

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

I do this and I get the following error:

Syntax Error: Expected Name, found (

Am I missing something?

EDIT:

Seeing this graphql/graphql-js#1141 it appears that the fragment variables just allow you to pass them to the query as variables without explicitly defining them on said query which is a very underwhelming feature.

@axelhzf
Copy link

axelhzf commented Oct 10, 2018

@erickreutz do you know how to make it work? Do I need to make any change on the backend to understand this kind of queries? I am getting this error from the backend GraphQL error: Syntax Error: Expected "on", found (

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New addition or enhancement to existing solutions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants