apollographql / apollo-client

:rocket:  A fully-featured, production ready caching GraphQL client for every UI framework and GraphQL server.
https://apollographql.com/client
MIT License
19.36k stars 2.66k forks source link

The inline argument "subjectId" is expected as a variable but was not provided. #732

Closed voodooattack closed 7 years ago

voodooattack commented 8 years ago

This error pops up every time I reload a page, even though the variables are there.

vars

errors

stubailo commented 8 years ago

Can you paste in your query as well?

voodooattack commented 8 years ago

@stubailo Here it is:

export const withGroups = graphql(gql`
  query ($subjectId: ID!, $params: SearchParameters) {
    subject(id: $subjectId) {
      id
      title
      description
      groups (params: $params) {
        count
        items {
          id
          title
          description
          createdAt
          updatedAt
        }
      }
    }
  }
`, {
  options(ownProps) {
    const query = queryToState(ownProps.location.query);
    return {
      variables: {
        subjectId: ownProps.params.subjectId,
        params: {
          limit: query.limit || 10,
          offset: query.offset || 0,
          order: query.sortKey ? `${query.sortDsc ? '-': ''}${query.sortKey}` : null,
          where: query.where && Object.keys(query.where).length ? query.where : null
        }
      }
    };
  },
  props({ ownProps, data }) {
    return {
      errors: data.errors,
      loading: data.loading,
      refetchQuery: data.refetch,
      item: data.subject,
      items: data.loading ? [] : data.subject.groups.items,
      queryCount: data.loading ? 0 : data.subject.groups.count,
    };
  }
});
stubailo commented 8 years ago

Are you sure params.subjectId isn't null or indefined sometimes?

voodooattack commented 8 years ago

Yes, I get that error even when it's supplied as per the screenshots.

stubailo commented 8 years ago

Right, but can you put something to log that value? It might be null for a moment and then switch to the correct value for some reason.

stubailo commented 8 years ago

I'll definitely look into it but just want to confirm the situation.

voodooattack commented 8 years ago

It's query parameter (part of the URL of the page), so it can't be null. I also tried logging it and it's there every time.

hellogerard commented 8 years ago

FWIW, I am getting this too on React Native, although it is inconsistent. That is, I only get it every few times I load a screen. I'm almost certain that the problem goes away if I removed forceFetch: true.

screen shot 2016-10-11 at 5 17 52 pm

j-jackson commented 8 years ago

I'm seeing the same thing.

The initial html displays the user data and then the client refreshes and displays loading with the error below. I have logged the state on the server & client and they both show that the username is available.

screenshot 2016-10-12 11 36 40
import React, { Component } from 'react'
import { graphql } from 'react-apollo'
import { connect } from 'react-redux'
import gql from 'graphql-tag'
import log from '../../log'

class Profile extends Component {
  render() {
    const { loading, user } = this.props
    return loading ? <div>Loading...</div> : <div>{user.username}</div>
  }
}

Profile.propTypes = {
  user: React.PropTypes.object.isRequired,
  loading: React.PropTypes.bool.isRequired
}

const CurrentUserForLayout = gql`
  query currentUser($username: String) {
    user(username: $username) {
      _id
      username
    }
  }
`

const SiteWithUserData = graphql(CurrentUserForLayout, {
  options: (ownProps) => ({ variables: { username: ownProps.username } }),
  props: ({ data: { loading, user } }) => ({
    loading,
    user,
  }),
})(Profile)

export default connect(
  (state) => ({ username: state.currentUser.username })
)(SiteWithUserData)
stubailo commented 8 years ago

This is a result of some query manipulation we were doing before sending to the server, but we're removing that in 0.5 which should come out shortly. Follow along here: https://github.com/apollostack/apollo-client/issues/726

j-jackson commented 8 years ago

@stubailo thanks for update. In the meantime is there a workaround or should we leave Apollo until 0.5 is released?

stubailo commented 8 years ago

Let me see if I can come up with something.

stubailo commented 8 years ago

Does anyone have an app I can clone to reproduce this error?

stubailo commented 8 years ago

Also, am I correct that in both cases Redux router is being used?

j-jackson commented 8 years ago

No I'm not using Redux router.

