makeExecutableSchema should permit directives

[voodooattack]

Since the schema language now allows directives, makeExecutableSchema should pass on the directives argument to the GraphQLSchema constructor.

type GraphQLSchemaConfig = {
  query: GraphQLObjectType;
  mutation?: ?GraphQLObjectType;
  subscription?: ?GraphQLObjectType;
  types?: ?Array<GraphQLNamedType>;
  directives?: ?Array<GraphQLDirective>;
};

[helfer]

@voodooattack indeed, can you make a PR please? 🙂

[voodooattack]

@helfer I was thinking maybe something like an attachDirectivesToSchema function?

The problem I see is that there are two ways to use directives, the first is before the schema is constructed (AST manipulation using directives like decorators, something like this: https://gist.github.com/voodooattack/e12f1e451f9b82bc20cec1a49e44e23a), and the second is to interpret client requests (in which case, the directives would act on the schema and modify the resolvers).

How do you suggest we go about this?

[stubailo]

What would the new directives even do? Would they be passed into the resolvers somehow?

[voodooattack]

@stubailo Here's a use case https://github.com/lirown/graphql-custom-directives

[stubailo]

Wow, I did not know that was a thing you could do! I think adding a new option to makeExecutableSchema would be the way to go. Under the hood perhaps it could be implemented via an attachDirectivesToSchema function.

Please send a PR!

[voodooattack]

@helfer @stubailo Here's another example that generates a full database backend from schema language definitions with directives. https://gist.github.com/voodooattack/ce5f0afb5515ab5a153e535ac20698da

PS: I'd send a PR, but I'm not entirely clear on the big picture yet. What should the API look like?

[helfer]

@voodooattack the best way to start in my opinion would be to write a design doc similar to the ones we have in apollo client. The design doc should explain what feature you're adding, how it works, why it's useful and provide a few examples for how to use it. That way we can provide feedback on the idea before too much code is written.

[helfer]

@voodooattack are you still interested in this? I think the API could simply be to add an additional argument for directives to the makeExecutableSchema function and pass that through to graphql-js.

[DxCx]

this can be a great feature! 👍

[helfer]

Closing this for lack of activity. If you're still interested, please send us a PR @voodooattack ! 🙂

[timbrandin]

Hey people, I have successfully implemented attachDirectivesToSchema locally, I'm not sure where in this code it fits, but this is how it looks.

// directives.js

import { chainResolvers } from 'graphql-tools';

export default {
  translate: (value) => {
    // Translate field value.
    return value;
  },
};

// Helper to attach directives to the schema.
export function attachDirectivesToSchema(info, directives) {
  function attachSelectionSetDirectives({ selections }, parentType) {
    return selections.forEach((selection) => {
      const key = selection.name.value;
      const typeDef = parentType._fields[key]; // eslint-disable-line
      if (typeDef) {
        const fieldNodeType = typeDef.type.ofType || typeDef.type;
        const directiveResolvers = selection.directives.map((directive) => {
          const name = directive.name.value;
          const params = directive.arguments.reduce((acc, arg) => ({
            ...acc,
            [arg.name.value]: arg.value.value,
          }), {});
          if (directives[name]) {
            return (obj, args, ...rest) => directives[name](obj, { ...args, ...params }, ...rest);
          }
          return null;
        }).filter(x => !!x);
        // Keep track of the original resolver.
        if (!typeDef.original) {
          typeDef.original = [typeDef.resolve];
        }
        // Chain the resolvers for the directives.
        if (directiveResolvers.length > 0) {
          typeDef.resolve = chainResolvers([
            ...typeDef.original,
            ...directiveResolvers,
          ]);
        } else if (typeDef.original) {
          typeDef.resolve = typeDef.original[0];
        }
        // Recurse down the selectionSet.
        if (selection.selectionSet) {
          attachSelectionSetDirectives(selection.selectionSet, fieldNodeType);
        }
      }
    }, {});
  }

  return attachSelectionSetDirectives({ selections: info.fieldNodes }, info.parentType);
}

// schema.js

import directives, { attachDirectivesToSchema } from './directives';

const resolvers = {
  Query: {},
};

const typeDefs = `
  type Query {
    field: String
  }

  schema {
     query Query
  }

  directives @translate on FIELD
`

const schema = makeExecutableSchema({
  typeDefs,
  resolvers: {
    ...resolvers,
    __schema: (obj, args, context, info) => {
      attachDirectives(info, directives);
      return obj;
    },
  },
});

My first idea was to chain in a directive resolvers on every field in the build step and look at the info for any directives at the current level and determine if it should run, but I quickly realised that this method would run way to much. So my second attempt is this where I jointly go through the query and corresponding type, and if found any directives at each level I chain them in after the original resolver has executed. One drawback is that the schema is mutable so one have to remove any leftover directives on fields that had them before, and also make sure they don't start stacking up more and more that will be executed.

But anyhow, this way it's easy to add directives to the schema.

I hope it helps someone!

Cheers,
// T

[helfer]

Hey @timbrandin! That looks interesting. Do the directives get attached on every request? If so, is that necessary, or could they be applied to the resolvers at the time when the schema is built?

[timbrandin]

They do get attached on every request. My initial idea was as you mention to attach a filter to every field which would determine which and if a directive should be executed. But I came to the conclusion that having a filter on the request walk through the query and attach would be less resource demanding than a filter that ran for every field on every request, but of course I haven't done the math for this so I could be wrong.

[agonbina]

I took the time to figure out how to actually get directives to work(n.b. custom directives are not part of the official graphql spec for some reason)

With a schema definition like the following:

# Throws an errors if the user is not authenticated
directive @authenticated(roles: [String]) on QUERY | FIELD
# Slugifies a string input
directive @slugify on FIELD
# Sets a default value for a field
directive @default(value: String!) on FIELD

type Profile {
  title: String! @slugify
  # You can even chain them!
  username: String! @slugify @default(value: "kitty") 
}

type Query {
  profile: Profile! @authenticated(roles: ["admin"])
}

Here is the solution that I came up with:

import { makeExecutableSchema, forEachField } from 'graphql-tools'
import { getArgumentValues } from 'graphql/execution/values'

// create the schema as usual
cont schema = makeExecutableSchema(...)

// the resolvers for the directives defined in your schema
const directiveResolvers = {
  default (result, source, args) {
    const { value } = args
    return result || value
  },
  slugify (result, source, args) {
    const slugified = result + '-slugified'
    return slugified // or return Promise.resolve(slugified)
  },
  authenticated (result, source, args, context) {
    const { roles = [] } = args 
    // do something with roles if you need to
    return Promise.reject('You are not authorized to view this field.')
  }
}

// The utility iterator that patches the original resolver of a field  to apply any directive resolvers
forEachField(schema, (field: any) => {
  const directives = field.astNode.directives as DirectiveNode[]
  directives.forEach(directive => {
    const directiveName = directive.name.value
    const resolver = directiveResolvers[directiveName]

    if (resolver) {
      const oldResolve = field.resolve
      const Directive = schema.getDirective(directiveName)
      // Resolve the arguments for the directive (ex. for @authenticated it will be { roles: ['admin'] }
      const args = getArgumentValues(Directive, directive) 

      field.resolve = function () {
        const [ source, _, context, info ] = arguments
        let promise = oldResolve.call(field, ...arguments)

        // If you return a primitive from the default resolver
        const isPrimitive = !(promise instanceof Promise)
        if (isPrimitive) {
          promise = Promise.resolve(promise)
        }

        return promise.then(result => resolver(result, source, args, context, info))
      }
    }
  })
})

What do you guys think? Happy to submit a PR to add this as a helper function similar to the other ones that mutate the executable schema(ex for adding mocks, etc).

p.s. ignore the TS types, some of them doesn't seem to be well defined (ex the type for the field is omitting some properties - maybe on purpose?)
p.p.s I just realised that this is only handling "field level" directives. which means if you have a directive on a query/mutation argument this will never be touched.

I am really struggling to understand why directives haven't been paid much attention given how useful they could be ...

[giautm]

@agonbina :Hi, i just edited your code to solve a problem at here.
a better version of attachDirectives:

export const attachDirectives = (resolvers, schema: GraphQLSchema) => {
  forEachField(schema, (field: GraphQLField<any, any>) => {
    const directives = field.astNode.directives;
    directives.forEach((directive: DirectiveNode) => {
      const directiveName = directive.name.value;
      const resolver = resolvers[directiveName];

      if (resolver) {
        const originalResolver = field.resolve || defaultFieldResolver;
        const Directive = schema.getDirective(directiveName);
        const directiveArgs = getArgumentValues(Directive, directive);

        field.resolve = (...args) => {
          const [source, _, context, info] = args;
          return resolver(() => {
            const promise = originalResolver.call(field, ...args);
            if (promise instanceof Promise) {
              return promise;
            }
            return Promise.resolve(promise);
          }, source, directiveArgs, context, info);
        };
      }
    });
  });
};

[giautm]

i opened a PR for this issues: https://github.com/apollographql/graphql-tools/pull/518/files
I will write tests in next days. But i want your helps to document it. Tks

[gregberge]

I made graphql-directive. It is inspired by your work but I extended the support. Directive works on FIELD and FIELD_DEFINITION locations. You can use custom directive right now 😄 !

Be careful is you use Apollo InMemory Cache, custom directives are not yet supported. I submitted a pull-request to fix it.

[stubailo]

#518 is merged! Please add some docs for it :] #530

[stubailo]

Launched in 2.13.0

[stefanholzapfel]

Cool thank you! Is there any documentation available already?

Would be especially interesting...

1.) What are the allowed "on" parameters now (have seen different naming in different sources)? QUERY, FIELD, FIELD_DEFINITION, ... and where are they actually allowed?

