Error when trying to add a directive with any arguments via makeExtendSchemaPlugin

[benweint]

Summary

I'm trying to write a Postgraphile plugin which (among other things) adds several directives to my GraphQL schema. Adding directives via makeExtendSchemaPlugin seems to work if those directives have no arguments, but if they do have arguments, I see an error:

Plugin code

const AddDirectivePlugin = makeExtendSchemaPlugin(() => {
  return {
    typeDefs: gql`
      directive @foobar(x: Int!) on FIELD_DEFINITION
    `,
  };
});

Error messsage:

/Users/ben/src/ouch-my-finger/node_modules/graphile-build/dist/makeNewBuild.js:232
                throw new Error("Must not call build.getTypeByName before 'init' phase is complete");
                      ^

Error: Must not call build.getTypeByName before 'init' phase is complete
    at Object.getTypeByName (/Users/ben/src/ouch-my-finger/node_modules/graphile-build/dist/makeNewBuild.js:232:23)
    at getType (/Users/ben/src/ouch-my-finger/node_modules/graphile-utils/dist/makeExtendSchemaPlugin.js:436:32)
    at getType (/Users/ben/src/ouch-my-finger/node_modules/graphile-utils/dist/makeExtendSchemaPlugin.js:443:53)
    at /Users/ben/src/ouch-my-finger/node_modules/graphile-utils/dist/makeExtendSchemaPlugin.js:527:34
    at Array.reduce (<anonymous>)
    at getArguments (/Users/ben/src/ouch-my-finger/node_modules/graphile-utils/dist/makeExtendSchemaPlugin.js:524:25)
    at /Users/ben/src/ouch-my-finger/node_modules/graphile-utils/dist/makeExtendSchemaPlugin.js:312:42
    at Array.forEach (<anonymous>)
    at init (/Users/ben/src/ouch-my-finger/node_modules/graphile-utils/dist/makeExtendSchemaPlugin.js:126:30)
    at SchemaBuilder.applyHooks (/Users/ben/src/ouch-my-finger/node_modules/graphile-build/dist/SchemaBuilder.js:98:30)

Steps to reproduce

1. Check out the add-directive-with-args-error branch of my fork of ouch-my-finger here
2. Within the working copy, try to start a postgraphile server (yarn postgraphile -c postgres:///my_db -s app_public) - it shouldn't matter what's in the DB

Expected results

I would expect the directive to be added to the GraphQL schema presented by Postgraphile, and to see no error on startup.

Actual results

I get this error:

❯ yarn postgraphile -c postgres:///ben -s ben
yarn run v1.22.21
warning ../../package.json: No license field
$ postgraphile -c postgres:///ben -s ben
Server listening on port 5678 at http://[::]:5678/graphql
/Users/ben/src/ouch-my-finger/node_modules/graphile-build/dist/makeNewBuild.js:232
                throw new Error("Must not call build.getTypeByName before 'init' phase is complete");
                      ^

Error: Must not call build.getTypeByName before 'init' phase is complete
    at Object.getTypeByName (/Users/ben/src/ouch-my-finger/node_modules/graphile-build/dist/makeNewBuild.js:232:23)
    at getType (/Users/ben/src/ouch-my-finger/node_modules/graphile-utils/dist/makeExtendSchemaPlugin.js:436:32)
    at getType (/Users/ben/src/ouch-my-finger/node_modules/graphile-utils/dist/makeExtendSchemaPlugin.js:443:53)
    at /Users/ben/src/ouch-my-finger/node_modules/graphile-utils/dist/makeExtendSchemaPlugin.js:527:34
    at Array.reduce (<anonymous>)
    at getArguments (/Users/ben/src/ouch-my-finger/node_modules/graphile-utils/dist/makeExtendSchemaPlugin.js:524:25)
    at /Users/ben/src/ouch-my-finger/node_modules/graphile-utils/dist/makeExtendSchemaPlugin.js:312:42
    at Array.forEach (<anonymous>)
    at init (/Users/ben/src/ouch-my-finger/node_modules/graphile-utils/dist/makeExtendSchemaPlugin.js:126:30)
    at SchemaBuilder.applyHooks (/Users/ben/src/ouch-my-finger/node_modules/graphile-build/dist/SchemaBuilder.js:98:30)

[benjie]

It's likely that what you're trying to do doesn't make sense (e.g. I suspect it requires graphql/graphql-spec#300, which GraphQL doesn't support at this time) and I can't think of anything useful that a FIELD_DEFINITION directive would do in PostGraphile currently...

Nonetheless, this is the code that needs reworking:

crystal/graphile-build/graphile-utils/src/makeExtendSchemaPlugin.ts

Lines 569 to 585 in f13e3ca

	} else if (def.type === GraphQLDirective) {
	const definition = def.definition as DirectiveDefinitionNode;
	// https://github.com/graphql/graphql-js/blob/3c54315ab13c6b9d337fb7c33ad7e27b92ca4a40/src/type/directives.js#L106-L113
	const name = getName(definition.name);
	const description = getDescription(definition.description);
	const locations = definition.locations.map(
	getName,
	) as DirectiveLocation[];
	const args = getArguments(definition.arguments, build);
	// Ignoring isRepeatable and astNode for now
	const directive = new GraphQLDirective({
	name,
	locations,
	args,
	...(description ? { description } : null),
	});
	typeExtensions.GraphQLSchema.directives.push(directive);

Specifically it needs to be more like the code in the block above it, i.e. there needs to be a build.registerDirective(name, scope, () => ({ locations, args, description })) instead. This'll involve building that out across the entire stack; which is currently very low priority due to the aforementioned "it's not useful" issue.

[benweint]

Thanks @benjie! The directive location in this example is somewhat arbitrary. Let me explain the motivating use case:

We're using a Postgraphile service as a subgraph within a supergraph composed together via Apollo federation. With Apollo federation, an executable directive must be defined and identically so across all subgraphs in order for it to appear in the final supergraph schema.

This means that even if our Postgraphile subgraph has no need for this particular directive (ours indeed doesn't), we still have a requirement of defining the directive in our schema, so as to not cause it to be removed from the composed supergraph schema.

The test case as written is against a FIELD_DEFINITION directive, but the same problem reproduces with a directive defined as applicable to FIELD, QUERY, MUTATION, etc (locations for executable directives).

[benjie]

Makes sense. We're looking for a few people to fund work on Federation support (or do it!) so if that's something you're able to help with, do get in touch. It's likely to be a good couple of weeks of work (not specifically this directive thing, that's probably closer to a half day to a day... If it works the way I hope it might; but of course this is part of the whole Federation super-task) and there's only a few people interested so it's relatively low priority for me otherwise.