FormidableLabs / next-urql

Convenience utilities for using urql with NextJS.
MIT License
56 stars 7 forks source link

Execute a urql query inside getInitialProps #39

Closed addstar34 closed 4 years ago

addstar34 commented 4 years ago

I'd like to execute a urql query inside getInitialProps. The reason for this is to check if the current user is logged in or not and if not redirect before starting to render the page.

Here's my code:

// _app.js

function MyApp({ Component, pageProps }) {
  return <Component {...pageProps} />
}

MyApp.getInitialProps = async ctx => {
  // this seems hacky but ctx.ctx is what is available in a pages `getInitialProps` so 
  // I think it might be worthwhile putting urqlClient on `appCtx` in `withUrqlClient` HOC?
  const ctxWithUrql = { ...ctx, ctx: { ...ctx.ctx, urqlClient: ctx.urqlClient } }
  // calls page's `getInitialProps` and fills `appProps.pageProps`
  const appProps = await App.getInitialProps(ctxWithUrql);

  return { ...appProps }
}

export default withUrqlClient({url: process.env.GRAPHQL_ENDPOINT})(MyApp)
// pages/account.js

function Account(props) {
  // cache-only request because current user will be retrieved in `getInitialProps`
  const [result] = useQuery({ query: `{ currentUser { id } }`, requestPolicy: 'cache-only'})
  return (
    <div>
      <h1>Account id {result.currentUser.id}</h1>
    </div>
  )
}

export default withAuthRequired(Account);
// lib/auth/withAuthRequired.js

const executeUrqlQuery = async (client, query) => {
  return new Promise((resolve, reject) => {
    pipe(
      client.executeQuery(createRequest(query.query)),
      subscribe((op) => {
        // setting the requestPolicy here doesn't work, I'm not sure the correct way to set it?
        op.operation.context.requestPolicy = query.requestPolicy
        resolve([op.data, op.error])
      })
    )
  })
}

function withAuthRequired(WrappedComponent) {
  const withAuthRequiredWrapper = props => {
    return <WrappedComponent {...props} />
  }

  withAuthRequiredWrapper.getInitialProps = async ctx => {
    const { urqlClient } = ctx

    const [result, error] = await executeUrqlQuery(urqlClient, { query: `{ users { email } }`, requestPolicy: 'cache-only' });

    if (error) {
      // redirect to login
      return
    }

    let pageProps
    if (typeof WrappedComponent.getInitialProps === 'function') {
      pageProps = await WrappedComponent.getInitialProps.call(
        WrappedComponent,
        ctx
      )
    }

    return { ...pageProps }
  }

  return withAuthRequiredWrapper
}

I'm logging this as an issue for a couple of reasons:

JoviDeCroock commented 4 years ago

Hey @addstar34

On the client you can also use the client.query method, this returns an observable with an extra method named toPromise, I think that might help the executeUrqlMethod out:

const { data } = await urqlClient
    .query(myUserQuery, variables, { requestPolicy: 'cache-only' })
    .toPromise();

What do you mean with:

In withUrqlClient HOC it would be nicer if we put urqlClient on appCtx

It's on ctx, I'm probably misunderstanding the question

addstar34 commented 4 years ago

Hey @addstar34

On the client you can also use the client.query method, this returns an observable with an extra method named toPromise, I think that might help the executeUrqlMethod out:

const { data } = await urqlClient
    .query(myUserQuery, variables, { requestPolicy: 'cache-only' })
    .toPromise();

Oh this is nice and means I won't need that executeUrqlMethod. I just tested it out and while it works setting the request policy doesn't seem to work. I set the requestPolicy as cache-only but the request is still hitting the network, any ideas why that might be the case?

What do you mean with:

In withUrqlClient HOC it would be nicer if we put urqlClient on appCtx

It's on ctx, I'm probably misunderstanding the question

In _app.js I have to do const ctxWithUrql = { ...ctx, ctx: { ...ctx.ctx, urqlClient: ctx.urqlClient } } to make urqlClient available in other pages getInitialProps but it looks like @parkerziegler has a fix for this coming in #38

JoviDeCroock commented 4 years ago

I just ran the tests and it does seem to set the requestPolicy correctly on the Operation, was this network request the case before as well? If so we might just be missing a filter in one of our exchanges.

addstar34 commented 4 years ago

Do you mean was it the case before when I was using my executeUrqlQuery fn? If I use that function as per my first post and set cache-only yeah it still hits the network and thats why I thought I was setting it incorrectly.

Doing it as you suggested this is the code I ran:

const response = await urqlClient
    .query(`{ users { email } }`, {}, { requestPolicy: 'cache-only' })
    .toPromise()

console.log('response', response)

and here you can see the request hits the network:

image

JoviDeCroock commented 4 years ago

Yes we're probably missing a filter somewhere. This is a bug on urql itself

addstar34 commented 4 years ago

oh wow I'm glad we spotted that. Do you need me to do anything to get this over to the urql repo? I see you work there 😄

JoviDeCroock commented 4 years ago

@addstar34 I'll pick this up, made a draft PR. Thank you so much for notifying us!

addstar34 commented 4 years ago

no prob, thanks for your help @JoviDeCroock

addstar34 commented 4 years ago

Got another issue which I think might be related to this.

When I do the same query in getInitialProps and in the Component I get an error ReactDOMServer does not yet support Suspense.

Here's my code:

// authenticatedPage.js

import React, { useEffect } from 'react'
import Router from 'next/router'

function authenticatedPage(WrappedComponent) {
  const authenticatedPageWrapper = ({ redirect, ...rest }) => {
    useEffect(() => {
      if (redirect) {
        Router.replace('/sign-in')
      }
    }, [])

    if (redirect) {
      return null
    }

    return <WrappedComponent {...rest} />
  }

  authenticatedPageWrapper.getInitialProps = async ctx => {
    const { urqlClient } = ctx

    const { data, error } = await urqlClient
      .query(`{ users { id, email } }`, {}, { requestPolicy: 'cache-first' })
      .toPromise()

    if (error || !data?.users) {
      return { redirect: true }
    }

    let pageProps
    if (typeof WrappedComponent.getInitialProps === 'function') {
      pageProps = await WrappedComponent.getInitialProps.call(
        WrappedComponent,
        ctx
      )
    }

    return { ...pageProps }
  }

  return authenticatedPageWrapper
}

export default authenticatedPage
// pages/account.js

function Account() {
  const [result] = useQuery({
    query: `{ users { id, email } }`,
    requestPolicy: 'cache-only',
  })

  if (result.fetching) return <div>fetching</div>
  if (result.error) return <div>error fetching</div>

  const { data: { users: [{ id, email }] } } = result;

  return (
    <div>
      <h1>Account ID: {id} Email: {email}</h1>
    </div>
  )
}

export default authenticatedPage(Account)

As you can see I do this same query { users { id, email } } in both authenticatedPageWrapper.getInitialProps and within the page account. If I remove the email from the query in authenticatedPageWrapper.getInitialProps but leave it in account then the error is gone.

parkerziegler commented 4 years ago

@addstar34 if you're still struggling with second issue with duplicate queries, do you mind writing it up in a new issue description over in the urql monorepo? It's an interesting bug and one I'd love to see a minimal reproduction in a CodeSandbox for. Thanks!