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

updateQueries updates _all_ instances of a parameterized query, not the "appropriate" one #930

Closed
GreenAsJade opened this issue Nov 17, 2016 · 10 comments

Comments

@GreenAsJade
Copy link

GreenAsJade commented Nov 17, 2016

I have a query that gets the graph of a specificproject, like this:

export const projectQuery = gql`
query projectQuery($projectId: Int!) {
  project(id: $projectId) {
    id archived
    address{shortDisplay}
    contract {client description originalPrice
      variations {id, statusId, reasonId, description}
    }
    primaryContact {contactId}
  }
}

and a mutation that adds a variation to a specific project like this:

 mutate({variables: {variation: variation, projectId: projectId},
                updateQueries: {
                    // Make sure that variation list is updated with the new variation
                    projectQuery: (prev, {mutationResult}) => {
                        console.log("createVariation projectQuery updater", prev, mutationResult)
                        return update(prev, {
                          project: {
                            contract: {
                              variations: {$push: [mutationResult.data.createVariation]}}}
                        })
                    }
                }
            })

The problem is that I can see this updateQueries function being called for every instance of the projectQuery that is in the store when the mutation runs.

This results in the information for every cached project being updated - so they all wrongly get a variation added to them, when only the specific one should have.

Debugging this issue raised another unexpected result: updateQueries is called for every instance of the query in the store - even ones that are identical. I have two components in a render tree that each make the projectQuery for the project that is on the route. This results in updateQueries being called twice even when the application has only ever looked at one project. That's quite surprising.

So:

  • Is it a bug that updateQueries doesn't handle selection of correctly parameterised instances of the query to operate on?
  • Is it a bug that the same query is cached multiple times (and thus updated multiple times by a given mutation)?
@yurigenin
Copy link

Please see issue #903. It basically mentions similar issues.

@yurigenin
Copy link

Another related one #902

@GreenAsJade
Copy link
Author

GreenAsJade commented Nov 17, 2016

I agree that the two referenced issues are about the same topic. #902 seems like the clear-cut bug part of this, and hopefully will get resolved quickly (the multiple instances of the same query in the store)

Unfortunately, #903 (parameterised queries all getting updated) got "contaminated" with a separate question about optimistic responses, and in the end I found it hard to understand which issue got the attention of the developers, and which was left dangling :(

This issue is specifically about the fact that all instances of a parameterized query get updated. It would be good to understand whether this is a clear cut bug also. At the moment it feels like it is: apollo has all the information it needs to determine which query instances should be updated by this mutation (IE it's those that have the matching parameters)

@helfer
Copy link
Contributor

helfer commented Nov 17, 2016

@GreenAsJade The behavior you described is actually the intended behavior, as unintuitive as it may seem at first. updateQueries simply updates a query by name, so if there are multiple instances of that query, updateQueries will run for each of them. The reason is that updateQueries was designed as a pretty flexible way to update any currently active query, not just the current one (that's why it's called updateQueries and not updateQuery).

What that means is that you have to deal with possible overlap in the cache and make sure the update is only pushed to the array once, etc. This is also true when you have a mutation and a subscription that update the same array in the cache, so I'd say it's not that surprising.

I agree that it might be worth adding in react-apollo that updates just the current query. The problem with that though is that we'd have to deal with all sorts of race conditions. We'll get around to doing that eventually, but for now you have to do it yourself. 😬

That said, you may also want to consider using the result reducers as documented here: http://dev.apollodata.com/react/cache-updates.html#resultReducers

Also note that you can check the matching parameters yourself, they are passed into updateQueries in the options parameter:

(parameter) options: {
    mutationResult: Object;
    queryName: Object;
    queryVariables: Object;
}

@yurigenin
Copy link

@helfer the problem is not that it updates multiple queries but that it updates multiple queries that are stopped long time ago and not active. I do not think it is by design?

@GreenAsJade
Copy link
Author

@yurigenin "Lingering 'inactive' queries" is not actually the problem addressed by this issue. I think that is "related but different", and covered by #902.

Here I am reporting that a mutation on an object of a specific id triggers updateQueries on all queries of all objects of that type (not just the specific one that was mutated).

As I type this, I realise that with the current API it couldn't be any other way: we can only tell updateQueries the name of the query that we want to update, but not the parameters that were involved.

This means that every cached query of a given type (name) will have the updateQueries function run on it.

To me this does not make sense. My app makes two queries:

       projectQuery(1)
       projectQuery(2)

Then I do a mutation on the content of project number 2. "Clearly" I should be able to say "updateQueries please update projectQuery(2)"

Therefore, I think that this issue is actually an "enhancement request", to make the updateQueries functions able to take parameters of the original query, and thus only be activated on those.

Note that in the mean time there is a "workaround", which is to check in prev whether this mutation should apply to this instance of the query.

I think the documentation needs to mention this, and I may submit a PR for that before too long.

@yurigenin
Copy link

@GreenAsJade You can do it already. There is a queryVariables parameter passed to your reducer. There you can check which values your variables have for this reducer call and only reduce state for those results that correspond to the current variable values.

@GreenAsJade
Copy link
Author

That sounds good. Where is it documented?

@yurigenin
Copy link

@GreenAsJade I don't think it is but you can discover a lot by tracing the call stack in the debugger.

@helfer
Copy link
Contributor

helfer commented Nov 18, 2016

@GreenAsJade here's the signature of the second argument to the updateQueries reducer:

(parameter) options: {
    mutationResult: Object;
    queryName: Object;
    queryVariables: Object;
}

As I write this I realize that we really need to improve our docs for this... -> https://github.com/apollostack/react-docs/issues/112

@helfer helfer closed this as completed Dec 17, 2016
@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.
Projects
None yet
Development

No branches or pull requests

3 participants