2.) How to apply multiple directives on one query/field?
Using allVotes: [Vote] @isAuthenticated @hasPermission(permissison: "TEST") results in only the second directive being called.

[HofmannZ]

@stefanholzapfel, exactly looking for the same answers.

[giautm]

@stefanholzapfel 👍
1.) Only FIELD_DEFINITION was supported for now.
2.) This case ready tested in the test-cases. You can see it here.

Updated: I want to know how you implement isAuthenticated directive? Your directive should check auth token before call next().

[stefanholzapfel]

Yes, I use JWT and check if the authorization header is present and correct on the request. If not, I throw an error, which I created using the apollo-errors package.

Intercept request and store auth info in context:

graphQLServer.use(
    '/graphql',
    bodyParser.json(),
    graphqlExpress(
        (request) => {
            const data = {
                schema: schema,
                formatError: formatError
            };
            // Check if token is present and write username and permission into the context for usage in directives
            if (request.headers.authorization) {
                const tokenPayload = jwt.verify(request.headers.authorization, config.secret);
                data.context = {};
                data.context.user = tokenPayload.username;
                data.context.permission = tokenPayload.permission;
            }
            return data;
        })
);

In directive:

const directiveResolvers = {
    isAuthenticated(query, source, args, context) {
        if (!context.user) {
            throw new AuthenticationError();
        }
        return query();
    }
}

