nestjs-query icon indicating copy to clipboard operation
nestjs-query copied to clipboard

Proposal for Enhanced GraphQL Polymorphism and Fragment Support via Explicit DTO Separation

Open smolinari opened this issue 4 months ago • 2 comments

I hope you won't shun this suggestion, because yes, I am assisted by AI in creating it. But, I've been working hard at all the alternatives and for what I wish to achieve, I need the suggestion below to happen.

I'm mainly looking for your knowledge and opinion on the idea and reasoning.

Worst case, I'll use a fork of the library to make this work, but maybe you might like the whole idea too and we can collaborate on it.

Again, I'm first and foremost looking for your input/ feedback. Thanks!

So here goes.... 😁

Hello,

We are currently leveraging nestjs-query in a platform application where we extensively use Typegoose's discrimination feature for various entities. This allows us to model polymorphic data effectively at the database layer. Imagine a number of different document types, but stored in the same collection. This is a powerful tool for schema extension and polymorphism.

Trying to use discriminators with Typegoose and Nestjs-Query, we've encountered significant challenges in exposing and querying this polymorphic data via GraphQL, particularly when attempting to use GraphQL fragments along with our newly added fixes for discriminators.

This write-up outlines a conceptual addition to nestjs-query's internal architecture that we believe would natively address these challenges, significantly improving the developer experience for complex, data-rich applications.


The Current Status Quo and the Problem

nestjs-query currently intertwines the concepts of an ORM/ ODM entity and its GraphQL Data Transfer Object (DTO). The DTOClass provided to NestjsQueryGraphQLModule.forFeature serves a dual purpose: it defines the GraphQL schema (via @ObjectType, @Field decorators) and acts as the type for the underlying QueryService to interact with the ORM/ ODM entity.

While this design simplifies common CRUD operations for direct entity-to-GraphQL mappings, it creates significant friction when dealing with polymorphism and GraphQL fragments:

  1. Difficulty with GraphQL Interface/Union Types: It's challenging to define a base DTO (e.g., TodoItemDTO) as a GraphQL InterfaceType and have its discriminated DTOs (e.g., TodoTaskDTO, TodoAppointmentDTO) automatically implement it. nestjs-query's current schema generation doesn't natively infer or create these polymorphic GraphQL types from ORM/ ODM discrimination metadata.
  2. Manual resolveType and Resolver Boilerplate: Without native support, we are forced to manually define GraphQL UnionTypes or InterfaceTypes and implement custom @ResolveField resolvers. These resolvers must manually fetch the underlying ORM/ ODM entity, determine its discriminated type, and then map it to the correct concrete GraphQL DTO. This negates much of nestjs-query's automation.
  3. Lack of Native Fragment Support: The core problem manifests as GraphQL validation errors when attempting to use fragments on polymorphic fields. GraphQL expects a field to be typed as an Interface or Union to allow fragments,but nestjs-query requires DTOs to be concrete ObjectTypes, and thus, true polymorphism isn't available, leading to errors like "Fragment cannot be spread here as objects of type "TodoItem" can never be of type "TodoTask"."

Proposed Conceptual Change: Explicit DTO Separation

We propose a conceptual separation within nestjs-query between:

  1. ORM/ODM Entity DTOs (Internal Representation): A plain TypeScript class that strictly mirrors the properties of the ORM/ ODM entity (e.g., TodoItemEntityDTO). This would be the internal representation that nestjs-query's Assemblers and QueryServices primarily work with. It would include the discriminator key (e.g., documentType in Mongoose or dtype for TypeORM).
  2. GraphQL DTOs (External Representation): The classes decorated with @nestjs/graphql decorators (e.g., TodoItemDTO, TodoTaskDTO, TodoAppointmentDTO). These would define the GraphQL schema, including InterfaceTypes and ObjectTypes that implement them.

