TriPSs / nestjs-query

Easy CRUD for GraphQL.
https://tripss.github.io/nestjs-query/
MIT License
164 stars 46 forks source link

Cannot use 2 offset and cursor pagination at the same time #201

Closed ValentinVignal closed 1 year ago

ValentinVignal commented 1 year ago

Describe the bug

I have an entity todo-item, and I'm trying to create 2 resolvers:

Have you read the Contributing Guidelines?

Yes

To Reproduce Steps to reproduce the behavior:

  1. Follow the steps to install the dependencies and create the example from https://tripss.github.io/nestjs-query/docs/introduction/example (or checkout https://github.com/ValentinVignal/reproducible/tree/ptc-org/nestjs-query/cursor-and-offset-pagination)
  2. Update the todo item module resolvers to :
      resolvers: [
        {
          DTOClass: TodoItemDTO,
          EntityClass: TodoItemEntity,
          create: { disabled: true },
          update: { disabled: true },
          delete: { disabled: true },
          read: {
            one: { disabled: true },
            many: {
              name: 'todoItemCursor',
            },
          },
        },

        {
          DTOClass: TodoItemDTO,
          EntityClass: TodoItemEntity,
          pagingStrategy: PagingStrategies.OFFSET,
          create: { disabled: true },
          update: { disabled: true },
          delete: { disabled: true },
          read: {
            one: { disabled: true },
            many: {
              name: 'todoItemOffset',
            },
          },
        },
      ],
  1. run npm run start

Expected behavior

I would want to have 2 resolvers, one with cursor pagination and one with offset pagination.

Current behavior

Instead I get the error:

/Users/valentin/Perso/Projects/reproducible/node_modules/graphql/type/schema.js:219
        throw new Error(
              ^
Error: Schema must contain uniquely named types but contains multiple types named "TodoItemConnection".
    at new GraphQLSchema (/Users/valentin/Perso/Projects/reproducible/node_modules/graphql/type/schema.js:219:15)
    at GraphQLSchemaFactory.create (/Users/valentin/Perso/Projects/reproducible/node_modules/@nestjs/graphql/dist/schema-builder/graphql-schema.factory.js:38:24)
    at GraphQLSchemaBuilder.generateSchema (/Users/valentin/Perso/Projects/reproducible/node_modules/@nestjs/graphql/dist/graphql-schema.builder.js:35:52)
    at GraphQLSchemaBuilder.build (/Users/valentin/Perso/Projects/reproducible/node_modules/@nestjs/graphql/dist/graphql-schema.builder.js:22:31)
    at GraphQLFactory.generateSchema (/Users/valentin/Perso/Projects/reproducible/node_modules/@nestjs/graphql/dist/graphql.factory.js:27:69)
    at ApolloDriver.generateSchema (/Users/valentin/Perso/Projects/reproducible/node_modules/@nestjs/graphql/dist/drivers/abstract-graphql.driver.js:23:36)
    at GraphQLModule.onModuleInit (/Users/valentin/Perso/Projects/reproducible/node_modules/@nestjs/graphql/dist/graphql.module.js:109:54)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at callModuleInitHook (/Users/valentin/Perso/Projects/reproducible/node_modules/@nestjs/core/hooks/on-module-init.hook.js:51:9)
    at NestApplication.callInitHook (/Users/valentin/Perso/Projects/reproducible/node_modules/@nestjs/core/nest-application-context.js:223:13)

Desktop (please complete the following information):

I tried to see if there was a way to give a custom name to the connections, but I couldn't find it. Maybe a connectionName parameter could be added somewhere so I could specify connectionName: 'TodoItemCusorConnection' and connectionName: 'TodoItemOffsetConnection'

ValentinVignal commented 1 year ago

I see it is possible to provide a connectionName to the ReadResolverOpts<DTO> :

          read: {
            connectionName: 'TodoItemOffsetConnection',
            one: { disabled: true },
            many: {
              name: 'todoItemOffset',
            },
          },

but it is being overridden in https://github.com/TriPSs/nestjs-query/blob/6f58030695cfb774d591cc2af2910ebc1e115f03/packages/query-graphql/src/resolvers/read.resolver.ts#L61

ValentinVignal commented 1 year ago

For the ones looking for a workaround, I managed to got both by doing:

 resolvers: [
        {
          DTOClass: TodoItemDTO,
          EntityClass: TodoItemEntity,
          create: { disabled: true },
          update: { disabled: true },
          delete: { disabled: true },
          read: {
            one: { disabled: true },
            many: {
              name: 'todoItemCursor',
            },
          },
        },
        {
          DTOClass: TodoItemDTO,
          dtoName: 'TodoItemOffsetPaginated',
          EntityClass: TodoItemEntity,
          pagingStrategy: PagingStrategies.OFFSET,
          create: { disabled: true },
          update: { disabled: true },
          delete: { disabled: true },
          read: {
            one: { disabled: true },
            many: {
              name: 'todoItemOffset',
            },
          },
        },
      ],

But I think this issue should stay open as connectionName doesn't seem to do anything because it always overridden

TriPSs commented 1 year ago

Hi @ValentinVignal, thanks for reporting, will try to check this out soon.

ValentinVignal commented 1 year ago

@TriPSs I wouldn't mind trying to work on this, I sent you a message for some help/advices

TriPSs commented 1 year ago

@ValentinVignal that would be awesome, thanks! Let me know if you need any help.

ValentinVignal commented 1 year ago

How should I contact you if I have some questions ?

TriPSs commented 1 year ago

You can comment here or if you have a more general question inside discussions, I receive emails for all notifications and tend to reply when I see them :)

TriPSs commented 1 year ago

@ValentinVignal FYI: I also created a slack (See README), let's see if that works.