[HofmannZ]

@giautm The current test case could also pass if only the last directive gets executed.
@stefanholzapfel Yeah, using a somewhat similar way or authentication.

I tested around with some very generic code, and the behaviour is far from expected. Since this is slightly off topic for this issue I created a new one, see #630.

[AndreasGalster]

I was trying out the samples in here and the new official docs but neither of them work for me because my executable schema doesn't have astNodes on the fields. Is there any reason why? How do I get an executable schema with astNodes so the snippets in here work?

[kriswep]

Can anyone elaborate why the normal resolver args aren't passed to the directiveResolvers?
How would one implement a requirement where you need to check authentication and sthg like author of the node in question? (Only allowed to change a post, if you are authenticated and the author)
Is that possible?
Saw an implementation where one accessed some req.body.variables from context, but that doesn't seem to be a generic working way.

[stefanholzapfel]

@kriswep
You are right, I can't find a way to access the resolver args without executing the next() parameter to get access to it's context.
@giautm Any idea?

[lastmjs]

I'm running into the same issue. I'll be working on it

[LawJolla]

@kriswep and @stefanholzapfel

I opened an issue for this question, though it seems to have fallen on deaf ears.
#649

I point out that in the directive resolver "binding", the args seem to be purposefully omitted. I'm wondering why.