How it would work:

  • ORM/ODM Modules (nestjs-query-typegoose, nestjs-query-typeorm, nestjs-query-mongoose(theoretically)): These modules would be responsible for:
    • Registering the ORM entities and their discrimination metadata (base entity, discriminator key, mapping of discriminator values to concrete entity classes).
    • Providing a mechanism to generate or infer the ORM Entity DTOs from the ORM/ ODM entities.
  • Core nestjs-query-graphql: This module would be enhanced to:
    • Detect Discrimination: When configuring NestjsQueryGraphQLModule.forFeature, it would receive metadata from the ORM modules indicating if a DTO is part of a discrimination hierarchy.
    • Automated InterfaceType Generation: If a DTO is identified as the base of a discrimination, NQG would automatically generate a GraphQL InterfaceType for it.
    • Automated implements Clause: Discriminated DTOs would be generated as ObjectTypes that automatically implement the generated InterfaceType.
    • Automated resolveType Injection: For any GraphQL field returning the generated InterfaceType (or a list of it), NQG would automatically inject a resolveType function. This function would inspect the discriminatorKey on the underlying ORM/ODM Entity DTO (or the raw entity) and return the correct concrete GraphQL ObjectType.
    • Refined Assemblers: Assemblers would be responsible for mapping between the ORM/ODM Entity DTOs and the GraphQL DTOs, handling the polymorphic conversion.

Benefits of this Change (Solving the Problems)

This conceptual shift would provide significant benefits, particularly for platform development:

  1. Native GraphQL Polymorphism: nestjs-query would natively generate GraphQL InterfaceTypes and handle the resolveType logic for discriminated entities, aligning with GraphQL best practices.
  2. True Fragment Support: This directly solves the Fragment cannot be spread here error, enabling clients to use fragments for querying polymorphic data seamlessly.
  3. Clearer Separation of Concerns: Decouples the database schema from the GraphQL API schema, allowing each to evolve more independently.
  4. Scalability for Platforms: For applications with potentially thousands of discriminated entities, this automation would drastically reduce the need for manual UnionType definitions and custom resolvers, eliminating a massive amount of boilerplate code.
  5. Improved Developer Experience: Developers would simply define their ORM/ ODM specialized entity DTOs and their GraphQL DTOs (with the base DTO as an InterfaceType), and nestjs-query would handle the complex polymorphic mapping and schema generation automatically, providing the proper and massively powerful polymorphic abstraction desired.

Concrete Examples from Our typegoose-discriminators E2E Test (Current Failures)

Consider our TodoItem example, where TodoTask and TodoAppointment are discriminated types of TodoItem.

Current Failure Example 1: Querying all TodoItems with Fragments In our e2e test, we attempt to query all TodoItems and use fragments to retrieve type-specific fields:

query GetAllTodoItemsWithDetails {
  todoItems { # This field currently returns [TodoItemDTO!]! where TodoItemDTO is an ObjectType
    id
    title
    documentType
    ... on TodoTask { # This fragment fails
      dueDate
    }
    ... on TodoAppointment { # This fragment fails
      location
    }
  }
}

This query currently fails with errors like: "Fragment cannot be spread here as objects of type "TodoItem" can never be of type "TodoTask"." This is because TodoItemDTO is generated as a concrete ObjectType, not an InterfaceType that TodoTaskDTO and TodoAppointmentDTO implement.

Current Failure Example 2: SubTask Relation to TodoItem When SubTaskDTO has a one-to-one relation to TodoItemDTO:

// In SubTaskDTO
@Relation('todoItem', () => TodoItemDTO, { /* ... */ })
todoItem!: TodoItemDTO;

This currently fails if TodoItemDTO is an InterfaceType because @Relation expects a concrete ObjectType. We are forced to use a UnionType and a custom resolver, adding significant boilerplate.


We believe this enhancement would significantly elevate nestjs-query's capabilities for complex, real-world applications and address a critical pain point for developers working with polymorphic data. We would greatly appreciate your thoughts and feedback on this proposed conceptual change.

This also relates to this PR offering about TypeORM polymorphism too. I bet he hasn't tried to query across the polymorphic entities i.e. get all of them in one query via fragments. Because, it won't work. 😄

Thank you for your time and consideration.

Scott

smolinari avatar Jul 25 '25 07:07 smolinari

During the implementation of the custom service feature for discriminated DTOs, a fundamental bug was discovered in the CRUDResolver that prevented the use of custom service methods in any scenario, not just with discriminated DTOs. This is the example given in the docs for the custom service addition (for example with Typegoose).

@QueryService(TodoItemEntity)
export class TodoItemService extends TypegooseQueryService<TodoItemEntity> {
  constructor(@InjectModel(TodoItemEntity) model: ReturnModelType<typeof TodoItemEntity>) {
    super(model);
  }

  async markAllAsCompleted(): Promise<number> {
    const entities = await this.query({ filter: { completed: { is: true } } });

    const { updatedCount } = await this.updateMany(
      { completed: true }, // update
      { id: { in: entities.map((e) => e.id) } }, // filter
    );
    // do some other business logic
    return updatedCount;
  }
}

The module setup for this scenario:

@Module({
  imports: [
    NestjsQueryGraphQLModule.forFeature({
      imports: [NestjsQueryTypegooseModule.forFeature([TodoItemEntity])],
      services: [TodoItemService],
      resolvers: [
        {
          DTOClass: TodoItemDTO,
          ServiceClass: TodoItemService,
        },
      ],
    }),
  ],
})
export class TodoItemModule {}

The Issue: Failing to Build the Extra Mutation/ Query Resolver

Yup. If you use a custom service without a custom resolver too, the automated CRUD resolver gets created, but the resolver for the custom service methods won't be built.

The interesting thing about this is, none of the e2e tests cover this scenario. That was my first scratch-my-head moment.

During the troubleshooting, we (AI and I) had a long hard time figuring out why the library wasn't working. Only after supposing the library itself holds the cause of the problem, were we able to dig deeper and find the root cause.

The Bug: Incorrect Prototype Inspection and Missing Metadata

The CRUDResolver was intended to automatically create GraphQL queries and mutations for any custom methods defined on a provided ServiceClass. However, it failed for two reasons:

  1. Incorrect Prototype Inspection: The logic was using Object.getPrototypeOf(opts.ServiceClass.prototype), which incorrectly looked at the prototype of the class the service extended (e.g., TypegooseQueryService), not the service itself. This meant it could never find any custom methods.
  2. Unreliable Metadata Reflection: The resolver relied on Reflect.getMetadata('design:returntype', ...) to determine the GraphQL return type of the custom method. This is notoriously unreliable in certain compilation environments and for complex types, often returning undefined and causing the mutation to be skipped.

The Solution: The new @QueryServiceMethod Decorator

To create a robust and explicit solution, the following changes were made:

  1. New @QueryServiceMethod() Decorator: A new decorator was created in packages/query-graphql/src/decorators/query-service-method.decorator.ts. This decorator allows developers to explicitly provide the necessary metadata for a custom service method.

    // Usage in a custom service
    import { QueryServiceMethod } from '@ptc-org/nestjs-query-graphql';
    import { Int } from '@nestjs/graphql';
    
    // ...
    @QueryServiceMethod({ returnType: () => Int, type: 'mutation' })
    async myCustomMutation(): Promise<number> {
      // ...
    }
    

Note: The default type is mutation, and the other possible value is query. This allows the dev to determine the type of resolver Nestjs-Query should build for the service method.

  1. Updated CRUDResolver: The CRUDResolver was updated to look for metadata attached by the new @QueryServiceMethod() decorator. This bypasses the unreliable reflect-metadata lookup and provides a clear, explicit way to define custom mutations.

  2. Comprehensive Unit Tests: New unit tests were added for both the @QueryServiceMethod() decorator and the CRUDResolver to ensure this functionality is robust and protected against future regressions.

This fix not only resolves the issue for discriminated DTOs but also provides a stable and well-documented way for any user of the library to extend a QueryService with custom mutations.

I hope it is ok to package all this in one PR. If not, let me know.

Scott

smolinari avatar Jul 31 '25 15:07 smolinari

To that last post. This was sadly over-engineering on my part.

My initial thinking was that if I have a custom service added, the library should build out a resolver for it. I even got the feature working. But, after realizing that the new decorator would end up being a massive ugly options object, if the resolver needed to be anything other than something simple, it basically meant we'd be having the userland devs creating a custom resolver via config in the service method decorator. The proper path to using a custom service method (or more than one) is to simply require devland to create their own custom resolver too.

In that vein, I've also added tests for this scenario in the typegoose-discriminator e2e tests to demonstrate the custom overriding to enable specialized endpoints and services above and beyond the default CRUD generated by the library.

Scott

smolinari avatar Aug 02 '25 04:08 smolinari