Collaborate with apollo-accounts

[TimMikeladze]

Hi @gadicc, I've been working on a similar set of packages https://github.com/apollo-accounts/. After seeing your packages it got me thinking that maybe we should combine our efforts rather than having two different implementations of accounts systems for apollo being published and maintained.

Let's go over each other's work so far and see what ideas comes up? I'm also willing to setup a video call or slack (I'm in the apollostack slack) if you want to discuss in real time.

[gadicc]

Hey! Wow, wish we came across each others work sooner. Definitely makes sense to work together.

How far along are you? Is the apollo-accounts-example working and up to date? From my side, I was actually just about to announce this. There is 100% test coverage on all core packages (apollo-passport, local, rethinkdb, mongodb, but not for react yet) and starter kits for saturn and Meteor. Notably missing currently is the Meteor style accounts-ui config, and email support (for verification and email recovery).

I'm actually quite over time budget with this, and the earliest I'll be able to take a proper look at things again is only next week. I did have a quick look over the apollo-account repos though. Great work so far! (and a lot of it). Between the two, there is both overlap but also fairly extensive work in different directions. I'm not quite sure the best way yet on how to combine efforts. But yes, would probably be a good idea to go over all the different aspects comparing the different approaches we've both taken, to discuss. Will get to this as soon as I can.

[TimMikeladze]

To answer your first question, apollo-accounts-example is not up to date.

I think the largest distinction between our projects is that you're using Passport whereas I'm using Grant. I tried a Passport implementation originally and found myself wrestling with finding integration points into Passport. Plus In my opinion I found Grant's codebase to smaller and simpler to understand than Passport's.

It looks like we had similar ideas creating database specific packages which must implement the api.

You've done significantly more work on the GraphQL portion that I have (I especially like the discover functionality). It looks like you also have integration with Redux.

On my end I've still got to finish the middleware for verifying access and refresh tokens and setting the logged in user's id. Then implement the GraphQL for local login, signup and password recovery. However the core functionality of logging in with an outh service, having user records created, and tokens issued is functioning.

I think between our projects you've done more in terms of functionality but I'm finding mine to have a simpler design (granted I'm operating under a bias having written that code).

In case it helps, this is what the initialization code looks like on the server. It's very similar to yours.

That's all I've got so far, need to dig into your codebase a bit more.

import Accounts from 'apollo-accounts-knexjs'

const config = {
   server : {
      secret,
   }
   github: {
      key,
      secret
  }
}

const accounts = new Accounts(config, connector);

// Middleware to setup callback routes
// Verify the tokens sent in the header
// Decode token and set user id
app.use(accountsExpress(accounts));

app.use('/graphql', apolloExpress((req) => {
  const userId = req.userId;

  return {
    schema: [executableRootSchema],
    context: {
      accounts,
      userId,
    },
  };
}));

[TimMikeladze]

@gadicc Gentle reminder about this issue 😄

[gadicc]

Hey @TimMikeladze, thanks for your patience! I promise I didn't forget about this, was just quite a crazy week. (Bold / italic use below is more to make this long reply easier to scan through, rather than overly emphasize the actual statements)

On that note, I do want to be up front about my time. As I mentioned before, I was about to stop working on apollo-passport for a while, now that it's usable - aside from smaller, more incremental improvements as needed (I wanted mainly to invite others to get involved). Merging projects (this is the goal here, right?) is, I agree, the right thing to do, but obviously will involve a fairly big time investment from both of us, and I just want to be honest that my time is fairly limited atm.

Did you have any ideas of how to best integrate the two projects? From my side, it's preferable to use apollo-passport as the base, simply because there's already 100% coverage on all core packages, automated testing is all in place, PRs will report coverage changes, contributing guidelines are all in place, basic functionality is all working and there are starter kits for meteor & saturn. It would be great to never move backwards in that we know that every merge keeps everything working, although this does give apollo-passport a big initial advantage (discussion on specific parts follows below).

Regarding Grant vs Passport, I definitely agree with you. Passport is not ideal but I decided to go with it because it's so well known and so many people are already using it. Notably:

	Passport	Grant
Strategies	300+	150+
Github Stars	8k	1k

This was my motivation for the extra pain in dealing with it. I believe I've solved everything except getting detailed error text for some cases, I need to look at it again. But if I have a working Passport implementation, and you have one for Grant, it makes me wonder if we can (or should?) support both.

The other thing I wanted to ask you about here was your use of extractors. Do we need to build one for every strategy? This is possibly another advantage of Passport; it makes these requests for you and also normalizes the output to common field names used by all the plugins (you still have access to the raw fields though). The only "extra" things we'd need to add are the instructions to register (a la accounts-ui) and service icon.

Name wise, apollo-accounts, of course, is a lot more descriptive. The only other thought I had here was that there isn't too much about the project that is apollo specific, and I had wondered about having apollo/graphql as just one type of transport, essentially making this is an authentication microframework that could also impact adoption. Of course that's a more long term thought.

Database packages, yes, I don't think there's any other way to do it :> It's a nice model that's easy for others to emulate, e.g. @tomitrescak made a MongoDB driver before I even had a working release ready :> Perhaps agreeing on the API would be a great first step moving forward. On that note, however:

Meteor compatibility was important for me, I really wanted to be able to repeatedly swap between Meteor accounts and apollo-passport, to ease developer pain and allow for incremental adoption. This is the one of the reasons I used the same schema (the other being that it's quite sensible), and also let's us use a single, familiar User object format everywhere (including in end-user code or other potential packages). To that end, it was my intention for any SQL database connectors to split up the data amongst their respective tables as needed.

Re GraphQL, this all works but is a bit of a pain mostly due to ardatan/graphql-tools#110. I'm currently merging the typeDefs with regexs as an interim workaround, but as soon as I have time I want to try @tomitrescak's apollo-modules.

Re Redux, it was a design goal to make this optional. It's very easy to integrate and super comfortable to use as expected with Redux, but the react package includes a Provider and connect-like function for those not using Redux. On that note, besides for that, this is my least favourite package (with no tests); it does all work though. It looks like your material design package is much better structured, but of course, has a requirement on material-ui, which may preclude it's use for a lot of devs. In general I'd like to aim for any react implementation to use react-storybook for design.

Re design, things did start off simpler but did get a lot more complicated as the project developed, especially working around Passport, as you noticed. I'll be happy to take suggestions to simplify the code. I care more though about how simple the end-user API is which must remain a priority.

I'm also behind on refreshing tokens but the local package is all done, except for emailing (verification / password reset; you can change passwords though if you know your old one). (And then as mentioned earlier, a setup wizard like accounts-ui isn't done yet)

I also wonder if we should invite others to get involved in this discussion to hear outside input on some of these issues, what do you think?

[TimMikeladze]

I hear you on the time. Mines limited but I can dedicate enough time to keep the project moving. The majority of my projects require an accounts system so I'd prefer to have a proper solution than hacking together something every time.

Passport vs Grant

My concern with Passport is that compared to Grant it appears to be larger and more complex then it needs to be.

The different strategies are split up across packages whereas in Grant they are all colocated into a single JSON config file. On the other hand Passport does standardize the user profile response. This is what my extractors are doing in order to obtain a unique identifier for the user from the service. You're right in saying that an individual extractor is need per service however I think there's a common pattern here which we can use. It's a matter of knowing which endpoint(s) supplies profile information. A possible implementation is specifying them in Grant's ouath.json file.

Passport is also limited to just Express whereas Grant supports Koa and Hapi as well, as does Apollo Server.

Passport is more popular than Grant but also consider the issue counts, 170 vs 0. Passport does have an advantage on number of strategies supported however I think it would be easier to add more strategies to Grant than it would to Passport (create separate strategy package vs adding to a single json config). Not to mention I think it's safe to assume that the majority of developers are using just a handful of popular oauth services out of the hundreds.

In short, I have more confidence in Grant, especially if it comes down to sending PRs if there are issues. Whether Passport or Grant, the package we choose provides a core functionality of this project which is why I think it's important to make this decision carefully before moving forward.

UI

I spent more time on the material design package because it was the fastest for me to build. I agree that material-ui cannot be a requirement. I have fallen in ❤️ with React Storybook in the past month and definitely want to use it for React components in this project.

I'm thinking there should be individual packages for unstyled, material, bootstrap, etc Account UIs. They could be strictly presentational and be consumed by higher order components which enhance them with additional functionality like GraphQL usingreact-apollo or connecting them to a Redux store.

Transport and Naming

Having GraphQL as just a type of transport rather than the only one is a great idea. We could certainly achieve this. In fact in my packages the only usages of Apollo/GraphQL are for local account mutations (login, signup, recover) and the client network interface middleware to send the tokens in the header.

You're right, Apollo Accounts is more descriptive but if Apollo/GraphQL are just a transport then perhaps a more generic name would be more suitable like JS Accounts or Node Accounts?

Meteor and API

Meteor's end user API for Accounts is great and I think trying to emulate it is a step in the right direction. Especially for compatibility purposes if you're swapping out Meteor Accounts with this set of packages.

Moving forward

We should definitely involve more people in the discussions, Apollo's Slack would be a good place to start. Perhaps @tomitrescak and @davidyaha have some ideas since they both have packages in similar veins. Maybe @stubailo has some insights into what the future is for Meteor accounts?

• Move all the repos under a single organization and get the documentation and issues ready for consumption by other contributors.
• Decide upon the end user API.
• Evaluate and improve the implementations of database specific packages.
• Passport vs Grant, choosing the latter will involve rewriting some code (on my part all the server side packages have tests).
• UI packages

[stubailo]

Especially for compatibility purposes if you're swapping out Meteor Accounts with this set of packages.

I think the big decision here is - how much compatibility do we want with Meteor accounts? I think it's not a huge deal personally as long as it's possible for people to write a shim that makes the two work together.

We've been thinking about possibly porting Meteor accounts to work with GraphQL, but I don't think there's anything in there that can't be replaced by the approach talked about here.

You're right, Apollo Accounts is more descriptive but if Apollo/GraphQL are just a transport then perhaps a more generic name would be more suitable like JS Accounts or Node Accounts?

I think the way to win here is to focus on a specific community at first, and the Node/JS community seems way too large as a starting point.

IMO branding it as "GraphQL accounts", and having Apollo tie in closely with it, could be a great idea. So you can rely on a specific GraphQL schema for the accounts data, but the client you use to access it is up to you.

[gadicc]

@TimMikeladze, I'm in the same boat. Accounts is critical for my current and most likely all future apps :)

@stubailo, great to see you here :>

• Passport vs Grant - I'm convinced :)
• UI - we're on the same page.
• Meteor compat - oh, I actually just meant the DB, not API compatibility. But yeah a shim for the API would probably be appreciated by a lot of people; might be a while before we can offer all the same features though. The main thing for me is that existing users can still log in after switching systems (in both directions).
• Name and transport - yes, I definitely want to focus exclusively on Apollo in the beginning. It's more about forward planning (structure, API, name). It's a bit petty but I'd love a shorter name to be able to prefix the package types, like blah-db-mongodb etc :> apollo-passport-db-mongodb seemed a pain, but I guess Babel does it for plugins and presets. Btw, looks like there is another node accounts WIP, we should give the author a heads up as things start to take shape here.

Moving Forward

Ok, I'll move everything over to apollo-accounts, please give me access. Would like to clarify a few more housekeeping matters first though.

• Can you take a look at my CONTRIBUTING.md and see if you agree with everything? The bit about PRs even with a commit bit is more long term; obviously I don't mean for the initial active development.
• Testing. I have a big preference for keeping tests in the same directory as the code - over many projects, I just find this pattern has better results. 100% coverage or near 100% coverage is important to me (and with coveralls we'll get a coverage diff in PRs). Happy you're also with mocha+chai. I prefer should() format, but if we don't allow both I'll switch to expect().
• Callbacks vs promises. Maybe premature to bring this up, but I prefer the latter (especially with await/async) wherever possible. Looks like you agree though :)
• Not a biggie, but, might be easier to keep client/server code in a single package, since they'll usually need to be kept in sync, and this would make it easier both on us for publishing and devs for updating.
• Redux. Did you agree on this being both supported but optional, with alternative Provider/connect functions for those who don't want it.

I'm still not 100% sure where to begin. The database packages are indeed a good start. But from there, to where? Are we going to try merge one project into the other? Do this package by package to decide which to be the base? Start fresh and merge in pieces feature by feature? What are your thoughts?

[TimMikeladze]

@gadicc you've been added to the organization. I left an issue for @mitica on that node accounts project.

• Contributing looks good.
• I've started keeping tests in the same directory lately and prefer it over having a dedicated test folder. I've only used expect but am cool with should.
• Client and server in the same directory sounds fine. I haven't had any problems keeping them separated though.
• Redux, supported but optional, so yes.

Where to begin? I think the cleanest approach would be to start fresh and pull in reusable code from our packages. Let's start at the database side. If we use my database code there needs to be some small tweaks made to align the database schema with Meteor's.

Have more to say, but short on time. more to come later.

[gadicc]

Thanks for squeezing in a quick reply! And for the org invite. I wonder though, if we're starting fresh, if it might be better to start a new org then, so it's clearer what's going on? Rather than potentially having 3 repos (your old, my old, our new) for each component? Or I could add you to and rename apollo-passport, and we could keep legacy code in there?

I'm happy to create a new repo template, with circle-ci and coveralls etc. Though the database connectors we could probably move over as is and adjust from there.

See what you think of the apollo-passport DB, particularly the API, since there's already full jsdocs with tests, my rethinkdb one and tomitrescak's mongodb one. I don't think you need to worry about Meteor compatibility, since there's no way to run current accounts on a non-MongoDB. But internally (in apollo-passport in general) I used Meteor user object format { emails: { address: "..." }, services: { facebook: { .... }, password: { ... } } - we can talk about this, but it's quite a convenient "standardized" way to pass everything around. It means for SQL we'd need to store the data in separate tables and reassemble it on load.

[TimMikeladze]

@gadicc the api looks good, it's pretty similar to what I have going on. Here's the base class https://github.com/apollo-accounts/apollo-accounts-server/blob/master/src/Accounts.js and here's the Sql adaptor for it https://github.com/apollo-accounts/apollo-accounts-knexjs/blob/master/src/index.js

If a repo is already created just move the existing code to a new branch and start a fresh on master. Getting coveralls setup would be great. I have CI configured to run the tests and linting, plus there's also a webpack dev server setup as well. That code might be useful.

[gadicc]

Ok great! I can also add coveralls, etc to any existing repo of course.

This might seem like a trivial issue, but have you had any more thoughts about naming? I just ask because it's a bit of a pain updating circle-ci, coveralls, etc afterwards. For now I think we're sticking with apollo-accounts, right? I spent sometime thinking about other names but anything cool was already taken :>

Btw, you're only waiting on me for a package skeleton, right? If I understood correctly, you're currently working on adapting apollo-accounts-knexjs to use the same API as the mongodb/rethinkdb packages? After that, I think, most of the server code will come from you (grant integration) and I could guess a lot of the client from me (self state management with optional redux adaptor). For react we said we'd use the same approach I used (supporting both redux and not) - for that, obviously the provider/connect I'll copy across, and then we can separate state management from one of the packages and get my basic react meteor theme working with it and your material-ui one.

I guess this will be a lot easier once we have final repos for each package and can open individual issues and assign them as needed.

[TimMikeladze]

On the topic of naming, there was a discussion here apollographql/apollo-server#163 that can be applicable here. Maybegraphql-accounts or graphql-server-accounts?

Yes waiting on you for a package skeleton. That sounds like a plan. I'm planning on dedicated a chunk of time towards this effort over the weekend.

[gadicc]

Argh, can't believe there's another name change :D Thanks for the ref and the quick reply. I guess graphql-accounts is the way to go then, except we use apollo-client 😕 I prefer without -server because we're a fullstack solution.

Ok great, yeah, I'm a bit behind but will get moving; also have more time coming up, probably only after the weekend though.

[TimMikeladze]

It's true that we're using apollo-client but I could see a case where we support any graphql client.

[gadicc]

Hey @TimMikeladze, looks like a lot of great work going on there :D I'm moving the passport stuff back to the original org for clarity (as per #10). Please let me know when you want me to look at anything or feel free to just assign stuff to me on github.

[TimMikeladze]

Thanks dude. Good call on moving the passport stuff. I'll be deleting my apollo-* repos as well.

I'll start assigning or mentioning you. Also been marking issues with the low hanging fruit label for quick stuff to pick up.