Neo4jGraphQL contaminates the global scope/and or mergeSchemas

[lirbank]

Probably related to #167

Describe the bug
A standalone Apollo Server, stop working if I instantiate Neo4jGraphQL (eg new Neo4jGraphQL(...)) even if the resulting schema is never used. This seems to happen in conjunction with mergeSchemas() - even if no Neo4jGraphQL schema is ever used.

To Reproduce
Consider this example:

import {
  ApolloServer,
  makeExecutableSchema,
  mergeSchemas,
} from 'apollo-server-express';
import express from 'express';
import { Neo4jGraphQL } from '@neo4j/graphql';

const baseSchema = makeExecutableSchema({
  typeDefs: /* GraphQL */ `
    type Query {
      stuff: String
    }

    type Mutation {
      mutateSomething(input: Int!): String!
    }
  `,
  resolvers: {
    Mutation: {
      mutateSomething: () => 'response',
    },
  },
});


// START FAULTY CODE
// NOTE: The Neo4jGraphQL instance is never even used.
// NOTE: If you remove the instantiation of Neo4jGraphQL here `new
// Neo4jGraphQL(...)` the issue disappears
new Neo4jGraphQL({
  typeDefs: /* GraphQL */ `
    interface Thing {
      id: ID!
    }

    type SomeType {
      someField: String!
    }
  `,
});
// END FAULTY CODE

const schema = mergeSchemas({
  // schemas: [baseSchema, neo4jGraphQL.schema],
  schemas: [baseSchema],
});

const port = 4000;
const apolloServer = new ApolloServer({ schema });
const app = express();
apolloServer.applyMiddleware({ app });

app.listen({ port }, () =>
  console.log(
    `🚀 Server ready at http://localhost:${port}${apolloServer.graphqlPath}`,
  ),
);

Now run this query:

mutation {
  mutateSomething(input: 1)
}

Which will fail with:

"Variable \"$_v0_input\" got invalid value { low: 1, high: 0 }; Expected type \"Int\". Cannot represent non number as Int"

As you can see, the result of new Neo4jGraphQL() is not even collected in a variable. Still the Apollo server breaks. If you comment out the lines between START FAULTY CODE and END FAULTY CODE, the Apollo server works like a charm and the example query will return:

{
  "data": {
    "mutateSomething": "response"
  }
}

Alternatively you can also just comment out the type definition, to make the vanilla Apollo server happy:

type SomeType {
  someField: String!
}

System (please complete the following information):

"@neo4j/graphql": "1.0.0",
"apollo-server-express": "2.24.0",
"express": "4.17.1",

[lirbank]

@darrellwarde The Trello card is on a private board so I can't see it. Can you provide some context here?

[darrellwarde]

Sorry @lirbank, it is a private board and my comment was posted automatically when I referenced this issue from our Trello instance.

Essentially, the plan is to remove our custom scalars which override the default Int and Float scalars. They're not working as expected in a number of places, so we will replace the (de)serialization of these values in other ways.

[lirbank]

Thanks @darrellwarde! I know this is hard to answer but this is a complete blocker for us so if there is any chance you can provide an indication of when this may be fixed that would be a great help, so we can plan accordingly. Do you think it is days, weeks or months we're talking about? Big thanks in advance!

[colinskow]

@lirbank meanwhile have you tried routing your external resolvers through Neo4jGraphQL instead of merging them through Apollo Server? Documentation here.

[darrellwarde]

Thanks @darrellwarde! I know this is hard to answer but this is a complete blocker for us so if there is any chance you can provide an indication of when this may be fixed that would be a great help, so we can plan accordingly. Do you think it is days, weeks or months we're talking about? Big thanks in advance!

I completely understand your frustration, and thank you for your patience.

I really don't like committing to timelines, so please take this with a pinch of salt and not as a commitment. But we work in development phases, the next one being 5 weeks long and starting next week. We'll be agreeing on its content today, but we're hoping it will include addressing a couple of key bugs, including this one. All going well, it will be released either during the phase, or at the end of the phase, depending on our order of attack.

We really don't want to leave users hanging for months on this one.

[lirbank]

@colinskow That's a good idea! I tried to do this with the, now deprecated, neo4j-graphql-js package and we couldn't get it to play (was some issue with GraphQL interfaces that we have in our base schema) but if this works with Neo4jGraphQL that'd be great! Totally forgot about this approach since we shelved it once before, thanks for bringing it up!

@darrellwarde Thanks a bunch, that's very helpful!

[tlester]

@darrellwarde - Just curious if this got a commitment for this sprint/dev cycle? Like @lirbank, this is a show stopper for us. I can let it go for a couple more weeks, but we go live on June 1st with our "alpha". Right now, I've created a workaround by writing a custom cypher to handle pagination, but this isn't a solution that can be long-term.

[darrellwarde]

A fix (hopefully!) for this has now been merged, and will be released shortly (aiming for tomorrow). Please do test and feel free to reopen this ticket if it hasn't addressed your problems! 🙂