introspectSchema throw when underlying schema have contraint directives

[yvele]

/label bug

I'm trying to make schema stitching with underlying schemas having GraphQL contraint directives.

But the stitching process is throwing an error on introspectSchema:

Error: Invalid or incomplete schema, unknown type: ConstraintString.
Ensure that a full introspection query is used in order to build a client schema.

    at getNamedType (/Users/yves/repo/node_modules/graphql/utilities/buildClientSchema.js:96:13)
    at getType (/Users/yves/repo/node_modules/graphql/utilities/buildClientSchema.js:87:12)
    at getInputType (/Users/yves/repo/node_modules/graphql/utilities/buildClientSchema.js:104:16)
    at buildInputValue (/Users/yves/repo/node_modules/graphql/utilities/buildClientSchema.js:249:16)
    at /Users/yves/repo/node_modules/graphql/jsutils/keyValMap.js:28:31
    at Array.reduce (<anonymous>)
    at keyValMap (/Users/yves/repo/node_modules/graphql/jsutils/keyValMap.js:27:15)
    at buildInputValueDefMap (/Users/yves/repo/node_modules/graphql/utilities/buildClientSchema.js:243:36)
    at fields (/Users/yves/repo/node_modules/graphql/utilities/buildClientSchema.js:218:16)
    at resolveThunk (/Users/yves/repo/node_modules/graphql/type/definition.js:370:40)

My underlying schema looks like that:

input AddItemInput {
  name: String! @constraint(minLength: 2, maxLength: 40)
}

import ConstraintDirective from "graphql-constraint-directive";

export default makeExecutableSchema({
  typeDefs,
  resolvers,
  schemaDirectives : { constraint: ConstraintDirective }
});

Edit: Related to confuser/graphql-constraint-directive#2

[rainum]

Same problem here with a similar setup, but error is a bit different:

Error: Directive constraint: Couldn't find type constraint in any of the schemas.
    at collectDirective (/project_dir/node_modules/graphql-import/src/definition.ts:172:15)
    at Array.forEach (<anonymous>)
    at collectNode (/project_dir/node_modules/graphql-import/src/definition.ts:161:21)
    at /project_dir/node_modules/graphql-import/src/definition.ts:135:7
    at Array.forEach (<anonymous>)
    at collectNewTypeDefinitions (/project_dir/node_modules/graphql-import/src/definition.ts:134:26)
    at Object.completeDefinitionPool (/project_dir/node_modules/graphql-import/src/definition.ts:49:39)
    at Object.importSchema (/project_dir/node_modules/graphql-import/src/index.ts:124:26)

[koresar]

@rainum your issue is unrelated IMO. You need to add this to your SDL:

directive @constraint(
    # String constraints
    minLength: Int
    maxLength: Int
    startsWith: String
    endsWith: String
    notContains: String
    pattern: String
    format: String

    # Number constraints
    min: Int
    max: Int
    exclusiveMin: Int
    exclusiveMax: Int
    multipleOf: Int
) on INPUT_FIELD_DEFINITION

As soon as you add it you'll get the error from above.

[koresar]

I debugged a little. TL;DR: The makeExecutableSchema does not replace the directive object in schema.

• The graphql-import people tell you to "declare the @constraint scalar in .graphql file". It effectively creates a directive object in schema. Which is right thing to do IMO. My IDE stops complaining about "undeclared directive!".
• The makeExecutableSchema accepts the schemaDirectives and applies the schema visitor to them. But it does not replace the previously declared directive generated by graphql-import.

Here is a screenshot. I made it in node.js debugger one step before exiting the makeExecutableSchema. Basically, that schema variable is what I get.

As the result, the @constraint directive is not being visited at runtime.

Everything above can be wrong due to my limited understanding.

[rainum]

@koresar yes, you're right. Now I'm getting the Can't find type ConstraintString error.

[koresar]

I have solved the problem, people.

The high level solution: do not use Prisma and Apollo packages in the same project. They are often incompatible.

