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

Simplify Apollo Auth Example #3350

Closed
adamsoffer opened this issue Nov 28, 2017 · 5 comments
Closed

Simplify Apollo Auth Example #3350

adamsoffer opened this issue Nov 28, 2017 · 5 comments

Comments

@adamsoffer
Copy link
Contributor

This part of the apollo auth example (the createUser & signinUser logic) feels a bit long winded.
https://github.com/zeit/next.js/blob/a8c344fa19ae242f90277391340463c5050b107e/examples/with-apollo-auth/pages/create-account.js#L46

Is it possible to simplify? Perhaps we can break this part up into another function or maybe even move it into it's own file. Thoughts?

@DevSpeak
Copy link
Contributor

DevSpeak commented Nov 29, 2017

@ads1018
The mutations could be placed in a seperate file.
Did you want all the logic to be moved into a file like example lib/auth.js ?

I didn't quite understand what you meant by simplify it?
Could also change to using state and async/await if that helps.

If you could explain some more what you're thinking, I would be happy to try put something together :)

@adamsoffer
Copy link
Contributor Author

adamsoffer commented Nov 29, 2017

@DevSpeak awesome! Yeah I think we can assign the graphql query and config object to two separate variables. For example:

const authQuery = gql`
  mutation Create($name: String!, $email: String!, $password: String!) {
    createUser(name: $name, authProvider: { email: { email: $email, password: $password }}) {
      id
    }
    signinUser(email: { email: $email, password: $password }) {
      token
    }
  }
`

const authConfig = {
  name: 'createWithEmail',
  ...
}

We can export these vars inside a separate file like you suggested or put them at the top of this one. Perhaps we rename checkLoggedIn.js to auth.js and export those variables inside there so all auth related functions and configurations are exported from a single file. These are just ideas.

I like the idea of using async/await :)

Also, we still need to update this example to apollo-client 2.0!

@DevSpeak
Copy link
Contributor

@ads1018 This is what I got so far, https://github.com/DevSpeak/next.js/tree/canary/examples/with-apollo-auth
Haven't arrived to the async/await idea yet.
Also for updating to apollo-client 2.0, I thought I already had did that? The tree's in next.js repo is confusing me.

@adamsoffer
Copy link
Contributor Author

Ah my bad, I was looking at the master branch. Sweet.

Great stuff! The only downside of putting the mutations in a separate file is that it's not immediately clear what the mutation prop does, but I think you make it easier on the reader by adding an inline comment to make note of where it's coming from. The page is much more succinct and easier to scan, and I like that all the logic related to auth can be found in a single file. 👍

@adamsoffer
Copy link
Contributor Author

We can close this issue now that #4070 is merged. Thanks @DevSpeak

@lock lock bot locked as resolved and limited conversation to collaborators Apr 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants