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

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

Closed
voodooattack opened this issue Oct 2, 2016 · 36 comments
Assignees
Labels
Milestone

Comments

@voodooattack
Copy link

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

vars

errors

@stubailo
Copy link
Contributor

stubailo commented Oct 2, 2016

Can you paste in your query as well?

@voodooattack
Copy link
Author

@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
Copy link
Contributor

stubailo commented Oct 3, 2016

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

@voodooattack
Copy link
Author

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

@stubailo
Copy link
Contributor

stubailo commented Oct 3, 2016

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
Copy link
Contributor

stubailo commented Oct 3, 2016

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

@voodooattack
Copy link
Author

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
Copy link

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
Copy link

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
Copy link
Contributor

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: #726

@stubailo stubailo self-assigned this Oct 12, 2016
@stubailo stubailo modified the milestone: Release 0.5 Oct 12, 2016
@j-jackson
Copy link

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

@stubailo
Copy link
Contributor

Let me see if I can come up with something.

@stubailo
Copy link
Contributor

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

@stubailo
Copy link
Contributor

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

@j-jackson
Copy link

No I'm not using Redux router.

Let me create a repo for you.

@conrad-vanl
Copy link
Contributor

conrad-vanl commented Oct 14, 2016

@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
Copy link

j-jackson commented Oct 14, 2016

@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
Copy link
Contributor

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

@j-jackson
Copy link

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

@pfulop
Copy link
Contributor

pfulop commented Oct 18, 2016

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

@helfer
Copy link
Contributor

helfer commented Oct 19, 2016

@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
Copy link
Contributor

helfer commented Oct 19, 2016

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
Copy link
Contributor

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
Copy link
Contributor

helfer commented Oct 19, 2016

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
Copy link
Contributor

conrad-vanl commented Oct 19, 2016

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
Copy link
Contributor

helfer commented Oct 19, 2016

@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
Copy link
Contributor

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
Copy link
Contributor

helfer commented Oct 19, 2016

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
Copy link
Contributor

helfer commented Oct 20, 2016

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 helfer mentioned this issue Oct 20, 2016
7 tasks
@helfer
Copy link
Contributor

helfer commented Oct 21, 2016

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

@voodooattack
Copy link
Author

@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
Copy link
Contributor

helfer commented Oct 23, 2016

@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
Copy link
Author

voodooattack commented Oct 24, 2016

@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
Copy link
Contributor

helfer commented Oct 24, 2016

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

@helfer helfer closed this as completed Oct 24, 2016
@voodooattack
Copy link
Author

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 <[email protected]mailto:[email protected]> 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//issues/732#issuecomment-255620145, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ABOewvZkMFFtDqJ15Ri_w4M8i9idTrxEks5q2-O8gaJpZM4KMByD.

@linonetwo
Copy link

@voodooattack

My bad, it was a case sensitivity issue

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

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

8 participants