enums are broken after transformSchema

[nfantone]

This is a follow up to a sub-issue derived from #1006 - opening a new one hoping it'll get more attention.

I have a simple schema defining an enum and a custom resolver for it.

enum PersonType {
  ADULT
  CHILDREN
  INFANT
}

{
  PersonType: {
    ADULT: 'ADT',
    CHILDREN: 'CHD',
    INFANT: 'INF'
  }
}

This works fine in queries/mutations if I use the schema returned by makeExecutableSchema, but doesn't if using the schema returned by transformSchema. Queries that would return a PersonType all end up with the following error:

{
  "timeThrown": "2019-01-09T20:00:11.786Z",
  "name": "GraphQLError",
  "message": "Expected a value of type \"PersonType\" but received: \"ADULT\""
}

If you don't ask for this particular field in your response, everything works just fine.

I'm using transformSchema to filter out some fields - but found out that even if I don't pass in any transformations (which I'd expect to return an exact copy of the given schema), the error is still there. Basically:

/* schema.js */
const schema = makeExecutableSchema({ ... });

// Works as expected
// module.exports = schema;

// Breaks enums (empty transformations array)
module.exports = transformSchema(schema, []);

This happens for every other enum in my schema. So there's evidently some difference between the original schema and the returned value from transformSchema. Schema introspection reveals the enum values correctly for both cases, however. And, after debugging, I can confirm that the definition for PersonType is present in both of them.

Debug breakpoint before return line of transformSchema function. targetSchema is the original while schema is the result after applying transformations.

Moreover, here are the resolve functions for the passengerType field (the enum on the query output): original schema (targetSchema) vs. transformed schema (schema).

You can see that they are most definitely not the same, starting from its name ("defaultMergedResolver").

Any advice on this?

[pthrasher]

I think the main issue here stems from your override. The GraphQL spec appears to claim that enums are meant to be what you said they were, and not to have custom resolvers.

https://graphql.github.io/graphql-spec/draft/#sec-Enums

My guess would be that a bug in graphql-js / graphql-tools is what allowed this to work for you at all in the first place.

I'm not a contributor on this project... was hunting for something else in issues and stumbled across your post which made me curious.

I know the above info is not super helpful, but my suggestion would be to not have custom resolvers for enums. Enums should return the value shown in the schema.

[nfantone]

@pthrasher Defining custom resolvers for enum values is absolutely supported by Apollo, according to its documentation. Whether they should do this or not as per spec, is another discussion altogether. Regardless, the fact remains that transformSchema result does not conform to the schema being passed.

[yaacovCR]

@nfantone, I am having trouble reproducing this with direct execution via graphql. Are you able to share a repo that reproduces this error?

[633kh4ck]

Same issue (using mergeSchemas), any updates?
@nfantone Did you find a solution (or at least workaround)?

[nfantone]

@633kh4ck We ended up not using schema transformations at all. As it turned out, for our use case (filtering some queries/mutations), graphql-import works wonders.

[yaacovCR]

Can you share a more complete code example? Would love to work on fixing this if I could reproduce it.

Looks like this area is covered in test suite...

https://github.com/apollographql/graphql-tools/blob/305466b18d510d6e27493a326a6110d0dfafb64d/src/test/testMergeSchemas.ts#L624

[nfantone]

@yaacovCR Good to read there's movement over here! Unfortunately, I opened this over 5 months ago and I'm no longer trying to use transformSchema in any of our projects.

However, I remember that reproducing the problem was as simple as setting up a schema as was described in the original issue:

1. Declare an enum and give it a custom resolver.
2. Then, call transformSchema(schema, []) and use its result to mount your /graphql.
3. Finally, try running any query that would read the enum field.

Maybe the problem was fixed already during this time? Also, I'm not entirely sure what that unit test you linked is trying to evaluate, but the resulting enum definitions from both schemas are most definitely not the same, as demonstrated in my analysis some months ago. A deep equality comparison should fail. In my use case, I'm not explicitly using mergeSchemas, but transformSchema, though.

[yaacovCR]

The unit test is comparing the original result with the merged result. The resolvers in the merged schema are wrapped, but (a) the tests don't crash and (b) the results are the same, so the different resolvers are supposedly working correctly. I think this issue may have been fixed, but I could feel differently very soon if I saw a more fleshed out recent reproduction. I tried from the original issue and it looked ok to me. :)

In general, not sure whether there is movement here... I opened up graphql-tools-fork to merge my own PRs and I am working on fixing what I can there.

[yaacovCR]

I was able to reproduce -- it had been fixed in root fields returning enum, but not nested fields.

This should be fixed now in graphql-tools-fork v5.2.0. Let me know!

(As always, I will try to separate this out to PR for upstream graphql-tools, but this may be difficult for this fix, as relies on my refactoring from other fixes.)

[yaacovCR]

I am not great at predicting the future, but I don't see this getting merged to upstream graphql-tools any time soon.

In general, I would love to hear from the package maintainers... It has been radio silence from them re: pull requests and issues for some time, hence the fork.

I would love to just bring everything back in, it's just so far a handful of (important?) bug fixes and a few new transformers.

[wmertens]

I can confirm that graphql-tools-fork fixes this issue I had with mergeSchemas

[kamilkisiela]

We recently released an alpha version of GraphQL Tools (#1308) that should fix the issue.

Please update graphql-tools to next or run:

npx match-version graphql-tools next

Let us know if it solves the problem, we're counting for your feedback! :)

[yaacovCR]

Rolled into #1306