Support the @include and @skip directives

[vladar]

I tried to add directives to my GraphQL query in Container, like:

Relay.createContainer(Component, {
  initialVariables: {
    showField1: true
  },
  fragments: {
    test: (variables) => Relay.QL`
      fragment on SomeType {
        field1 @include(if: $showField1) {
          ...
        }
        field2 @skip(if: $showField1) {
          ...
        }
      }
    `
  }
})

But they were cut out by Relay. Am I missing something?

[josephsavona]

@vladar Thanks for filing an issue - Relay doesn't support arbitrary directives (it uses the @relay(...) directive for some metadata). This is definitely something we would like to support going forward.

[vladar]

Thanks for clarifications. But @include and @skip are not arbitrary. They are part of GraphQL specification - http://facebook.github.io/graphql/#sec--skip

[josephsavona]

@vladar touche. yes, we should support these. I'll update the title of this issue to reflect this.

[AndrewIngram]

Is it necessary for Relay to be cutting out the directives? Presumably if they were left intact it would be possible to support arbitrary ones as well.

[wincent]

@AndrewIngram, that's a good point. I think the reason we've defaulted to a conservative policy here so far is that directives can drastically alter the semantics of a query, and Relay very much wants to be able to both statically analyze component queries and dynamically construct new ones at runtime (for example, to compute minimal diff queries in order to request only data that is not already present in the store).

In addition to that, directives as they exist in open source GraphQL are a relatively new addition to the language, so support for them is still pretty recent and basic.

But yes, we should not only handle directives in the spec, but also consider what the implications of "passing through" any other directives that we don't explicitly support might be.

[AndrewIngram]

Incidentally, this is the use case I'm trying to support:

const UserSearchContainer = Relay.createContainer(UserSearch, {
  initialVariables: {
    query: null
  },
  fragments: {
    viewer: (variables) => Relay.QL`
      fragment on User {
        first_name
        users (
          q: $query
          first: 10
        ) @include(if: $query) {
          edges {
            node {
              ${UserSearchList.getFragment('users')}
            }
          }
        }
      }
    `,
  }
});

At the moment, I'm handing this by making the users connection return a promise with an empty array if there's no query. But ideally we wouldn't even be hitting that part of the graph at all

[grydstedt]

+1

[josephsavona]

This is fixed in master and will be available in the next release :-)

[emilsjolander]

Did this make it into the latest release? I'm currently using 0.3.2 but it seems to be ignoring the include directives still.

[kassens]

cc @josephsavona

[aweary]

Any word on this? @josephsavona doesn't seem to be working for me either.

[guigrpa]

@josephsavona I can confirm it: the @include directive is apparently ignored

[josephsavona]

@guigrpa @aweary Do you mean that the queries are sent to the server without directives that were specified in a container fragment? Or are the directives there but are being ignored (e.g. Relay gives you the data even though the directive says to skip, or vice-versa)?

[aweary]

From what I saw the babel plugin just returned an error for the fragment, which was then called.

[josephsavona]

@aweary can you provide the fragment itself and the error message & stack (if present)?

[guigrpa]

Here's what I am seeing: this is the fragment:

  initialVariables: {
    projectId: null,
  },
  fragments: {
    viewer: () => Relay.QL`
      fragment on Viewer {
        ${SelectProject.getFragment('viewer')}
        node(id: $projectId) @include(if: $projectId) {
          ${ProjectDetails.getFragment('project')}
        }
      }
    `,
  },

The query that's being sent includes the node field without an id arg, so the GraphQL server returns an error: "Field \"node\" argument \"id\" of type \"ID!\" is required but not provided." The Relay client shows a warning (Warning: fetchWithRetries: HTTP error, retrying.) and then retries and retries...

[josephsavona]

@guigrpa I see what's happening - this is not about directives.

Relay requires that the node(id: $id) field be on the root - see the Object Identification guide. It appears that your schema nests this field inside a Viewer type, and this nesting is not supported by Relay. Containers should fetch data for any object of a type (in this case the fragment should probably be project => Relay.QLfragment on ProjectDetail { ... }``). See the routes guide for more information about this important distinction.

As for why it isn't working now, GraphQL is rejecting the query since $projectId is null. A better pattern would be to have separate fragments for viewer and project (which would use the node root query).

[aweary]

@josephsavona I should note that the error I was getting was due to using the @include directive in an inline fragment which I already noted here: #443

If I use it on each field it seems to be working just fine 👍

[josephsavona]

Thanks everyone for confirming that directives themselves are working as-expected. I'll leave this issue closed.

@guigrpa - please feel free to ask follow-up questions about the node field and route/container distinction on Stack Overflow