Alpha.3 - schemaDirectives not in use

[lirbank]

Per https://neo4j.com/docs/graphql-manual/2.0/api-reference/ and https://neo4j.com/docs/graphql-manual/current/api-reference/ one may provide custom directives via the schemaDirectives option to the constructor.

This used to work up until 2.0.0-alpha.2, but upgrading to 2.0.0-alpha.3 it seems the custom directives provided are ignored.

System:

"@neo4j/graphql": "2.0.0-alpha.3",

[darrellwarde]

Hey @lirbank, we actually have an integration for this (https://github.com/neo4j/graphql/blob/master/packages/graphql/tests/integration/custom-directives.int.test.ts#L39) which passed when we did the release, and is still passing now.

Could you give any further details about your specific use case? Ideally something easily reproducible in an integration test!

[lirbank]

Hi @darrellwarde, thanks for the reference. That test passes in our setup as well. However directives doesn't seem to work in mutations - at least not at the top level.

This test passes in alpha 2, but not in alpha 3 or alpha 4:

import neo4j from 'neo4j-driver';
import { Neo4jGraphQL } from '@neo4j/graphql';
import { SchemaDirectiveVisitor } from '@graphql-tools/utils';
import { graphql } from 'graphql';

type Field = Parameters<SchemaDirectiveVisitor['visitFieldDefinition']>[0];
type Details = Parameters<SchemaDirectiveVisitor['visitFieldDefinition']>[1];

class DisallowDirective extends SchemaDirectiveVisitor {
  public visitFieldDefinition(field: Field, details: Details) {
    field.resolve = function (...args) {
      // Disallow any and all access, all the time
      throw new Error('go away');
    };
  }
}

// NOTE: Neo4jGraphQL schemaDirectives type is not compatible with ApolloServer
// schemaDirectives type or @graphql-tools/utils schemaDirectives type, so we're
// casting the type here to force compatibility.
//
// https://www.apollographql.com/docs/apollo-server/v2
// https://www.graphql-tools.com/docs/legacy-schema-directives/
type SchemaDirectives = ConstructorParameters<
  typeof Neo4jGraphQL
>[number]['schemaDirectives'];

const schemaDirectives = {
  disallow: DisallowDirective,
} as unknown as SchemaDirectives;

const neo4jGraphQL = new Neo4jGraphQL({
  typeDefs: /* GraphQL */ `
    directive @disallow on FIELD_DEFINITION

    type Mutation {
      doStuff: String! @disallow
    }

    type Query {
      noop: Boolean
    }
  `,

  driver: neo4j.driver('bolt://localhost:7687'),
  resolvers: { Mutation: { doStuff: () => 'OK' } },
  schemaDirectives,
});

test('DisallowDirective', async () => {
  const gqlResult = await graphql({
    schema: neo4jGraphQL.schema,
    source: /* GraphQL */ `
      mutation {
        doStuff
      }
    `,
    contextValue: { driver: neo4j.driver('bolt://localhost:7687') },
  });

  expect(gqlResult.data).toBeNull();
  expect(gqlResult.errors).toBeTruthy();
});

[lirbank]

Here are some more expanded tests. All fail on Alpha 4 but pass on Alpha 2.

import neo4j from 'neo4j-driver';
import { Neo4jGraphQL } from '@neo4j/graphql';
import { SchemaDirectiveVisitor } from '@graphql-tools/utils';
import { graphql } from 'graphql';

type Field = Parameters<SchemaDirectiveVisitor['visitFieldDefinition']>[0];
type Details = Parameters<SchemaDirectiveVisitor['visitFieldDefinition']>[1];

class DisallowDirective extends SchemaDirectiveVisitor {
  public visitFieldDefinition(field: Field, details: Details) {
    field.resolve = function (...args) {
      throw new Error('go away');
    };
  }
}

// NOTE: Neo4jGraphQL schemaDirectives type is not compatible with ApolloServer
// schemaDirectives type or @graphql-tools/utils schemaDirectives type, so we're
// casting the type here to force compatibility.
//
// https://www.apollographql.com/docs/apollo-server/v2
// https://www.graphql-tools.com/docs/legacy-schema-directives/
type SchemaDirectives = ConstructorParameters<
  typeof Neo4jGraphQL
>[number]['schemaDirectives'];

const schemaDirectives = {
  disallow: DisallowDirective,
} as unknown as SchemaDirectives;

const neo4jGraphQL = new Neo4jGraphQL({
  typeDefs: /* GraphQL */ `
    directive @disallow on FIELD_DEFINITION

    type NestedResult {
      stuff: String! @disallow
    }

    type Mutation {
      doStuff: String! @disallow
      doNestedStuff: NestedResult!
    }

    type Query {
      getStuff: String! @disallow
      getNestedStuff: NestedResult!
    }
  `,

  driver: neo4j.driver('bolt://localhost:7687'),
  resolvers: {
    NestedResult: {
      stuff: (parent: string) => parent,
    },

    Mutation: {
      doStuff: () => 'OK',
      doNestedStuff: () => 'OK',
    },

    Query: {
      getStuff: () => 'OK',
      getNestedStuff: () => 'OK',
    },
  },
  schemaDirectives,
});

test('mutation top - DisallowDirective', async () => {
  const gqlResult = await graphql({
    schema: neo4jGraphQL.schema,
    source: /* GraphQL */ `
      mutation {
        doStuff
      }
    `,
    contextValue: { driver: neo4j.driver('bolt://localhost:7687') },
  });

  expect(gqlResult.data).toBeNull();
  expect(gqlResult.errors && gqlResult.errors[0].message).toBe('go away');
});

test('query top - DisallowDirective', async () => {
  const gqlResult = await graphql({
    schema: neo4jGraphQL.schema,
    source: /* GraphQL */ `
      query {
        getStuff
      }
    `,
    contextValue: { driver: neo4j.driver('bolt://localhost:7687') },
  });

  expect(gqlResult.data).toBeNull();
  expect(gqlResult.errors && gqlResult.errors[0].message).toBe('go away');
});

test('mutation nested - DisallowDirective', async () => {
  const gqlResult = await graphql({
    schema: neo4jGraphQL.schema,
    source: /* GraphQL */ `
      mutation {
        doNestedStuff {
          stuff
        }
      }
    `,
    contextValue: { driver: neo4j.driver('bolt://localhost:7687') },
  });

  expect(gqlResult.data).toBeNull();
  expect(gqlResult.errors && gqlResult.errors[0].message).toBe('go away');
});

test('query nested - DisallowDirective', async () => {
  const gqlResult = await graphql({
    schema: neo4jGraphQL.schema,
    source: /* GraphQL */ `
      query {
        getNestedStuff {
          stuff
        }
      }
    `,
    contextValue: { driver: neo4j.driver('bolt://localhost:7687') },
  });

  expect(gqlResult.data).toBeNull();
  expect(gqlResult.errors && gqlResult.errors[0].message).toBe('go away');
});

It seems the issue has to do with throwing errors in the visitFieldDefinition method.

[lirbank]

Versions:

"@neo4j/graphql": "2.0.0-alpha.2",
"@neo4j/graphql": "2.0.0-alpha.4",
"neo4j-driver": "4.3.1",
"@graphql-tools/utils": "7.10.0",
"graphql": "15.5.0",

[darrellwarde]

Amazing @lirbank, thank you so much for the detailed examples, going to try to figure this one out today! 🙂

[darrellwarde]

Fixed in #356, and hopefully you won't have to re-cast your schema directives once this is merged and released.

I would like to take this opportunity to ask, why are you using the legacy SchemaDirectiveVisitor class as opposed to the new functional approach? https://www.graphql-tools.com/docs/schema-directives/#implementing-schema-directives

[lirbank]

Awesome! We're porting an existing GraphQL solution to Neo4jGraphQL, so we already have some directives that were written a long time ago, but works well and are quite hardened by now. That's the only reason we're using legacy directives. If we were to write new ones it'd be in the functional approach.

[darrellwarde]

Hey @lirbank, I think I remember you telling us that before, thank you for the background.

I do, however, want to point out that @graphql-tools/[email protected] has just been released, and this completely removes support for legacy schema directives (https://github.com/ardatan/graphql-tools/releases/tag/%40graphql-tools%2Fschema%408.0.0).

We're not upgrading right now because the changes really break things for us. However, we will upgrade in the near future, so I would encourage you to perhaps think about rewriting those legacy schema directives so that you don't lose the ability to use our library when we do.

[lirbank]

Thanks @darrellwarde, that's really good to know! I'll plan for rewriting the directives.