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

NextJS Example using cookieStorage [WIP] #804

Closed
wants to merge 15 commits into from
Closed

NextJS Example using cookieStorage [WIP] #804

wants to merge 15 commits into from

Conversation

agustif
Copy link
Contributor

@agustif agustif commented Sep 14, 2019

AccountsJS + NextJS (SSR)
related to issue #802
This is very much a WIP

Although, it seems Home/Login/Signup work as it is.

It's using apollo-link, I haven't had any problems with localStorage for now.

Still need to work out dynamic routing for reset_password. two_factor. and verify_token.

Feedback very much appreciated @pradel @TimMikeladze @davidyaha

@codecov
Copy link

codecov bot commented Sep 14, 2019

Codecov Report

Merging #804 into master will decrease coverage by 0.1%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #804      +/-   ##
==========================================
- Coverage   95.27%   95.17%   -0.11%     
==========================================
  Files          81       80       -1     
  Lines        1819     1781      -38     
  Branches      355      371      +16     
==========================================
- Hits         1733     1695      -38     
  Misses         78       78              
  Partials        8        8
Impacted Files Coverage Δ
packages/client/src/token-storage-local.ts 100% <ø> (ø) ⬆️
...hql-api/src/modules/accounts/resolvers/mutation.ts 92.3% <0%> (-1.25%) ⬇️
packages/server/src/accounts-server.ts 90.81% <0%> (-0.65%) ⬇️
packages/rest-client/src/rest-client.ts 97.05% <0%> (-0.13%) ⬇️
packages/client/src/accounts-client.ts 98.71% <0%> (-0.04%) ⬇️
packages/rest-express/src/express-middleware.ts 100% <0%> (ø) ⬆️
...est-express/src/endpoints/verify-authentication.ts

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 2af6c19...776f7c6. Read the comment docs.

@agustif agustif changed the title NextJS Example [WIP] NextJS Example using cookieStorage [WIP] Sep 15, 2019
Copy link
Contributor Author

@agustif agustif left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks for cleaning that mess! I guess I need to learn to refactor better myself, I just hacked it away until it worked haha!

Copy link
Member

@TimMikeladze TimMikeladze left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good so far.

Could you expand this example to also include an apollo client which uses the accounts apollo link to send the tokens to the app's graphql server.

);

