Skip to content
This repository has been archived by the owner on Apr 15, 2020. It is now read-only.

Enums on fragments are broken after schema transform #13

Closed
AndKiel opened this issue Jul 18, 2019 · 5 comments
Closed

Enums on fragments are broken after schema transform #13

AndKiel opened this issue Jul 18, 2019 · 5 comments

Comments

@AndKiel
Copy link

AndKiel commented Jul 18, 2019

Given a union type where one of the members has an enum field, query results with an error GraphQLError: Expected a value of type "MyCustomEnum" but received: "EnumValue". Enums on fragments are broken after schema transforms. It probably also doesn't work for enum arrays.

@yaacovCR
Copy link
Owner

This seems fixable as we currently perform reparsing of enums based on the simple info.returnType of a field without checking for unions. Lists should work, though. Can you check?

See https://github.com/yaacovCR/graphql-tools-fork/blob/master/src/stitching/checkResultAndHandleErrors.ts#L70

You could maybe move the pace of a fix along just by including more of your test case code? Or submitting a PR for handling unions similar to how lists are handled.

I do not think this should be any different whether you are using fragments or not. Can you check that, too?

Thanks for submitting!

@AndKiel
Copy link
Author

AndKiel commented Jul 18, 2019

Here's a snippet with a minimalistic reproduction:

import { ApolloServer } from "apollo-server-express";
import { createTestClient } from "apollo-server-testing";
import { GraphQLSchema } from "graphql";
import { GraphQLResponse } from "graphql-extensions";
import { FilterRootFields, transformSchema } from "graphql-tools-fork";
import { buildSchemaSync, createUnionType, Field, ObjectType, Query, registerEnumType, Resolver } from "type-graphql";

function createTestSchema(): GraphQLSchema {
  enum TestEnum {
    One = "bug",
    Two = "issue"
  }

  registerEnumType(TestEnum, {
    name: "TestEnum"
  });

  @ObjectType()
  class TestObject {
    @Field(_type => TestEnum)
    public type: TestEnum = TestEnum.One;
  }

  @ObjectType()
  class SecondTestObject {
    @Field(_type => TestEnum)
    public type: TestEnum = TestEnum.Two;

    @Field(_type => [TestEnum])
    public typeArray: TestEnum[] = [TestEnum.One, TestEnum.Two]
  }

  const testUnionType = createUnionType({
    name: "TestUnionType",
    types: [TestObject, SecondTestObject]
  });

  @Resolver(_of => TestObject)
  class TestResolver {
    @Query(_type => testUnionType)
    public getUnion(): TestObject | SecondTestObject {
      return new SecondTestObject();
    }
  }

  return buildSchemaSync({
    resolvers: [TestResolver]
  });
}

function execute(schema: GraphQLSchema, query: string): Promise<GraphQLResponse> {
  const server = new ApolloServer({ schema });
  return createTestClient(server).query({ query });
}

const schema = createTestSchema();
const transformedSchema = transformSchema(schema, [
  new FilterRootFields((_operation, _fieldName, _field) => true)
]);

it("preserves enum on fragment when schema is transformed", async () => {
  const query = "query { getUnion { ... on SecondTestObject { type } } }";
  const response = await execute(schema, query);
  const transformedResponse = await execute(transformedSchema, query);

  expect(response).toEqual(transformedResponse);
  expect(transformedResponse).toEqual({
    data: {
      getUnion: {
        type: "Two"
      }
    }
  })
});

it("preserves enum array on fragment when schema is transformed", async () => {
  const query = "query { getUnion { ... on SecondTestObject { typeArray } } }";
  const response = await execute(schema, query);
  const transformedResponse = await execute(transformedSchema, query);

  expect(response).toEqual(transformedResponse);
  expect(transformedResponse).toEqual({
    data: {
      getUnion: {
        type: ["One", "Two"]
      }
    }
  })
});

Covers both enum and enum array for union type.
It works fine when it's not a union.

@yaacovCR
Copy link
Owner

Looks like we should add another case here:

https://github.com/yaacovCR/graphql-tools-fork/blob/master/src/stitching/checkResultAndHandleErrors.ts#L73

Pseudocode: (isUnionType(type) && type.resolveType(value) === (GraphqlEnumType || GraphqlScalarType))

Do you want to give that a try in PR?

@yaacovCR
Copy link
Owner

I actually misunderstood your bug, I thought it was of unions of enums, not unions of object types with enum fields. (And it turns out you cannot even have union of scalar types, only that of object types!)

The problem in this case was therefore not in the parseOutputValue logic, but rather that defaultMergedResolver was not getting called for unions (and probably interfaces!)

That should now be fixed. Here's hoping.

@AndKiel
Copy link
Author

AndKiel commented Jul 19, 2019

Sorry for the confusion, I thought that the snippet made it clear.
It's fixed now, thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants