feat: transition to graphql-modules v2

[darkbasic]

This PR doesn't simply port accounts-js to graphql-modules v1 but also updates every dep, including latest graphql-tools. That means I've basically rewritten some examples from scratch (like schema stitching). I tested it and it seems to work fine, also tests are passing. Unfortunately there is a bug in graphql-modules which breaks schema stitching (https://github.com/Urigo/graphql-modules/issues/1914) and at the moment there isn't any working version on the horizon. I circumnvented the issue by overriding the bundled graphql-tools version (https://github.com/darkbasic/accounts/blob/f078633c03e9a432db66fc8a507db7d207ef1e97/package.json#L75) and it works fine now, but I'm unsure if there might be any undefined behaviour because of that (couldn't find any). Should hopefully be fixed in a future graphql-modules release.

P.S. I didn't test the docusaurus because I've seen there was already a pending PR for that which updated all of its dependencies anyway.

[changeset-bot[bot]]

⚠️ No Changeset found

Latest commit: a3dadb797181b65017d736b0554fbfc28944487d

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]

Not sure why tests are failing on CI, they are working on my machine:

lerna success run Ran npm script 'coverage' in 24 packages in 82.8s:
lerna success - @accounts/client-magic-link
lerna success - @accounts/client-password
lerna success - @accounts/client
lerna success - @accounts/database-manager
lerna success - @accounts/mongo-magic-link
lerna success - @accounts/mongo-password
lerna success - @accounts/mongo-sessions
lerna success - @accounts/mongo
lerna success - @accounts/redis
lerna success - @accounts/typeorm
lerna success - @accounts/e2e
lerna success - @accounts/error
lerna success - @accounts/express-session
lerna success - @accounts/graphql-api
lerna success - @accounts/magic-link
lerna success - @accounts/oauth-instagram
lerna success - @accounts/oauth-twitter
lerna success - @accounts/oauth
lerna success - @accounts/password
lerna success - @accounts/rest-client
lerna success - @accounts/rest-express
lerna success - @accounts/server
lerna success - @accounts/two-factor
lerna success - @accounts/types

Might be worth trying to re-run the CI.

[codecov[bot]]

Codecov Report

Merging #1189 (a3dadb7) into master (79575f8) will increase coverage by 0.75%. The diff coverage is 92.10%.

@@            Coverage Diff             @@
##           master    #1189      +/-   ##
==========================================
+ Coverage   95.29%   96.05%   +0.75%     
==========================================
  Files         106       96      -10     
  Lines        2381     2305      -76     
  Branches      492      480      -12     
==========================================
- Hits         2269     2214      -55     
+ Misses        103       85      -18     
+ Partials        9        6       -3     

Impacted Files	Coverage Δ
...s/graphql-api/src/modules/accounts/schema/types.ts	100.00% <ø> (ø)
...phql-api/src/modules/accounts/schema/schema-def.ts	57.14% <50.00%> (-42.86%)	:arrow_down:
packages/graphql-api/src/utils/context-builder.ts	94.11% <93.75%> (+3.20%)	:arrow_up:
.../modules/accounts-magic-link/resolvers/mutation.ts	100.00% <100.00%> (ø)
...rc/modules/accounts-password/resolvers/mutation.ts	100.00% <100.00%> (ø)
...i/src/modules/accounts-password/resolvers/query.ts	100.00% <100.00%> (ø)
packages/graphql-api/src/modules/accounts/index.ts	100.00% <100.00%> (+28.57%)	:arrow_up:
...hql-api/src/modules/accounts/resolvers/mutation.ts	93.33% <100.00%> (ø)
...raphql-api/src/modules/accounts/resolvers/query.ts	100.00% <100.00%> (ø)
...raphql-api/src/modules/accounts/schema/mutation.ts	100.00% <100.00%> (ø)
... and 12 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 79575f8...a3dadb7. Read the comment docs.

[pradel]

Thank you so much for the pr @darkbasic, I will be able to have a look only next week 🙏

[pradel]

