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

Q: Error: Unexpected arg kind: ObjectValue #100

Closed
fson opened this issue Aug 17, 2015 · 11 comments
Closed

Q: Error: Unexpected arg kind: ObjectValue #100

fson opened this issue Aug 17, 2015 · 11 comments
Assignees

Comments

@fson
Copy link
Contributor

fson commented Aug 17, 2015

I tried to pass an input object as an argument to a field, which resulted in following error: Unexpected arg kind: ObjectValue.

The fragment in question looks like this:

fragment on Movie {
  id,
  credits(orderBy: {field: "name", order: ASC}) {
    edges {
      node {
        name
      }
    }
  }
}

The error is thrown here, in GraphQLPrinter.js. It seems only builtin scalar types and variables are supported by GraphQLPrinter. Does this mean the only way to pass an input object in Relay is to write credits(orderBy: $orderBy) and pass it in the variables instead?

@josephsavona
Copy link
Contributor

The main reason we didn't support input objects right off the bat is that it's possible for them to contain nested variables, which complicates processing of arguments at runtime. It isn't too difficult to add support for them, but the simple workaround is a variable as you suggested.

Example nested variable:

  ...
  credits(orderBy: {field: "name", order: $order}) {...}

@fson
Copy link
Contributor Author

fson commented Aug 18, 2015

Thanks for the explanation @josephsavona!

A quick follow up question: I changed this to a variable like this:

module.exports = Relay.createContainer(Movie, {
  initialVariables: {
    orderBy: { field: 'name', order: 'DESC' },
  },
  fragments: {
    movie: () => Relay.QL`
      fragment on Movie {
        id,
        credits(first: 10, orderBy: $orderBy) {
          edges {
            node {
              name
            }
          }
        }
      }
    `,
  },
});

Now I'm getting an error on the server: Argument "orderBy" expected type "ReindexOrderBy" but got: {field: "name", order: "ASC"}. (That is, the schema expects an enum ASC but Relay printed it as a string.)
Is printRelayQuery not printing enums inside input objects a known bug or should I open a separate issue about that?

@josephsavona
Copy link
Contributor

This is a known limitation - we didn't have time to add support for printing enums inside input objects. Again, this isn't difficult but it requires adding quite a bit more metadata about arguments in order to print them correctly.

cc @leebyron @dschafer is there anything we can do to make it easier for tools to autogenerate valid inputs?

@fson
Copy link
Contributor Author

fson commented Aug 18, 2015

@josephsavona I noticed the enum values sent separately in the request body using the variables property would already work. When does Relay send variables separately and when does it bake them in the query string?

I wonder if I could somehow force them to be sent separately and use this to work around the fact that enums can't be printed. If not, I'll just try to avoid enums in input objects for now.

@josephsavona
Copy link
Contributor

Relay inlines all variables in queries in order to allow the same fragment to be used with different values. The variables HTTP query param ends up being ignored (because no variables appear in the query) - they're only useful for debugging what the root variables were.

For an example of why inlining is necessary, consider the ProfilePicture component. It might appear in multiple places in the application hierarchy, each referencing the same $size variable, but with different values:

// Parent component
foo: () => Relay.QL`
  fragment on Story {
    author {
      ${ProfilePicture.getFragment('photo', {size: 64}),  # large photo for author
    },
    comments {
      ...
         ${ProfilePicture.getFragment('photo', {size: 32}),  # small photo for commenters
  }
`
// ProfilePicture:
photo: () => Relay.QL`fragment on User { 
  profilePicture(size: $size) {...}
}

The solution for enums within object values is to construct a new variable for every input object, so that it can be sent as plain JSON instead of an inline value.

@fson
Copy link
Contributor Author

fson commented Aug 19, 2015

Thanks for the explanation @josephsavona. That makes sense.

Just making sure I understood the proposed solution for supporting enums within object values, if we were to implement that the fragments and variables sent to the server would be something like this?

fragment _efda7a5a on Story {
  author {
    ..._e8b7be43
  }
  comments {
    ...
        ..._71beeff9
  }
}
fragment _e8b7be43 on User {
  profilePicture(size: $size_6b9df6f) {...}
}
fragment _71beeff9 on User {
  profilePicture(size: $size_98dd4acc) {...}
}

variables (with generated unique names):

{
  "size_6b9df6f": 64,
  "size_98dd4acc": 32
}

@josephsavona
Copy link
Contributor

Heads up that I'll send a PR for this soon.

@josephsavona
Copy link
Contributor

I've started adding support here - https://github.com/josephsavona/relay/commits/inputobjects - but this will also require some upstream changes. printRelayQuery needs to return {text, variables} instead of the text alone.

@fson
Copy link
Contributor Author

fson commented Aug 21, 2015

@josephsavona Looks good so far!

Noticed printRelayQuery is in __forks__. What is the "canonical" repo for it?

@clentfort
Copy link

Correct me if I'm wrong: __forks__ contains modules that differ from facebooks internal version of relay. Since facebook does not yet use the RFC version of GraphQL internally they need a different printer for their queries.

@josephsavona
Copy link
Contributor

Yup, that's is. This is the same Relay we use internally and the forks account for the (minor) differences in GraphQL versions.

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

4 participants