Let me create a repo for you.

conrad-vanl commented 8 years ago

@j-jackson are you by chance using redux-storage? We are, and I noticed that this error goes away if I remove apollo from redux-storage.

image

Also, we use redux-remote-devtools which has stopped working recently. Coincidently, removing apollo from redux-storage made the devtools start working again suddenly. Potentially there's a larger issue here? @hellogerard (above) and I are working on the same project - so this is a react-native app as well.

j-jackson commented 8 years ago

@conrad-vanl thank you for the suggestion but unfortunately I'm not using redux-remote-devtools. Over the past 2 weeks I have tried setting up the above query in a 3 different boilerplates with different configurations but I always end up with this error.

At this stage I'm only using redux to store the current user which I get on the server from the hostname:

export default (req, res) => {
  const currentUser = getUserByHostname(req.headers.host)  
// Current user is  { username: 'johndoe' ' domain: johndoe.com' }
  const initialState = { currentUser }

  match({ routes, location: req.originalUrl }, (error, redirectLocation, renderProps) => {
    if (redirectLocation) {
      res.redirect(redirectLocation.pathname + redirectLocation.search)
    } else if (error) {
      log.error('ROUTER ERROR:', error)
      res.status(500)
    } else if (renderProps) {
      const client = new ApolloClient({
        ssrMode: true,
        networkInterface: createNetworkInterface(apiUrl, {
          credentials: 'same-origin',
          rejectUnauthorized: false
        }),
      })

      const store = configureStore(initialState, client)

      const component = (
        <ApolloProvider store={store} client={client}>
          <RouterContext {...renderProps} />
        </ApolloProvider>
      );

      StyleSheetServer.renderStatic(() => getDataFromTree(component).then(context => {
        res.status(200)

        const { html, css } = StyleSheetServer.renderStatic(() => ReactDOM.renderToString(component))

        const page = <Html content={html} state={context.store.getState()} jsUrl={jsUrl} cssUrl={cssUrl} aphroditeCss={css}/>
        res.send(`<!doctype html>\n${ReactDOM.renderToStaticMarkup(page)}`)
        res.end()
      }).catch(e => log.error('RENDERING ERROR:', e)))
    } else {
      res.status(404).send('Not found')
    }
  })
}

Then my client:

const client = new ApolloClient({
  networkInterface: createNetworkInterface(apiUrl, {
    opts: {
      credentials: 'same-origin',
      rejectUnauthorized: false
    },
    transportBatching: true,
  }),
  queryTransformer: addTypename,
  dataIdFromObject: (result) => {
    if (result.id && result.__typename) { // eslint-disable-line no-underscore-dangle
      return result.__typename + result.id // eslint-disable-line no-underscore-dangle
    }
    return null
  },
  initialState: window.__APOLLO_STATE__,
  shouldBatch: true
})

const initialState = window.__APOLLO_STATE__
const store = configureStore(initialState, client)
const history = syncHistoryWithStore(browserHistory, store)

export default class Main extends React.Component {
  render() {
    return (
      <ApolloProvider store={store} client={client}>
        <Router history={history} routes={routes} />
      </ApolloProvider>
    );
  }
}

which appears to all work great except that I cannot access state.currentUser.username in my query even though I can see it in devTools and it I can see that it is there when I log it on the server and client.

const SiteWithUserData = graphql(CurrentUserForLayout, {
  options: (ownProps) => ({ variables: { username: 'johndoe'} }), // <= This works but it should be ownProps.username
  props: ({ data: { loading, user } }) => ({
    userLoading: loading,
    user: user,
  }),
})(Site)

export default connect(
  (state) => ({ username: state.currentUser.username })
)(SiteWithUserData)
conrad-vanl commented 8 years ago

@j-jackson possibly just a typo, but redux-storage (not devtools) is what's causing the error for us

j-jackson commented 8 years ago

@conrad-vanl yes sorry, it was a typo I meant redux-storage

pfulop commented 7 years ago

Had similar problem with different argument (obviously) but after trying 0.5 preview it seems to be fixed (at least in my scenarios).

helfer commented 7 years ago

@stubailo I just managed to reproduce this in react-apollo. The variable is sent to the server, so it may be a server issue, or something to do with the request format. edit: never mind, the server response is there, so it's almost certainly an error in the client.

helfer commented 7 years ago

Alright, I think I've gotten to the bottom of this. The error is thrown from graphql-anywhere because we provide it with the wrong variables for a query. The way that happens is that due to SSR (or store rehydration, to be more precise) there's a previous query in the store which happens to have the same id as the new query, so previousVariables is actually defined here.

I'm going to do three things to address this: 1) Update the docs to clarify that only the data part of the store should be sent across. 2) Make sure the initialState option to apollo client ignores query state. 3) Throw an error in APOLLO_QUERY_INIT if a query with that id is already in the store.

stubailo commented 7 years ago

Throw an error in APOLLO_QUERY_INIT if a query with that id is already in the store.

Is this better than completely overwriting that query?

helfer commented 7 years ago

I think it's better, because then it'll make noise when it fails, rather than silently failing somewhere down the line. I'm assuming that there's clearly something wrong if we end up with two queries that have the same id.

conrad-vanl commented 7 years ago

curious - could this possibly fix the error we're experiencing with using apollo with redux-storage? Edit: Ehhhhhh...i bet that's hard to answer :)

helfer commented 7 years ago

@conrad-vanl yes, definitely. You could try fixing it by just storing the data part of the store, like so:

state = { data: client.store.getState().data }

or whatever the redux-storage equivalent is. All you have to do is make sure the query part of the store is empty.

conrad-vanl commented 7 years ago

or whatever the redux-storage equivalent is. All you have to do is make sure the query part of the store is empty.

Gotcha... Yeah I remember now that was our working theory but we never got around to testing it, and now understand what you were saying above better.

helfer commented 7 years ago

So it turns out that we actually do want to overwrite queries in APOLLO_QUERY_INIT when we do a refetch or setVariables. I'll do a bit of refactoring to clean that up.

helfer commented 7 years ago

I fixed the store rehydration problem that was the proximate cause for the bug I saw in GitHunt, but I suspect that there are other bugs with the same symptoms. If anyone could make a reproduction (or provide a failing test-case) that would be great!

helfer commented 7 years ago

If anyone could test this with 0.5.0-1 and tell me if it still happens, that would be great!

voodooattack commented 7 years ago

@helfer Same issue, different error message:

Error

Here's the query:

export const withUser = graphql(gql`
  query ($userID: ID!) {
    user(id: $userId) {
      id,
      firstName,
      lastName,
      email,
      avatar,
      roles
    }
  }
`, {
  options(ownProps) {
    console.log(ownProps.params.userId);
    return {
      variables: {
        userId: ownProps.params.userId
      }
    };
  },
  props({ ownProps, data }) {
    if (data.error)  return { error: data.error };
    if (data.loading) return { loading: true };
    return {
      user: data.user,
      refetchUser: data.refetch
    }
  }
});

It logs the user ID as expected in the console.

helfer commented 7 years ago

@voodooattack that looks like a different error. Is that the same query that you had the other error with before?

And are you absolutely sure that you are using apollo-client 0.5.0-1??

voodooattack commented 7 years ago

@helfer Yes, I'm using 0.5.0-1.

The query is identical to the earlier query, the only difference is the return type.

It also logs the query variable correctly in the console.

Edit: My bad, it was a case sensitivity issue. Everything works now!

helfer commented 7 years ago

Alright, I'll close this then. We can reopen if necessary.

voodooattack commented 7 years ago

No, but It still happens with the original query as well (both are identical except for the return type)

Sent from my iPhone

On Oct 24, 2016, at 12:46 AM, Jonas Helfer notifications@github.com<mailto:notifications@github.com> wrote:

@voodooattackhttps://github.com/voodooattack that looks like a different error. Is that the same query that you had the other error with before?

You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/apollostack/apollo-client/issues/732#issuecomment-255620145, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ABOewvZkMFFtDqJ15Ri_w4M8i9idTrxEks5q2-O8gaJpZM4KMByD.

linonetwo commented 7 years ago

@voodooattack

My bad, it was a case sensitivity issue

I'm encountering case sensitivity issue too, thanks for mentioning that.