Specifying query complexity in decorators does not take effect

[hjiung]

Describe the bug
TypeGraphQL documentation describes how to apply query complexity analysis based on graphql-query-complexity package. When actually applying it by specifying complexity for fields, whether with a simple number or with a function that takes field arguments and child complexity, both do not take effect, resulting in incorrect query complexity.

To Reproduce
I think any TypeGraphQL schema with non-default('default complexity' here means the defaultComplexity in graphql-query-complexity's simpleEstimator) complexity as a number or function would reproduce this bug, but for to be sure, minimal reproducible code(I can think of) is at this Repo. Clone this repo and build with tsc -b ./src, run with node ./dist/app.js will run the apollo server.
Now querying

query {
  human {
    id
    name
    age
  }
}

should return complexity of 8, since id field is 1, name field is 5(by complexity: 5 at its @Field decorator) and age field is 1, and lastly, human field by itself is 1. However, running this query will print out Used complexity: 4 instead of Used complexity: 8.

Expected behavior
Expect query complexity field to be accurately computed, wherever decorator the complexity is defined at as long as its valid one mentioned in docs, whatever type(number or function) it is described in.

Enviorment (please complete the following information):

• OS: Windows
• Node v10.15.3
• Package version 0.17.5
• TypeScript version 3.5.3

Additional context
While debugging into TypeGraphQL source code to find the cause, I think setting complextiy field at GraphQL type build time is not safe to use (at least) with graphql-query-complexity package. As I can understand, TypeGraphQL seems to build its own metadata with user-defined type definitions, then call graphql-js's GraphQLSchema constructor with that TypeGraphQL-built type information(metadata?). (Guessed from schema-generator.ts file.) But graphql-js does not take complexity into its consideration, hence not setting the field complexity to its finally built graphql type object.(Guessed from constructor of GraphQLScalarType(here), GraphQLObjectType(here) at graphql-js's 'src/type/definition.js file.) But graphql-query-complexity tries to get complexity from already-built-by-graphql-js-type (here), which does not have information about complexity TypeGraphQL has set, because it's been lost while building final graphql type objects by graphql-js. I don't know my guessing is correct or not, but if it is correct, by current method, complexity may not be set at all, resulting in all complexity falls back to simpleEstimator's defaultComplexity.

If I'm getting something wrong, please feel free to point them out!

[MichalLytek]

Cannot reproduce - please checkout the examples:
https://github.com/MichalLytek/type-graphql/tree/8e112d1c7148623781594a48a029ece4870f11b9/examples/query-complexity

If you use NestJS or buildTypeDefsAndResolvers, it won't work.

[hjiung]

I'll try with the example. FYI, I didn't use NestJS or buildTypeDefsAndResolvers, to be sure I'm following the docs correctly ;)

After trying with that example and still cannot get complexity working, I'll comment about the triage. Thx for the fast comment.

[hjiung]

Well... I'm seeing that when using graphql-js's lexicographicSortSchema function for schema built with buildSchema of TypeGraphQL, complexity doesn't take effect.

I copy-and-pasted your example, only slightly changing:

• ../../src imports to type-graphql imports
• @Resolver((of) => Recipe) to @Resolver(Recipe) at recipe-resolver.ts line 6.
  • It won't run when type function is given.
  • stack trace(path above project directory is omitted.):

    at definitions.forEach.def (C:\Users\...\typegraphql-query-complexity\node_modules\type-graphql\dist\metadata\metadata-storage.js:132:52)
    at Array.forEach (<anonymous>)
    at MetadataStorage.buildFieldResolverMetadata (C:\Users\...\typegraphql-query-complexity\node_modules\type-graphql\dist\metadata\metadata-storage.js:122:21)
    at MetadataStorage.build (C:\Users\...\typegraphql-query-complexity\node_modules\type-graphql\dist\metadata\metadata-storage.js:77:14)
    at Function.generateFromMetadataSync (C:\Users\...\typegraphql-query-complexity\node_modules\type-graphql\dist\schema\schema-generator.js:29:51)
    at Function.<anonymous> (C:\Users\...\typegraphql-query-complexity\node_modules\type-graphql\dist\schema\schema-generator.js:16:33)
    at Generator.next (<anonymous>)
    at C:\Users\...\typegraphql-query-complexity\node_modules\tslib\tslib.js:110:75
    at new Promise (<anonymous>)
    at Object.__awaiter (C:\Users\...\typegraphql-query-complexity\node_modules\tslib\tslib.js:106:16)
    (node:14156) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 2)
    (node:14156) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.```
    
    

With only 2 changes above, example code works correctly. It produces complexity point of 6 for graphql query

query {
  # Count of 1 will multiply 1 to child complexity.
  recipes(count: 1) {
    title # This will produce complexity of 1.
    ratingsCount # This will produce complexity of 5, defined as, at FieldResolver.
  }
  # (1 + 5) * 1 = 6, total complexity.
}

But after applying lexicographicSortSchema after buildSchema by changing

/// index.ts Line 11-13
const schema = await buildSchema({
    resolvers: [RecipeResolver],
});

with

/// index.ts Line 11-13
const schema = lexicographicSortSchema(await buildSchema({
    resolvers: [RecipeResolver],
}));

same graphql query will product complexity point of 3. Can this be confirmed?

[MichalLytek]

@Resolver((of) => Recipe) to @Resolver(Recipe) at recipe-resolver.ts line 6

It can't affect query complexity nor the schema build itself.

But after applying lexicographicSortSchema

I can confirm that as it strip off all additional properties of GraphQLObjectType.

https://github.com/graphql/graphql-js/blob/a5bbe713f4b704247d56616e522f3d855354b974/src/utilities/lexicographicSortSchema.js#L108-L110

You can raise an issue in graphql-js about that but they will they you that you should use extenstions property instead. So you should raise an issue in graphql-query-complexity repo about supporting reading the metadata from the extenstions property, not the config itself.

[hjiung]

As a workaround, below code snippet will produce the almost same effect with lexicographicSortSchema.

const schema = buildSchemaSync({
  resolvers,
});

// Manually sort lexicographically.
const sortedTypeMap: any = {};
const sortedQueryTypeFields: any = {};
const sortedMutationTypeFields: any = {};

for (const typeMapKey of Object.keys(schema.getTypeMap()).sort()) {
    sortedTypeMap[typeMapKey] = schema.getTypeMap()[typeMapKey];
}
(schema as any)._typeMap = sortedTypeMap;

for (const queryTypeFieldKey of Object.keys(schema.getQueryType()!.getFields()).sort()) {
    sortedQueryTypeFields[queryTypeFieldKey] = schema.getQueryType()!.getFields()[queryTypeFieldKey];
}
(schema as any)._queryType._fields = sortedQueryTypeFields;

for (const mutationTypeFieldKey of Object.keys(schema.getMutationType()!.getFields()).sort()) {
    sortedMutationTypeFields[mutationTypeFieldKey] = schema.getMutationType()!.getFields()[mutationTypeFieldKey];
}
(schema as any)._mutationType._fields = sortedMutationTypeFields;

(schema as any)._implementations.Node.sort((a: any, b: any) => a.name < b.name ? -1 : 1);

Anyway, I might as well raise an issue at graphql-query-complexity repo about using extensions property graphql-js provides.

Btw, for @Resolver decorator, I just want to mention that yes, it doesn't have any relation with query complexity issue, but it will raise an error I previously posted when used with type functions(e.g., @Resolver((of) => Recipe)). If this error keeps happening, I will post another issue regarding this.

[MichalLytek]

but it will raise an error I previously posted when used with type functions(e.g., @resolver((of) => Recipe)). If this error keeps happening, I will post another issue regarding this.

It should happen only when you use compilation to ES5 using babel or sth. Can you reproduce the issue in a new example repository?

[MichalLytek]

@hjiung
It should be fixed now via #437 🎉
Please npm i type-graphql@beta to install v0.18.0-beta.2 release with that changes 😉

[hjiung]

Sorry for checking late. I'll try with v0.18.0-beta2 version! Thanks ;)