Adrinalin4ik / Nestjs-Graphql-Tools

NestJS Graphql Tools is a flexible solution that provides a bunch of decorators for solving problems like n+1 request, filtering, sorting, pagination, polymorphic relation, graphql field extraction. It is fully based on decorators. To use it you can just add a decorator to your resolver.
GNU General Public License v3.0
80 stars 8 forks source link

Use dataloader with @ResolveReference #27

Closed ydomenjoud closed 1 year ago

ydomenjoud commented 1 year ago

First of all, specials thanks for this awesome library.

Describe the bug With Graphql Federation, you use @ResolveReference to resolve a reference to an entity. Unfortunately, @GraphqlLoader() and @Loader() annotations doesn't work with @ResolveReference. I ran into this error :

error {
  message: "Cannot read properties of undefined (reading 'path')",
  locations: [ { line: 1, column: 34 } ],
  path: [ '_entities', 0 ],
  extensions: {
    code: 'INTERNAL_SERVER_ERROR',
    stacktrace: [
      "TypeError: Cannot read properties of undefined (reading 'path')",
      '    at UsersResolver.descriptor.value (/work/users-application/node_modules/nestjs-graphql-tools/lib/loader.ts:125:51)',
      '    at /work/users-application/node_modules/@nestjs/core/helpers/external-context-creator.js:67:33',
      '    at processTicksAndRejections (node:internal/process/task_queues:96:5)',
      '    at async target (/work/users-application/node_modules/@nestjs/core/helpers/external-context-creator.js:74:28)'
    ]
  }
}

It seems to be related to line 96 in loader.ts , where ctx.getArgs() have different shape with graphql federation :

  const args = ctx.getArgs();
  const { req } = args[2];
  const info = args[3];

On federated graph, req is in args[1] and info in args[2] . Theses updates solved my issue :

export const Loader = createParamDecorator((_data: unknown, ctx: ExecutionContext) => {
    const args = ctx.getArgs();
    let req;
    let info;
    if(args.length < 4) {
        req = args[1].req;
        info = args[2]
    } else {
        req = args[2].req;
        info = args[3]
    }
return {
....

To Reproduce Steps to reproduce the behavior:

Expected behavior We expect no error and result return from the gateway

Screenshots If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

Additional context we can use also fieldName to search if this is a FederatedGraph by looking info.fieldName='_entities' too, or testing info.path.prev === undefined

Adrinalin4ik commented 1 year ago

@ydomenjoud thank you for such a good description. I'll try to sort it out a bit later and make you know once it's done.

Adrinalin4ik commented 1 year ago

@ydomenjoud this issue was fixed as part of 0.8.1 version. Please consider to upgrade to the latest version 0.8.2, it also include vulns fixes.

Adrinalin4ik commented 1 year ago

@ydomenjoud thank you for the description once again. I've added new section to readme related to federation and used your description there.

Adrinalin4ik commented 1 year ago

This issue was closed as it was fixed as part of 0.8.1 version.