[Draft] Transition to graphql-modules v1

[darkbasic]

This is just a draft and the API is subject to changes.
It's already working, but there are many changes I want to do.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 44ae64a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[darkbasic]

@pradel unfortunately graphql-modules v1 doesn't play well with apollo-server: Urigo/graphql-modules#1725
You're basically forced to use graphql-modules in your project and even then your latencies will double (Urigo/graphql-modules#1724).

The fix would be easy but it's passed more than 1 year since the PR has been created and still no signs of merging: apollographql/apollo-server#4167

[darkbasic]

For the moment I simply updated the graphql-server-typescript example to simply use graphql-modules itself.

[pradel]

The latency issue is really bad... It's pretty hard to justify the upgrade for the users if the execution is 2X slower

[pradel]

So there is no way to not use graphql-modules by just using the schema it provides like it was possible with v0?

[darkbasic]

Not with Apollo server, but it's possible with other GraphQL servers.

[pradel]

@darkbasic any update here? I saw that the guild released a new package https://the-guild.dev/blog/introducing-envelop could it be something that we could use here?

[darkbasic]

AFAIK envelop is not fully supported on Apollo as well. I will have to think more about this, the problem is Apollo not supporting custom execute.

[pradel]

Ok so no good solutions for now.. Wondering if it makes sense to finish/release the other graphql-modules v1 migration to unblock some users.

[darkbasic]

You mean the other PR? It faces the same Apollo limitations.

[pradel]

Yeah it does face the same apollo limitations but at the same time it's working, just performance is really bad (This should be in the accounts-js docs to warn users)... I know some people are blocked by graphql-modules v0 and really need the migration to v1. You can also use the v1 without apollo by using other graphql servers.

[darkbasic]

Yeah it does face the same apollo limitations but at the same time it's working

Can you please tell me which PR do you refer to? As far as I remember they both weren't working as well.

[pradel]

Ah okay then maybe I misunderstood something, I am talking about this one #1141

[pradel]

I also just saw that apollo released apollo server v3, don't know if it's fixed in this version.

[darkbasic]

Nope, they just ignored my request: apollographql/apollo-server#5262 (comment)

Feel free to ping them about it.

[darkbasic]

A workaround would be using something like this: Urigo/graphql-modules#1724 (comment)

[pradel]

Yeah definitely not perfect but at least it would work..

[darkbasic]

I will try to give it another shot next week to refresh my mind about the possible alternatives.

[darkbasic]

@pradel it took more than I anticipated to find some spare time to give this another try, but today I've managed to find some and this is how it's going to look like with apollo-server and the new API:

import { createApplication } from 'graphql-modules';

const server = new ApolloServer({
  schema: createApplication({
    modules: [
      createAccountsCoreModule({ accountsServer }),
      createAccountsPasswordModule({ accountsPassword }),
    ],
    schemaBuilder: ({ typeDefs: accountsTypeDefs, resolvers: accountsResolvers }) =>
      makeExecutableSchema({
        typeDefs: mergeTypeDefs([typeDefs, ...accountsTypeDefs]),
        resolvers: mergeResolvers([...accountsResolvers, resolvers]),
      }),
  }).createSchemaForApollo(),
  context: ({ req, connection }) => {
    return context({ req, connection }, { accountsServer });
  },
});

server.listen(4000).then(({ url }) => {
  console.log(`🚀  Server ready at ${url}`);
});

and this is with boosts:

const accounts = await accountsBoost({
  services: {
    password: {
        [...]
      },
    },
  },
  tokenSecret: 'secret',
  schemaBuilder: ({ typeDefs: accountsTypeDefs, resolvers: accountsResolvers }) =>
    makeExecutableSchema({
      typeDefs: mergeTypeDefs([typeDefs, ...accountsTypeDefs]),
      resolvers: mergeResolvers([...accountsResolvers, resolvers]),
    }),
} as any);

accounts.listen({
  port: 4000,
});

If this is ok I'll rebase my branch (or rewrite it from scratch since I also need to port the codebase to the latest graphql-tools).

[pradel]

@darkbasic Looks great!

[darkbasic]

I've finished re-porting from scratch latest master to GraphQL Modules V1 and I've also updated every single dep to latest because I was annoyed by the tons of warnings.
Now it starts the long, tedious process of re-porting all of the examples (and testing that everything still works).
It probably makes sense to open the new PR only once I've finished with the examples, then you can start testing while I will port my WIP MikroORM adapter to GraphQL Modules V1 and mikro-orm v5.
At that point I will be able to know if the new API fits my needs (and thus I can port my application to accounts-js) or if I will have to make further improvements.

[pradel]

Amazing work! Can't wait to merge the pr :D

[darkbasic]

Sorry for taking that long, but I've rewrote the schema stitching example from scratch and I've found a bug in GraphQL Modules which I took quite some time to figure out. I've managed to fix it, but it will require a new GraphQL Modules release. I'll keep you posted.

[darkbasic]

There are still a few things I need to test before I will open the new PR, but in the meantime if you want you can have a look at https://github.com/darkbasic/accounts/tree/v1

[darkbasic]

PR is ready (#1189), I'm closing this.