Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Types excluded from query/mutation generation should still be included in the resulting schema #221

Closed
lirbank opened this issue May 19, 2021 · 7 comments
Labels
bug report Something isn't working confirmed Confirmed bug
Projects

Comments

@lirbank
Copy link

lirbank commented May 19, 2021

This may be related to #146

The following fails with Error: "Car" defined in resolvers, but not in schema:

import { Neo4jGraphQL } from '@neo4j/graphql';
import neo4j from 'neo4j-driver';

const neo4jGraphQL = new Neo4jGraphQL({
  driver: neo4j.driver(
    'bolt://localhost:7687',
    neo4j.auth.basic('neo4j', 'neo4j'),
  ),
  typeDefs: /* GraphQL */ `
    interface Vehicle {
      id: ID!
    }

    type Car implements Vehicle @exclude {
      id: ID!
    }

    type Boat implements Vehicle @exclude {
      id: ID!
    }

    type Query {
      vehicles: [Vehicle!]!
    }

    # NOTE: There must be at least one mutation or this code errors with `Error: "Mutation" defined in resolvers, but not in schema`
    type Mutation {
      noop: Boolean
    }
  `,

  resolvers: {
    Vehicle: {
      __resolveType(vehicle: { readonly __typename: string }) {
        return Math.random() > 0.5 ? 'Car' : 'Boat';
      },
    },

    Car: {
      id(car: { readonly id: string }) {
        return car.id;
      },
    },

    Boat: {
      id(boat: { readonly id: string }) {
        return boat.id;
      },
    },

    Query: {
      vehicles() {
        return [{ id: Math.random().toString() }];
      },
    },
  },
});

export const schema = neo4jGraphQL.schema;

A workaround to fix this is to not exclude all CRUD operations, so the Car and Boat types are included in the schema. This works but the schema will include unwanted read queries.

import { Neo4jGraphQL } from '@neo4j/graphql';
import neo4j from 'neo4j-driver';

const neo4jGraphQL = new Neo4jGraphQL({
  driver: neo4j.driver(
    'bolt://localhost:7687',
    neo4j.auth.basic('neo4j', 'neo4j'),
  ),
  typeDefs: /* GraphQL */ `
    interface Vehicle {
      id: ID!
    }

    # Keeping READ
    type Car implements Vehicle @exclude(operations: [CREATE, UPDATE, DELETE]) {
      id: ID!
    }

    # Keeping READ
    type Boat implements Vehicle
      @exclude(operations: [CREATE, UPDATE, DELETE]) {
      id: ID!
    }

    type Query {
      vehicles: [Vehicle!]!
    }

    # NOTE: There must be at least one mutation or this code errors with `Error: "Mutation" defined in resolvers, but not in schema`
    type Mutation {
      noop: Boolean
    }
  `,

  resolvers: {
    Vehicle: {
      __resolveType(vehicle: { readonly __typename: string }) {
        return Math.random() > 0.5 ? 'Car' : 'Boat';
      },
    },

    Car: {
      id(car: { readonly id: string }) {
        return car.id;
      },
    },

    Boat: {
      id(boat: { readonly id: string }) {
        return boat.id;
      },
    },

    Query: {
      vehicles() {
        return [{ id: Math.random().toString() }];
      },
    },
  },
});

export const schema = neo4jGraphQL.schema;

System

"@neo4j/graphql": "1.0.1",

@lirbank lirbank added bug report Something isn't working inbox labels May 19, 2021
@darrellwarde
Copy link
Contributor

darrellwarde commented May 19, 2021

Hey @lirbank, thanks for raising. 🙂 This happens because graphql-compose removes unused types upon schema generation. Can I ask, what is your use case for the type if it is not used anywhere at all in the schema?

@lirbank
Copy link
Author

lirbank commented May 19, 2021

Hi @darrellwarde! The bigger use case is that we have an existing, somewhat big, GraphQL API based on Apollo Server and MongoDB. We want to start using Neo4j for some parts of our API and wanted to utilize Neo4jGraphQL for this. Instead of having two schemas and merge them, which doesn't work currently, the plan is to have a single schema and run everything through Neo4jGraphQL.

For this to work I've added @exclude to every existing type, since we don't want Neo4jGraphQL to touch them. And this mostly works, and all(most) tests are passing (hooray!). But just almost! In three places we have queries that return interfaces instead of types, and for this to work, the types must be available, but Neo4jGraphQL removes them when I add the @exclude directive.

Apollo Server complains coz there is a resolver for Car, but no schema. But even if it didn't, it would crash at runtime since it there is no type for the items being returned. Hope you see what I mean? The example above is contrived and the resolvers for Car and Boat does not really do anything, but imagine they do have separate logic and different additional fields beyond what the Vehicle interface dictates and I am sure you see what I mean. For a simple use case as above, where both Car and Boat are identical, one would obviously replace Vehicle (interface), Car (type), and Boat (type), with a single type Vehicle (type).

Back to use cases; In essence we're trying to get an existing schema (type defs and resolvers) to play with Neo4jGraphQL, so we can gradually adopt Neo4jGraphQL and Neo4j. This is also the reason I have lately been posting a series of issues related to, what may seem like corner cases, but that come out of an actual attempt to start adopting Neo4jGraphQL for an existing, and somewhat large API. Hope that makes sense?

@darrellwarde
Copy link
Contributor

It makes an awful lot of sense, and it's really great that you're raising issues that you're encountering using an existing dataset and API! 🙂

We may need to address this in graphql-compose itself if we want to fix this "nicely", rather than putting workarounds in our library. I want to have a bit of a play around with graphql-compose to see if there's any alternative way to perform schema generation without cleaning up, but failing that will move onto raising an issue there.

@process0
Copy link

process0 commented Sep 1, 2021

I'm also interested in this. It seems graphql-compose has support for preserving unused types. graphql-compose/graphql-compose#294

@process0
Copy link

process0 commented Sep 1, 2021

I can confirm that the keepUnusedTypes works here.

Replace

const generatedTypeDefs = composer.toSDL();

with:

const generatedTypeDefs = printSchema(composer.buildSchema({ 
        keepUnusedTypes: true
}))

@darrellwarde darrellwarde added this to Needs triage in Bug Triage Dec 21, 2021
@darrellwarde darrellwarde moved this from Needs triage to Confirmed in Bug Triage Dec 21, 2021
@darrellwarde darrellwarde added confirmed Confirmed bug and removed bug report Something isn't working labels Dec 21, 2021
@darrellwarde
Copy link
Contributor

Just going through some older issues and seen your reply here @process0.

Sadly not quite as simple as adding this in for us, as the schema will be then be littered with many unused types which we generate but don't use.

I think a better approach here will be to generate the schema without these types, and then manually add them back in to the generated schema.

@darrellwarde darrellwarde added bug report Something isn't working and removed inbox labels Dec 22, 2021
@darrellwarde
Copy link
Contributor

Closing this issue, citing that the workaround is to add these types/resolvers in to the schema following generation by the library. Do feel free to reopen if this is not a suitable workaround.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug report Something isn't working confirmed Confirmed bug
Projects
Bug Triage
Confirmed
Development

No branches or pull requests

3 participants