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

Custom scalar args broken as query variables after schema transform #18

Closed
AndKiel opened this issue Aug 23, 2019 · 6 comments
Closed

Comments

@AndKiel
Copy link

AndKiel commented Aug 23, 2019

Given:

  • Custom ScalarType X with a class representing it internally
  • Query or Mutation with an argument of ScalarType X
  • Any schema transforms

When:

  • Required argument of ScalarType X is passed as query variable ($argScalar) instead of inline value inside the query

Then:

  • Weird stuff happens, see the reproduction and results

Minimalistic reproduction:

import { ApolloServer } from "apollo-server-express";
import { createTestClient } from "apollo-server-testing";
import { GraphQLScalarType, GraphQLSchema, Kind } from "graphql";
import { GraphQLResponse } from "graphql-extensions";
import { FilterRootFields, transformSchema } from "graphql-tools-fork";
import {
  Arg,
  buildSchemaSync,
  Query,
  Resolver
} from "type-graphql";

function createTestSchema(): GraphQLSchema {
  class MyCustomScalar {
    private readonly prefix = "Xxx-";

    constructor(private value: string) {}

    public encode(): string {
      return this.prefix + this.value;
    }

    public static parse(value: string): MyCustomScalar {
      if (typeof value === "string") return new MyCustomScalar(value.slice(4));
      console.log("-> inside .parse", typeof value, value);
      throw new Error("SOMETHING WENT WRONG! SHOULD NEVER HAPPEN!");
    }
  }

  const MyCustomScalarType = new GraphQLScalarType({
    name: "MyCustomScalar",
    parseValue: (value: string): MyCustomScalar => {
      console.log("-> inside .parseValue", typeof value, value);
      return MyCustomScalar.parse(value)
    },
    serialize: (value: MyCustomScalar): string => {
      console.log("-> inside .serialize");
      return value.encode()
    },
    parseLiteral: (ast): MyCustomScalar | undefined => {
      console.log("-> inside .parseLiteral");
      if (ast.kind === Kind.STRING) {
        return MyCustomScalar.parse(ast.value)
      }
      return undefined;
    }
  });

  @Resolver()
  class MyTestResolver {
    @Query()
    public queryWithScalarArg(@Arg("id") id: MyCustomScalar): MyCustomScalar {
      return id;
    }
  }

  return buildSchemaSync({
    resolvers: [MyTestResolver],
    scalarsMap: [
      {
        type: MyCustomScalar,
        scalar: MyCustomScalarType
      }
    ]
  });
}

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

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

describe("schema transformation", () => {
  it("preserves custom scalars on arguments when variables are used", async () => {
    const scalarValue = "Xxx-scalarValue";
    const query = "query MyTestQuery($argScalar: MyCustomScalar!) { result: queryWithScalarArg(id: $argScalar) }";
    const variables = { argScalar: scalarValue };
    console.log("> executing query on original schema");
    const response = await execute(schema, query, variables);
    console.log("> executing query on transformed schema");
    const transformedResponse = await execute(transformedSchema, query, variables);

    expect(transformedResponse).toEqual(response);
    expect(transformedResponse.errors).toBeUndefined();
    expect(transformedResponse.data).toEqual({
      result: scalarValue
    });
  });
});

Expectation:

- Expected
+ Received

-   "data": Object {
-     "result": "Xxx-scalarValue",
-   },
-   "errors": undefined,
+   "data": null,
+   "errors": Array [
+     [GraphQLError: Variable "$argScalar" got invalid value { value: "scalarValue", prefix: "Xxx-" }; Expected type MyCustomScalar. SOMETHING WENT WRONG! SHOULD NEVER HAPPEN!],
+   ],

What is being logged:

    > executing query on original schema
    -> inside .parseValue string Xxx-scalarValue
    -> inside .serialize
    > executing query on transformed schema
    -> inside .parseValue string Xxx-scalarValue
    -> inside .parseValue object MyCustomScalar { value: 'scalarValue', prefix: 'Xxx-' }
    -> inside .parse object MyCustomScalar { value: 'scalarValue', prefix: 'Xxx-' }
@yaacovCR
Copy link
Owner

Thanks for great reproduction!

@yaacovCR
Copy link
Owner

My guess is that it is because I am passing an empty object to delegateToSchema rather than the args here:

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

I did that because args are unchanged, but I forgot that any custom stuff needs to be reserialized. This should get better covered by tests if indeed the cause.

@AndKiel
Copy link
Author

AndKiel commented Aug 26, 2019

@yaacovCR, thanks, the fix is working, my tests are passing.
But I see that some graphql-tools-fork tests are failing due to incompatible types. Hopefully, nothing really broke and maybe the typings are incorrect.

@yaacovCR
Copy link
Owner

yaacovCR commented Aug 26, 2019

Saw that too, was not on my personal set up, I'm thinking it has to do with latest update to graphql. Will have to check later with a clean install with the latest updates of all packages

@yaacovCR
Copy link
Owner

graphql/graphql-js#2104

@yaacovCR
Copy link
Owner

Fixed.

yaacovCR added a commit that referenced this issue Sep 22, 2019
yaacovCR added a commit that referenced this issue Sep 22, 2019
yaacovCR added a commit that referenced this issue Sep 22, 2019
yaacovCR added a commit that referenced this issue Sep 22, 2019
yaacovCR added a commit that referenced this issue Oct 3, 2019
yaacovCR added a commit that referenced this issue Oct 25, 2019
yaacovCR added a commit that referenced this issue Oct 25, 2019
yaacovCR added a commit that referenced this issue Nov 4, 2019
yaacovCR added a commit that referenced this issue Dec 31, 2019
yaacovCR added a commit that referenced this issue Dec 31, 2019
yaacovCR added a commit that referenced this issue Jan 8, 2020
yaacovCR added a commit that referenced this issue Jan 21, 2020
yaacovCR added a commit that referenced this issue Feb 27, 2020
yaacovCR added a commit that referenced this issue Mar 26, 2020
yaacovCR added a commit that referenced this issue Mar 26, 2020
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