HomePage.getInitialProps = async function(ctx) {
const token = auth(ctx);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once you have the access token you need to called accountsServer.resumeSession which will verify the token, the session, and return the authenticated user. This is similar to what the context builder does in our graphql package.

You can find an example of that code here

export const context = (moduleName: string) => async (

@pradel
Copy link
Member

pradel commented Sep 19, 2019

Could you expand this example to also include an apollo client which uses the accounts apollo link to send the tokens to the app's graphql server.

@TimMikeladze that's a good idea, maybe our examples should contain some custom queries / mutations to show how you can integrate it with your code (something simple, maybe a todo list?)

@TimMikeladze
Copy link
Member

@agustif I pushed some additional code to add a graphql endpoint and create an accounts server with typeorm. It's on the https://github.com/accounts-js/accounts/tree/example-nextjs branch.

I'm not quite sure how to go about pushing changes directly to this PR... @pradel any ideas?

@pradel
Copy link
Member

pradel commented Sep 21, 2019

@TimMikeladze I am using https://desktop.github.com/ where you have a gui to clone a pr directly on your machine and then you can push on the same pr :)

@agustif
Copy link
Contributor Author

agustif commented Sep 22, 2019

I accepted all incoming changes @tim so yarn.lock wouldn't bitch! seems OK now!

@stolinski
Copy link
Contributor

This is awesome. Thank you. I can't wait to dive into this pr.

@TrillCyborg
Copy link

I was having some trouble getting the TokenStorage to work in this example so I implemented my own. Hopefully this will help someone else who ran into this issue as well.

import cookies from 'next-cookies'
import Cookies from 'js-cookie'

export const tokenStorage = (ctx: any) => ({
  setItem: async (key: string, value: string) => {
    Cookies.set(key, value)
  },
  getItem: async (key: string) => {
    const allCookies = cookies(ctx)
    const item = allCookies[escape(key)]
    return item
  },
  removeItem: async (key: string) => {
    Cookies.remove(key)
  }
})

@pradel
Copy link
Member

pradel commented Oct 7, 2019

@TrillCyborg I am curious, how do you do to inject the token storage to accounts-js afterwards (I guess inside getInitialProps right)?

@TrillCyborg
Copy link

@TrillCyborg I am curious, how do you do to inject the token storage to accounts-js afterwards (I guess inside getInitialProps right)?

Precisely. I pass the ctx in getInitialProps to tokenStorage. Since the app is SSR the Apollo/Accounts client needs to be created on each request.

@agustif
Copy link
Contributor Author

agustif commented Oct 15, 2019

Hey could someone maybe help wrap this up?

Would love to see it merge, but I'm not sure what's missing by @TimMikeladze requsets also maybe you @pradel can help?

Thank you guys

@stolinski
Copy link
Contributor

I'm working on my own implementation on this and am hung up on a few things. Next recommends a HOC approach.

https://github.com/zeit/next.js/blob/canary/examples/api-routes-apollo-server-and-client/apollo/client.js

Here, they use an initApolloClient function to initialize the client.

I was able to get user accounts working with SSR using this approach, https://github.com/zeit/next.js/blob/canary/examples/with-apollo-auth/lib/apollo.js
However not with accounts-link. Besides the big bug in accounts-link causing an infinite loop, there is a lot of messy-ness around the load order of things.

Has anyone unlocked this approach completely?

@TrillCyborg
Copy link

I'm working on my own implementation on this and am hung up on a few things. Next recommends a HOC approach.

https://github.com/zeit/next.js/blob/canary/examples/api-routes-apollo-server-and-client/apollo/client.js

Here, they use an initApolloClient function to initialize the client.

I was able to get user accounts working with SSR using this approach, https://github.com/zeit/next.js/blob/canary/examples/with-apollo-auth/lib/apollo.js
However not with accounts-link. Besides the big bug in accounts-link causing an infinite loop, there is a lot of messy-ness around the load order of things.

Has anyone unlocked this approach completely?

I have accounts.js working great in a next.js project using this HOC method. What messy-ness are you running into?

@stolinski
Copy link
Contributor

stolinski commented Nov 14, 2019

I have accounts.js working great in a next.js project using this HOC method. What messy-ness are you running into?

I'd love to check out your implementation, or at least your withApollo. I have things a bit more worked out now, but I'm having a hard time getting the session refreshed without using accounts-link. I'll clean my up a bit and post it in a gist here.

@stolinski
Copy link
Contributor

@TrillCyborg After hacking away on it today, I'm running into issues regarding the order of initializing things where GraphQLClient requires apolloClient to be initialized, but apolloClient is only initialized within withApollo, however since accountsPassword needs to be exported, the whole thing becomes circular. I guess at the end of the day, I'm not able to get the loading order correct

@TrillCyborg
Copy link

TrillCyborg commented Nov 14, 2019

@TrillCyborg After hacking away on it today, I'm running into issues regarding the order of initializing things where GraphQLClient requires apolloClient to be initialized, but apolloClient is only initialized within withApollo, however since accountsPassword needs to be exported, the whole thing becomes circular. I guess at the end of the day, I'm not able to get the loading order correct

This is what's working for me. You need to use the HOC in each page that talks to GraphQL. I hope it helps you. https://gist.github.com/TrillCyborg/7ce2ba18e89176cbff02f1b86a262ca1

@stolinski
Copy link
Contributor

Thank so much for this. @TrillCyborg moving everything to react context was the missing piece of the loading order puzzle.

@stolinski
Copy link
Contributor

@TrillCyborg all is working well with this approach and I've implemented exactly, however because opts.ctx is undefined on creation of the accounts, I'm getting TypeError: Cannot read property 'req' of undefined at nextCookies (index.js:8) when using the tokenStorage to log in.

It seems like when the React context is created client side , the accountsGraphQL, accountsPassword, accountsClient are created with ctx empty. What was your solution for this?

@TrillCyborg
Copy link

@TrillCyborg all is working well with this approach and I've implemented exactly, however because opts.ctx is undefined on creation of the accounts, I'm getting TypeError: Cannot read property 'req' of undefined at nextCookies (index.js:8) when using the tokenStorage to log in.

It seems like when the React context is created client side , the accountsGraphQL, accountsPassword, accountsClient are created with ctx empty. What was your solution for this?

Are you using the withApollo HOC on all your pages talking to Apollo or accounts.js?

Since the HOC uses getInitialProps you need it on components with a default export in the pages directory.

@stolinski
Copy link
Contributor

stolinski commented Nov 15, 2019

@TrillCyborg Yah, wrapping pages like...

const Home = () => {
  return (
    <MainLayout>
      <Hero />
    </MainLayout>
  )
}

export default withApollo(Home)

All mutations give same error.

Before I had deleted the previously saved cookies, the code was SSRing the logged in user just fine, so the server side loading of things work fine.

ctx is defined on server side within tokenStorage but not client for some reason. I would post more code, but my withApollo and apollo.ts files the exact same as yours currently.

I'm attempting login via context

const { accountsPassword } = useContext(ApolloContext)
...
      await accountsPassword.login({
        user: {
          email,
        },
        password,
      })

I'm just now realizing that getInitialProps is only called client side via client side routing by NextJS design. So accountsPassword and the like are being created by this line in withApollo


    const {
      client,
      accountsGraphQL,
      accountsPassword,
      accountsClient,
    } = useMemo(
      () => apolloClient || initApollo({ initialState: apolloState }),
      []
    )

Which is initializing without passing ctx, which is why it's unavailable client side.

@stolinski
Copy link
Contributor

I don't know why I didn't think of this sooner, but I just adjusted the getItem method.

import cookies from 'next-cookies'
import Cookies from 'js-cookie'

export const tokenStorage = (ctx: any) => ({
  setItem: async (key: string, value: string) => {
    Cookies.set(key, value)
  },
  getItem: async (key: string) => {
    let allCookies
    if (typeof window !== 'undefined') {
      allCookies = Cookies.get()
    } else {
      allCookies = cookies(ctx)
    }
    const item = allCookies[escape(key)] || allCookies[key]
    return item
  },
  removeItem: async (key: string) => {
    Cookies.remove(key)
  },
  getItemClientSync: (key: string) => {
    return Cookies.get(key)
  },
})

@edw19
Copy link

edw19 commented Mar 16, 2021

Is there a better way to use a HOC for JS accounts with the new apollo configuration?
https://github.com/vercel/next.js/blob/canary/examples/api-routes-apollo-server-and-client/apollo/client.js
with this code

@jamie-oconnell
Copy link

Hi All,

Does anyone have a recent example for using accounts-js client with nextjs?

@agustif agustif closed this Nov 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants