Schema first code generation creates duplicate IQuery interfaces

[drieshooghe]

Is there an existing issue for this?

• I have searched the existing issues

Current behavior

When using the GraphQLFederationDefinitionsFactory to build type declarations based on multiple GraphQL schemas, a duplicate empty IQuery interface/class gets added.

/*
 * -------------------------------------------------------
 * THIS FILE WAS AUTOMATICALLY GENERATED (DO NOT MODIFY)
 * -------------------------------------------------------
 */

/* tslint:disable */
/* eslint-disable */

export interface IQuery {
}

export interface IQuery {
    bar(): Nullable<boolean> | Promise<Nullable<boolean>>;
    foo(): Nullable<boolean> | Promise<Nullable<boolean>>;
}

type Nullable<T> = T | null;

Minimum reproduction code

https://github.com/drieshooghe/nestjs-graphql-bug

Steps to reproduce

1. yarn
2. yarn generate-typings

Expected behavior

The output doesn't contain the duplicate interface/class.

Package version

10.0.21

Graphql version

@apollo/subgraph: "2.0.5"

NestJS version

9.0.11

Node.js version

16.13.1

In which operating systems have you tested?

• macOS
• Windows
• Linux

Other

I think this PR is causing the issue: #2081

[kamilmysliwiec]

Would you like to create a PR for this issue?

[drieshooghe]

I can't really wrap my head around the changes made in packages/graphql/lib/graphql-ast.explorer.ts.
So I'm afraid I won't be able to be of much assistance, perhaps @katainaka0503 can pinpoint the location of the issue?

[katainaka0503]

@drieshooghe In this PR (#2081) , I made the change to write schema all at once. Before the change, contents which are added by extend were automatically merged on TS file. But after the change, each definition is written with out merging. I think we should have added some merging process.

[kamilmysliwiec]

@katainaka0503 would you like to create a PR to fix this issue? Otherwise we might need to revert that PR

[katainaka0503]

@kamilmysliwiec Yes. I think I can work on this issue by the end of September.

[katainaka0503]

I've quickly investigated the issue.

When extend is used without original type definition in the graphql schema definition, graphql-tag will emit
an ast which will be converted to invalid typescript.

For example, schema in the repository is ( https://github.com/drieshooghe/nestjs-graphql-bug ) interpreted by graphql-tag as below.

{
    "kind": "Document",
    "definitions": [
        {
            "kind": "ObjectTypeDefinition",
            "name": {
                "kind": "Name",
                "value": "Query"
            },
            "interfaces": [],
            "directives": [],
            "fields": []
        },
        {
            "kind": "ObjectTypeExtension",
            "name": {
                "kind": "Name",
                "value": "Query"
            },
            "interfaces": [],
            "directives": [],
            "fields": [
                {
                    "kind": "FieldDefinition",
                    "name": {
                        "kind": "Name",
                        "value": "bar"
                    },
                    "arguments": [],
                    "type": {
                        "kind": "NamedType",
                        "name": {
                            "kind": "Name",
                            "value": "Boolean"
                        }
                    },
                    "directives": []
                },
                {
                    "kind": "FieldDefinition",
                    "name": {
                        "kind": "Name",
                        "value": "foo"
                    },
                    "arguments": [],
                    "type": {
                        "kind": "NamedType",
                        "name": {
                            "kind": "Name",
                            "value": "Boolean"
                        }
                    },
                    "directives": []
                }
            ]
        }
    ],
    "loc": {
        "start": 0,
        "end": 79
    }
}

Simply adding a sqlfile like below fixes the problem

type Query

, which is interpreted by graqhql-tag as below. Definitions are merged into one ObjectTypeDefinition.

{
    "kind": "Document",
    "definitions": [
        {
            "kind": "ObjectTypeDefinition",
            "name": {
                "kind": "Name",
                "value": "Query"
            },
            "interfaces": [],
            "directives": [],
            "fields": [
                {
                    "kind": "FieldDefinition",
                    "name": {
                        "kind": "Name",
                        "value": "bar"
                    },
                    "arguments": [],
                    "type": {
                        "kind": "NamedType",
                        "name": {
                            "kind": "Name",
                            "value": "Boolean"
                        }
                    },
                    "directives": []
                },
                {
                    "kind": "FieldDefinition",
                    "name": {
                        "kind": "Name",
                        "value": "foo"
                    },
                    "arguments": [],
                    "type": {
                        "kind": "NamedType",
                        "name": {
                            "kind": "Name",
                            "value": "Boolean"
                        }
                    },
                    "directives": []
                }
            ]
        }
    ],
    "loc": {
        "start": 0,
        "end": 60
    }
}

@drieshooghe I think this can be a quickfix.

@kamilmysliwiec
Actually, this is a regression and this kind of change should not be introduced with a patch version update. So I think it is good to fix this issue by merging ObjectTypeDefinition and ObjectTypeExtension with the same name.
Would it be alright to continue with this plan?

[katainaka0503]

After further investigation, I found that this is the problem of GraphQLFederationDefinitionsFactory as GraphQLDefinitionsFactory doesn't allow extend type Query without original type Query.

Actually this is caused by buildSubgraphSchema, which fills empty Query definition when there is no original type Query definition and there is extend type Query definition.

I made a PR to merge duplicate definition using graphql-tools.

[kamilmysliwiec]

Let's track this here #2353