directives are lost when using custom scalars

[theClarkSell]

I'm clearly missing something fundamental and I'm hoping someone here could point me in the right direction. I've been working through trying to understand all the ins and outs of Directives and vistScalar and visitInterface seem to have lead me to nothing but dead ends.

If one creates a directive aligned to SCALAR. When does visitScalar actually fire?

directive @onScalar on SCALAR

scalar PositiveInt @onScalar

type Session {
  id: ID!
  title: String
  rating: PositiveInt
}


class OnScalar extends SchemaDirectiveVisitor {
  visitScalar(scalar) {
    console.log('visitScalar');
  }
}

Thank you in advance!

[ardatan]

Could you share a reproduction of your issue as a GitHub repo or CodeSandbox?

[theClarkSell]

For sure of course. https://codesandbox.io/s/visitscalar-example-8zxsr. I tried to break down all of the different pieces pretty small. of course the example domain is just a bogus scenario.

Sample Query

query{ sessions {
  title
  rating
}}

[yaacovCR]

They should fire on scheme creation....

https://github.com/ardatan/graphql-tools/blob/master/packages/schema/src/makeExecutableSchema.ts#L76
https://github.com/ardatan/graphql-tools/blob/master/packages/utils/src/SchemaDirectiveVisitor.ts#L209
https://github.com/ardatan/graphql-tools/blob/master/packages/utils/src/visitSchema.ts#L180

[yaacovCR]

Can't seem to see logs from sandbox, when does it fire for you, or is it not firing?

[theClarkSell]

@yaacovCR I was just seeing them in the terminal window, but I simplified it to just be a console.log statement. As for should fire on schema creation, agree. I saw that code as too, but I'm not seeing that happen in my setup, like I see the other directives. I'm guessing, I just don't know what I'm doing, but I'm at a loss to what I'm doing wrong.

[yaacovCR]

graphql-tools since at least v4 overwrites the astNode from the original schema with the empty astNode from the custom scalar, which is likely undefined.

We could at least check for undefined before overwriting....

https://github.com/ardatan/graphql-tools/blob/master/packages/schema/src/addResolversToSchema.ts#L145
https://github.com/ardatan/graphql-tools/blob/master/packages/schema/src/addResolversToSchema.ts#L147
https://github.com/ardatan/graphql-tools/blob/master/packages/schema/src/addResolversToSchema.ts#L252
https://github.com/ardatan/graphql-tools/blob/master/packages/schema/src/addResolversToSchema.ts#L254

Likely affects enums with internal values as well, additional types, etc, etc.

[yaacovCR]

Workaround for now is to only select properties you want from custom scalar, ie parseValue, parseLiteral, serialize...

[yaacovCR]

Looks like i got it working...

See comment on PR for instructions on how to test: #1623 (comment)

See forked sandbox: https://codesandbox.io/s/visitscalar-example-6oxvj?file=/graphql/index.js

[yaacovCR]

@csell5 if you are able to confirm fix, would be great, another set of eyes would be much appreciated

[theClarkSell]

Verified... Thank you so much for getting after this so fast. What's the thinking behind not having a resolver function on some of these directives? I expected all directives to essentially execute the same.

[theClarkSell]

@yaacovCR Well, maybe there is still a bug here. It appears to only fire when makeExecutableSchema is called. See here:

https://codesandbox.io/s/visitscalar-example-8zxsr?file=/graphql/index.js

If you just pass in typedefs, resolvers, and the schemaDirectives to apollo server it will not fire. But if you either call makeExecutableSchema or take the result of makeExecutableSchema and pass it in, then it fires.

[yaacovCR]

Nothing we can do about that, we are now separate from Apollo, which is internally using v4, to use latest fixes, you have to explicitly construct schema with makeExecutableSchema

[yaacovCR]

@csell5 could you explain what you mean a little bit more in terms of your question about resolver function on some directives?

Maybe a code sample comparing what you expected and what you are seeing

[yaacovCR]

I apologize if it's kind of obvious my brain seems to be working a little slowly today

[theClarkSell]

Nothing we can do about that, we are now separate from Apollo, which is internally using v4, to use latest fixes, you have to explicitly construct schema with makeExecutableSchema

Right of course.. I wasn't even thinking about that.

[yaacovCR]

@csell5 and if you have a second to explain your question here?

#1587 (comment)

[theClarkSell]

@csell5 could you explain what you mean a little bit more in terms of your question about resolver function on some directives?

Maybe a code sample comparing what you expected and what you are seeing

Sure.. Let's just look at vistFieldDefinition and visitInputFieldDefinition visitFieldDefinition get's passed a field which on it has a resolve function that you can override and it will later get called on each request. There you have the opportunity either modify it, or act in someway. On the flip side of that directive we have visitInputFieldDefinition but it has no resolve function to override. Same would be true for visitScalar. It just seemed odd that there wouldn't be a resolve function on every directive regardless of if making sense to get called during a request. I've seen ya'll example of dynamically changing the types to a different scalar, but then why even bother with a directive other than for the arguments to pass along to the scalar.

Again, I'm just trying to get a grasp on the thinking here.

[ardatan]

Available in https://github.com/ardatan/graphql-tools/releases/tag/v6.0.10