Use getDirectives when retrieving cacheControl directives

[benatshippabo]

Issue

Currently we retrieve directives by calling .directives on the AST node, but this limits users with a code-first approach since in order for users to specify inheritMaxAge it has to be statically defined

Context

apollo-server/packages/apollo-server-core/src/plugin/cacheControl/index.ts

Lines 305 to 321 in f6c5c9f

	function cacheAnnotationFromType(t: GraphQLCompositeType): CacheAnnotation {
	if (t.astNode) {
	const hint = cacheAnnotationFromDirectives(t.astNode.directives);
	if (hint) {
	return hint;
	}
	}
	if (t.extensionASTNodes) {
	for (const node of t.extensionASTNodes) {
	const hint = cacheAnnotationFromDirectives(node.directives);
	if (hint) {
	return hint;
	}
	}
	}
	return {};
	}

apollo-server/packages/apollo-server-core/src/plugin/cacheControl/index.ts

Lines 305 to 321 in f6c5c9f

	function cacheAnnotationFromType(t: GraphQLCompositeType): CacheAnnotation {
	if (t.astNode) {
	const hint = cacheAnnotationFromDirectives(t.astNode.directives);
	if (hint) {
	return hint;
	}
	}
	if (t.extensionASTNodes) {
	for (const node of t.extensionASTNodes) {
	const hint = cacheAnnotationFromDirectives(node.directives);
	if (hint) {
	return hint;
	}
	}
	}
	return {};
	}

apollo-server/packages/apollo-server-core/src/plugin/cacheControl/index.ts

Lines 323 to 333 in f6c5c9f

	function cacheAnnotationFromField(
	field: GraphQLField<unknown, unknown>,
	): CacheAnnotation {
	if (field.astNode) {
	const hint = cacheAnnotationFromDirectives(field.astNode.directives);
	if (hint) {
	return hint;
	}
	}
	return {};
	}

graphql/graphql-js#3213 (comment)

graphql/graphql-js#1343

Proposal

We currently already use parts of @graphql-tools/*. We can use @graphql-tools/utils getDirectives so that users with a code-first approach are able to specify directives via extensions.directives.

So something like:

cacheAnnotationFromDirectives([
  // read directives from schema
  ...t.astNode.directives,
  // read directives specified under `extensions`
  ...getDirectives(schema, t.astNode)
])

and then users could do:

new GraphQLObjectType({
  name: 'ObjectInheritingMaxAge',
  fields: {
    date: { type: new GraphQLNonNull(GraphQLDate) },
  },
  extensions: {
    directives: {
      cacheControl: {
        inheritMaxAge: true,
      },
    },
  },
})

[glasser]

It's an interesting point. I do know that some movement has happened lately in the GraphQL world as far as potentially providing a more standard way of exposing directives (eg graphql/graphql-spec#300) and it might be a bit of a shame to adopt the non-standard extensions.directives from graphql-tools if a more standard way is coming soon. I'm not sure why it's impossible for TypeGraphQL to just create AST nodes directly...

[benatshippabo]

Got it. Makes sense to me