Graphql: Usage of @ResolvePoperty() causes multiple instantiations of resolver

[scomans]

I'm submitting a...

[ ] Regression 
[x] Bug report
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

When a @ResolveProperty() function gets executed and a request scoped service was injected into the resolver a new instance of a resolver gets created each time a @ResolveProperty() function gets executed.

Expected behavior

Only one instance of the resolver and the service get created per request.

Minimal reproduction of the problem with instructions

https://github.com/xXKeyleXx/graphql-minimal-resolve-property

Execute the following query and watch the console output.

query {
  getTestsByIds(ids: ["1","2","3","4"]) {
    id
    parent {
      id
    }
  }
}

Comment out the property resolver and it works like expected.

What is the motivation / use case for changing the behavior?

If a new instance of a dataloader gets created each time a property gets resolved it defeates the purpose of the dataloader 😅

Environment

@nestjs/common: 6.2.4,
@nestjs/core: 6.2.4,
@nestjs/graphql: 6.2.1,
apollo-server-express: 2.5.0
type-graphql: 0.17.4
graphql: 14.3.0 

For Tooling issues:
- Node version: v10.15.3
- Platform:  Linux (Docker) / Windows

[mriegr]

Can confirm. We experienced the same problem on our company.

[vnenkpet]

Same here. Expected to use data loaders as a request-scoped providers, now I'm getting this kind of behavior per request:

Resolver code (stripped of irrelevant parts):

export class PostResolver {
  constructor(
    private readonly commentsDataLoader: CommentsDataLoader,
  ) {}
  @ResolveProperty('comments')
  public async comments(@Parent() call: CallEntity) {
    return this.commentsDataLoader.load(call.id);
  }
}
...

Code of a service instantiated using a request-scoped provider:

export class CommentsDataLoader {
  private id: string;

  constructor(
    private readonly: DataLoader<string, Comment[]>,
  ) {
    this.id = uuid.v4();
  }

  public async load(id: string) {
    console.log(`loader id=${id} called`);
    return this.dataLoader.load(id);
  }
}

Logs when querying a list of posts with their comments:

Loader instantiated
Loader instantiated
Loader instantiated
Loader instantiated
Loader instantiated
Loader instantiated
Loader instantiated
loader id=d0418150-6316-4ad9-bcbd-9909f9cea3a5 called
loader id=ca125463-96a5-445c-b36e-7c9bb203874c called
loader id=b616becb-bcbf-45e0-bc3a-8e684dac62ae called

(The last 3 calls should have the same ID, and the loader should have been instantiated only once).

Actually this isn't the only problem with request-scoped providers (or rather, I've already reported a similar one with Interceptors) and GraphQL, so I wonder, isn't there some bigger underlying issue? Hope these things are easily fixable.

[kamilmysliwiec]

Thanks for reporting! Could you please check @next release? $ npm i @nestjs/graphql@next

[vnenkpet]

@kamilmysliwiec Nothing changes, still not working.

[vnenkpet]

@kamilmysliwiec Actually, I take that back, I forgot to rebuild my docker image to get upgraded dependencies. But now I started having a different problem (so no idea yet if the request-scoped provider works):

api_1       | (node:82) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'getObjectType' of undefined
api_1       |     at definitions.forEach.def (/home/app/node_modules/type-graphql/dist/metadata/metadata-storage.js:126:92)
api_1       |     at Array.forEach (<anonymous>)
api_1       |     at MetadataStorage.buildFieldResolverMetadata (/home/app/node_modules/type-graphql/dist/metadata/metadata-storage.js:122:21)
api_1       |     at MetadataStorage.build (/home/app/node_modules/type-graphql/dist/metadata/metadata-storage.js:77:14)
api_1       |     at Function.generateFromMetadataSync (/home/app/node_modules/type-graphql/dist/schema/schema-generator.js:29:51)
api_1       |     at Function.<anonymous> (/home/app/node_modules/type-graphql/dist/schema/schema-generator.js:16:33)
api_1       |     at Generator.next (<anonymous>)
api_1       |     at /home/app/node_modules/tslib/tslib.js:107:75
api_1       |     at new Promise (<anonymous>)
api_1       |     at Object.__awaiter (/home/app/node_modules/tslib/tslib.js:103:16)
api_1       | (node:82) 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)
api_1       | (node:82) [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.

My current package.json:

    "@nestjs/common": "6.1.1",
    "@nestjs/core": "^6.1.1",
    "@nestjs/graphql": "^6.2.2",
    "@nestjs/jwt": "^6.1.0",
    "@nestjs/passport": "^6.1.0",
    "@nestjs/platform-express": "^6.1.1",

[kamilmysliwiec]

It sounds more like a circular dependency issue in your code.

[vnenkpet]

You're right, but it seems like this was caused by the resolveProperty method on type-graphql's union working in @next. I had to remove workaround on a resolver. All good then. 👍

[scomans]

It seems to be fixed for us too 👍

[kamilmysliwiec]

Fix published as 6.2.3

[lock[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.