@darkbasic I started to review the pr, but the amount of unrelated changes makes it really hard :( Could it possible to split it in multiple prs? eg:

1. upgrade deps
2. this one

[pradel]

@darkbasic I upgraded the deps to limit the number of changes in this pr.

I circumnvented the issue by overriding the bundled graphql-tools version (darkbasic/accounts@f078633/package.json#L75) and it works fine now, but I'm unsure if there might be any undefined behaviour because of that (couldn't find any).

This should be solved by upgrading the deps right?

In general, pr looks good, I am just wondering if we could somehow find ways to reduce the boilerplate to create a new Graphql server.

Other than this, updating the docs and creating a migration guide will be needed with the new release, amazing job 🚀

[darkbasic]

I see that you've backported most of the fixes/updates into the main branch, thus I've rebased this PR against it.

[darkbasic]

This should be solved by upgrading the deps right?

AFAIK this issue should have been solved in graphql-modules 2.0.0, but I still didn't have the time to test it and thus I didn't bump the graphql-modules version. Hopefully I should be able to do so in the next days.

[darkbasic]

Ok I've rebased this PR once again and bumped graphql-modules to 2.0.0, which fixes the issue and doesn't require to override its @graphql-tools/wrap version.

[CaptainN]

Hi, thanks for a great project!

I'm running in to issues trying to set up a new project from scratch by following the guides. Issues like being unable to use the old declarations by default, since they've been removed from graphql tools, and Apollo 3 uses a newer version.

It's possible to work around the issues that I've found by using older versions of graphql tools, but it wasn't especially easy to figure that out. It might be nice to get those notes in to the older docs, if they are going to stick around after this releases.

I'll be happy to test this PR out in my new project, since I don't have any interest in using older modules. I'll give it a try this weekend. Seems like I'll have plenty of time with the coming blizzard!

[CaptainN]

@darkbasic I remember an old game engine called "Dark Basic". That brings me back!

[darkbasic]

@CaptainN that's exactly where my nickname comes from: I used it to develop some games with it when I was a kid :) It wasn't just a game engine, it was more of a programming language of its own.

By the way your timing is very precise: I'm planning to do further breaking changes here and I've just discussed them in my other MikroORM v5 PR. You're welcome to join the discussion.

[CaptainN]

Awesome! Maybe I'll make a PR for Prisma (based on this adapter), which is what I'm using. Would there be interest in having that adapter in the repo?

[darkbasic]

Sure if you're willing to maintain it, because I personally don't use Prisma. But I suggest you to wait a little bit until the waters settle down before PRing: there are a lot of breaking changes down the pipe right now.

[flashtheman]

Is it known when a version including this update is going to be released?

[pradel]

@darkbasic I am still willing to merge this one, happy to chat on Twitter or Discord!

[andredantasrocha]

Hi guys, I am pretty excited about this change. Any updates on that? it would be amazing to have it running in Apollo V3

Thanks!

[mrcleanandfresh]

Happy to help get this merged, if needed. I'm thinking it might solve this issue https://github.com/accounts-js/accounts/discussions/1238 for me.

[mrcleanandfresh]

@darkbasic since you originally did this work almost a year ago (in October 2021) does it make sense to version bump again, or are we past that point?

[darkbasic]

Hi, This PR is basically done, but it has been superseded by a larger one which basically re-architectures a big chunk of accounts-js in order to expose a better API and make new things possible. The re-architecture was mostly done as well and included graphql-modules v2 as well as a MikroORM v5 adapter, but I wanted to discuss it with @pradel before pouring further work into it. For many reasons months had passed and I didn't manage to do so, but I hope to do so in the following weeks. I will need to re-review my own code and find my notes before doing so, because unfortunately half an year is passed and that turns a 90% done work into a 60% done one. But we will get through this :)

[mrcleanandfresh]

I will need to re-review my own code and find my notes before doing so, because unfortunately half an year is passed and that turns a 90% done work into a 60% done one.

I hear that!

superseded by a larger one which basically re-architectures a big chunk of accounts-js in order to expose a better API and make new things possible. The re-architecture was mostly done as well and included graphql-modules v2 as well as a MikroORM v5 adapter, but I wanted to discuss it with @pradel before pouring further work into it. […] But we will get through this :)

Let me know if there’s anything I can do! I’d happily roll your changes into my project to help test too, like I said, anything I can do to help roll this out.

All of what you said sounds really good. I was actually a week or two away from either writing my own GraphQL API—because of out-of-date dependencies, the @auth directive issues I linked and security audit issues—or forking it and doing my own updates.

[darkbasic]

Let me know if there’s anything I can do! I’d happily roll your changes into my project to help test too, like I said, anything I can do to help roll this out.

Testing would be appreciated of course, once @pradel gets a chance to review the changes I would like to release an alpha and testing would be needed to ensure that there are no regressions.

and security audit issues

Do you have a link to your audit?

[mrcleanandfresh]

Do you have a link to your audit?

Here's the parseable version, and here's the JSON version.

@accounts/graphql-api is responsible for one of the high severity ones through @graphql-modules/core.

Then a critical and moderate vulnerability comes from mongoose, which I had to pin to "mongoose": "5.9.25" to get it to work with database-mongo-password or database-mongo packages. It's possible it's fixed now. It was a long time ago, when I first started using AccountsJS, but I was forced to use the same Mongoose as the example (the graphql-api demo), or I’d get a weird Mongo error when trying to hit certain resolvers. I think this was due to having a particular version of Mongo in accounts, and the much more up-to-date version of mongoose in my project.

The last one is Mercurius which I can't update that package yet because it causes the following chain reaction that ends with graphql-api needing to update some dependencies:

1. Upgrade fastify to version 4.x
2. Uninstall fastify-cors, fastify-formbody and fastify-static
3. Install @fastify/cors, @fastify/formbody and @fastify/static
4. Upgrade @graphql-tools/modules and @graphql-tools/schema - This will break buildExecutableSchema and Mecurius. It will complain about schemaDirectives no longer existing. Need to re-implement the AccountsJS @auth directive in this way.
5. Upgrade mecurius and mercurius-auth to latest which will break the installed version of graphql.

But in order to do 5, I have to upgrade graphql, which I can't do because @accounts/graphql-api itself is using an older version and the typings changed enough to where it's throwing compilation errors. It's a pretty big jump, something like version 13 to 17 or something.

Hope that helps. I didn't get a chance to proof read because I'm in a hurry, so hopefully that all makes sense. Let me know if there are any questions.

[darkbasic]

Ah you meant the npm thing, I thought someone did a real security audit on accounts-js. I already took care of this in my PR because I've upgraded every dependency to latest, but I'm pretty sure I will have to do so again because of how much time has passed.

[pradel]

Hey @darkbasic, it's been a long time I didn't check the changes, I don't really have time now but I can release the change made on this pr as an alpha tag, would that work for you in order to test? Btw if you want to chat I am on discord leopradel#0606 or Twitter @leopradel

[darkbasic]

Hi @pradel, there is no reason to release an alpha because the code I'd like to get merged is on a completely different branch and I didn't open a PR yet. I'd like to go through the code in a video call to show you the new architecture. I'll ping you on discord once I'm ready.

[mrcleanandfresh]

@darkbasic it would be beneficial to the community to have a somewhat similar tour around the architecture, in the form of additional documentation, as part of this release. Perhaps as a “what is accountsJS” or a Getting started section about which pieces to click together? Have you already put together something like that? I could contribute in that effort, perhaps a similar tour after @pradel approved things or they get more finalized, or you could record the tour on video and we can use it in the docs.

I think all the changes sound really good. I was just thinking about how the docs are pretty terse, and could use some additional details. Seems like a good opportunity.

[darkbasic]

I have rewritten all the examples and I plan to make a blog post when it will be time for a release, but the documentation still needs to be updated. If you want to do so you are more than welcome!

[darkbasic]

Hi, I've overhauled the whole accounts.js to a brand new architecture which better suits graphql-modules while making it simpler and more straightforward. I've also created a brand new MikroORM adapter and updated all the dependencies including @graphql-tools 10, graphql 16, graphql-modules 2, @apollo/server 4, typescript 5, mongo 5 etc. In accordance with @leopradel I've opened a new Discord server because he lost access to his Slack account, here is the invite link: https://discord.gg/nYSyrWPPdu Since the changes are many we're going through them in a video conference, so if someone wants to join us reviewing the code and/or helping ironing out the remaining issues please join the discord server and let us know.

[darkbasic]

It's on master.