Fix @composeDirective corner case (#2046)

@composeDirective wasn't working when @link of the link directive was not present.

The reason this is happening is that although we are waiting to process core schemas for think that aren't the link schema, we aren't doing the same for other directives. This fix determines if the schema blueprint is a federation schema, and if so, will wait to process non @link directives until they've all been processed (and the @link to the link spec has been synthetically inserted if necessary).

composition-js/src/__tests__/compose.composeDirective.test.ts

@@ -694,7 +694,6 @@ describe('composing custom core directives', () => {
  typeDefs: gql`
  extend schema
  @link(url: "https://specs.apollo.dev/federation/v2.1", import: ["@key", "@composeDirective"])
- @link(url: "https://specs.apollo.dev/link/v1.0")
  @link(url: "https://specs.apollo.dev/foo/v1.0", import: [{ name: "@foo", as: "@inaccessible" }])
  @composeDirective(name: "@inaccessible")
 

composition-js/src/__tests__/compose.test.ts

@@ -2467,7 +2467,7 @@ describe('composition', () => {
  expect(userType?.field('name')?.appliedDirectivesOf('inaccessible').pop()).toBeDefined();
  });
 
- describe('rejects @inaccessible and @external together', () => {
+ it('rejects @inaccessible and @external together', () => {
  const subgraphA = {
  typeDefs: gql`
  type Query {

internals-js/src/buildSchema.ts

@@ -100,7 +100,7 @@ export function buildSchemaFromAST(
  // is that:
  // 1. we can (enum values are self-contained and cannot reference anything that may need to be imported first; this
  // is also why we skip directive applications at that point, as those _may_ reference something that hasn't been imported yet)
- // 2. this allows the code to handle better the case where the `link__Purpose` enum is provided in the AST despite the `@link` 
+ // 2. this allows the code to handle better the case where the `link__Purpose` enum is provided in the AST despite the `@link`
  // _definition_ not being provided. And the reason that is true is that as we later _add_ the `@link` definition, we
  // will need to check if `link_Purpose` needs to be added or not, but when it is already present, we check it's definition
  // is the expected, but that check will unexpected fail if we haven't finished "building" said type definition.
@@ -215,7 +215,7 @@ function buildNamedTypeAndDirectivesShallow(documentNode: DocumentNode, schema:
  let type = schema.type(definitionNode.name.value);
  // Note that the type may already exists due to an extension having been processed first, but we know we
  // have seen 2 definitions (which is invalid) if the definition has `preserverEmptyDefnition` already set
- // since it's only set for definitions, not extensions. 
+ // since it's only set for definitions, not extensions.
  // Also note that we allow to redefine built-ins.
  if (!type || type.isBuiltIn) {
  type = schema.addType(newNamedType(withoutTrailingDefinition(definitionNode.kind), definitionNode.name.value));
@@ -332,9 +332,23 @@ function buildAppliedDirectives(
  for (const directive of elementNode.directives ?? []) {
  withNodeAttachedToError(
  () => {
- const d = element.applyDirective(directive.name.value, buildArgs(directive));
- d.setOfExtension(extension);
- d.sourceAST = directive;
+ /**
+ * If we are at the schemaDefinition level of a federation schema, it's possible that some directives
+ * will not be added until after the federation calls completeSchema. In that case, we want to wait
+ * until after completeSchema is called before we try to apply those directives.
+ */
+ if (element !== element.schema().schemaDefinition || directive.name.value === 'link' || !element.schema().blueprint.applyDirectivesAfterParsing()) {
+ const d = element.applyDirective(directive.name.value, buildArgs(directive));
+ d.setOfExtension(extension);
+ d.sourceAST = directive;
+ } else {
+ element.addUnappliedDirective({
+ extension,
+ directive,
+ args: buildArgs(directive),
+ nameOrDef: directive.name.value,
+ });
+ }
  },
  directive,
  errors,

internals-js/src/definitions.ts

@@ -18,7 +18,8 @@ import {
  SchemaDefinitionNode,
  TypeDefinitionNode,
  DefinitionNode,
- DirectiveDefinitionNode
+ DirectiveDefinitionNode,
+ DirectiveNode,
 } from "graphql";
 import {
  CoreImport,
@@ -522,11 +523,37 @@ export class Extension<TElement extends ExtendableElement> {
  }
 }
 
+type UnappliedDirective = {
+ nameOrDef: DirectiveDefinition<Record<string, any>> | string,
+ args: Record<string, any>,
+ extension?: Extension<any>,
+ directive: DirectiveNode,
+};
+
 // TODO: ideally, we should hide the ctor of this class as we rely in places on the fact the no-one external defines new implementations.
 export abstract class SchemaElement<TOwnType extends SchemaElement<any, TParent>, TParent extends SchemaElement<any, any> | Schema> extends Element<TParent> {
  protected readonly _appliedDirectives: Directive<TOwnType>[] = [];
+ protected _unappliedDirectives: UnappliedDirective[] = [];
  description?: string;
 
+ addUnappliedDirective({ nameOrDef, args, extension, directive }: UnappliedDirective) {
+ this._unappliedDirectives.push({
+ nameOrDef,
+ args: args ?? {},
+ extension,
+ directive,
+ });
+ }
+
+ processUnappliedDirectives() {
+ for (const { nameOrDef, args, extension, directive } of this._unappliedDirectives) {
+ const d = this.applyDirective(nameOrDef, args);
+ d.setOfExtension(extension);
+ d.sourceAST = directive;
+ }
+ this._unappliedDirectives = [];
+ }
+
  get appliedDirectives(): readonly Directive<TOwnType>[] {
  return this._appliedDirectives;
  }
@@ -943,6 +970,10 @@ export class SchemaBlueprint {
  onUnknownDirectiveValidationError(_schema: Schema, _unknownDirectiveName: string, error: GraphQLError): GraphQLError {
  return error;
  }
+
+ applyDirectivesAfterParsing() {
+ return false;
+ }
 }
 
 export const defaultSchemaBlueprint = new SchemaBlueprint();

internals-js/src/federation.ts

@@ -703,7 +703,9 @@ export class FederationBlueprint extends SchemaBlueprint {
  }
 
  onDirectiveDefinitionAndSchemaParsed(schema: Schema): GraphQLError[] {
- return completeSubgraphSchema(schema);
+ const errors = completeSubgraphSchema(schema);
+ schema.schemaDefinition.processUnappliedDirectives();
+ return errors;
  }
 
  onInvalidation(schema: Schema) {
@@ -878,6 +880,10 @@ export class FederationBlueprint extends SchemaBlueprint {
  }
  return error;
  }
+
+ applyDirectivesAfterParsing() {
+ return true;
+ }
 }
 
 function findUnusedNamedForLinkDirective(schema: Schema): string | undefined {