What I did:

• Removed everything Prisma (in my case - graphql-yoga and graphql-import packages).
• Rewrote schema stitching (removed #import statements and used Apollo's mergeSchema() JS function).

Now I'm using Apollo Stack v2 (currently RC). It has solved all the problems I had.

[tdharris]

There is more context to the above comment that can be found below (doesn't work):
confuser/graphql-constraint-directive#2

There is a hacky workaround suggested there that may not work for everyone.

Any solution or update on this?

[valeryq]

Same problem. When I tried to define new type in directive for field I get the same error.

My definition:

visitFieldDefinition(field) {
    field.type = new GraphQLObjectType({
      name: 'TestType',
      fields: {
        test: GraphQLString,
      },
    });
  }

Error on introspection:

buildClientSchema.js:102 Uncaught Error: Invalid or incomplete schema, unknown type: TestType. Ensure that a full introspection query is used in order to build a client schema.
    at n (buildClientSchema.js:102)
    at t (buildClientSchema.js:93)
    at i (buildClientSchema.js:116)
    at buildClientSchema.js:237
    at keyValMap.js:28
    at Array.reduce (<anonymous>)
    at r (keyValMap.js:27)
    at S (buildClientSchema.js:231)
    at fields (buildClientSchema.js:180)
    at E (definition.js:168)

[sandromartis]

Is there still no solution for this? Input field directives still break introspection in playground.

[yaacovCR]

Some issues are related to #1022, i.e. requirement to declare directives in schema with graphql-js v14. Planned workaround there is to have directive package authors also include instructions/exports for the directive sdl or to add option to assume valid sdl when calling buildSchema, although both less than ideal, definitely open to suggestions re all of above.

Other issues relate to constraint directive package itself which creates multiple different scalars of identical name, to attempt to avoid leaking these new constraint types via introspection, which is not officially supported, as far as I know, and makes the entire directive stop working in v5. For a different approach, see other packages available in #789.

Closing this issue for now, please feel free to reopen if more specific action needed.

[confuser]

@yaacovCR Why is this not officially supported when it's the recommended implementation on the docs which is also afflicted by this issue? https://www.apollographql.com/docs/apollo-server/schema/creating-directives/#enforcing-value-restrictions

[yaacovCR]

That example is still supported, notice the unique name of the created type, I believe that is the difference between supported example and your constraint directive.

I may be missing something, addressing this several months to years later. Ideally, I would like to continue to support the workflow you initiated, but I can't say that I see how at present.

[confuser]

That example is still supported, notice the unique name of the created type, I believe that is the difference between supported example and your constraint directive.

Could you elaborate on unique? The example shows the name is based on the max value, and shows two fields with the same directive max value, @length(max: 50) which is not unique. I don't recall the scalar type being unique or not as the cause of this particular problem. It's caused by a similar issue regarding schema healing described in #789.

[yaacovCR]

I meant unique in the sense that the length constraint is enforced by using a specific type with the appropriate length.

My understand of the issues in #789 relate to the desire to use validation by wrapping arguments rather than by using the serialized and parts functions within a dedicated scalar type. This requires using the directive to simply mark the schema with metadata and an additional function to wrap the required arguments all over the schema according to the metadata.

See #1234 to track potentially new approach to directives, ie functions that take a set of directive names as arguments and return a function that takes a schema that uses those directives in some way as an argument and returns a transformed schema.

[yaacovCR]

The reason why the constraint directive implementation no longer works in V5 is because schema healing has been reworked to no longer depend on -- or require -- mutation of the original schema.

This is part of an attempt throughout the code base to create immutable versions of all schema and type modification functions, as useful functions can then potentially be submitted to upstream graphql-js.

[mfellner]

We use the same approach as in the Apollo docs (https://www.apollographql.com/docs/graphql-tools/schema-directives/#enforcing-value-restrictions) to enforce a restriction on fields that have been annotated with a custom directive.

Even with unique names for each instance of the custom scalar type we're seeing the Invalid or incomplete schema error in GraphQL playground. Is there any known workaround?

I noticed that the error disappears if we declare the name(s) of the custom scalars with scalar MyScalarType in the SDL. However in that case the custom scalar type's serialize and parse methods are never called and the implementation no longer works.

We'd like to enforce restrictions on input type fields based on directive annotations in the SDL so there doesn't seem to be a different solution than using custom scalar types either?

[yaacovCR]

@mfellner , see below

We use the same approach as in the Apollo docs (https://www.apollographql.com/docs/graphql-tools/schema-directives/#enforcing-value-restrictions) to enforce a restriction on fields that have been annotated with a custom directive.

Hopefully that will work then, as we currently include tests that -- in theory -- should guarantee support.

Even with unique names for each instance of the custom scalar type we're seeing the Invalid or incomplete schema error in GraphQL playground. Is there any known workaround?

I noticed that the error disappears if we declare the name(s) of the custom scalars with scalar MyScalarType in the SDL.

The tests above assume the directive names are declared. This is annoying, see #1022, and comment above for thoughts about potentials for workarounds.

However in that case the custom scalar type's serialize and parse methods are never called and the implementation no longer works.

It should work. If you believe you are following the pattern in the documented example and associated linked test above, you should open a new issue as bug report with a minimal reproduction. I would suggest starting from the code within the test, and the creation of a failing test that mimics your code.

We'd like to enforce restrictions on input type fields based on directive annotations in the SDL so there doesn't seem to be a different solution than using custom scalar types either?

There is another alternative -- checking inputs on field resolution. See end of discussion at #789. Highlights are two relatively recent userland packages, which seems to me to be significant progress and required a great deal of thought and work by their authors:

1. apollo-validation-directives, by @barbieri
2. graphql-field-arguments-coercion, by @alexstrat

See, as well, suggestions for changes within the graphql spec referenced by @alexstrat at the end of the README for his project.

[yaacovCR]

@confuser et al, see #1468, I think for v6, we can restore the original v4 behavior without major issues so the directive will once again work. I don't think that will help with the underlying schema introspection issue, but at least we don't have to make the problem worse.

This is because the v5 schema healing that was causing a problem is really no longer in use within the code base as we have moved throughout to no longer modify schemas in place, but to rather create new schemas.

See #1463 for a directive-driven functional approach to schema modification that uses mapSchema to no longer modify the schema in place.

[yaacovCR]

@confuser v4 level compatibility for the constraint directive should hopefully be restored in v6 by #1419

[confuser]

Thanks @yaacovCR I've released v2 of graphql-constraint-directive which appears to be working well with v6

[yaacovCR]

Cool!

Just took a look, looks like you found a satisfactory approach for unique type names.

Your code made me think of something -- it looks like you are not letting your directive users customize the directive name. (https://github.com/confuser/graphql-constraint-directive/blob/master/index.js#L51) Did you think we didn't highlight well enough how to do that, or did you just choose not to?

I think we did forget to give an example of how you could export a directive definition that is also customizable with a type name. It would be fairly straightforward to export it as a function that embeds the desired directive name within the string.

This is all to pre-emptively avoid namespace clashes between different directives.

I think the constraint directive seems to be one of the more popular directives, and I suppose unlikely to cause problems by staking out a claim to that name first. You could always add the ability to customize the name later...

[confuser]

@yaacovCR I chose not to at this time to avoid fragmentation. The examples are clear apart from Declaring schema directives which falls somewhat short of passing a custom namespace.

The authDirective example shows an export of the type defs and a function to pass in the custom namespace, but it's not clear how that namespace is passed to the authDirectiveTypeDefs. Would suggest a variance in the example to perhaps make that clearer. Something along the lines of this would cover both:

const { authDirective, authDirectiveTypeDefs } = require('fake-auth-directive-package')('auth');

[yaacovCR]

Thanks, I gave it a try...