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

update mutation doesn't work #4031

Closed
masoud-moghini opened this issue Oct 20, 2018 · 29 comments
Closed

update mutation doesn't work #4031

masoud-moghini opened this issue Oct 20, 2018 · 29 comments

Comments

@masoud-moghini
Copy link

Intended outcome:
updated list of recipes be rendered

Actual outcome:
the old list get rendered

How to reproduce the issue:
follow what comes here !!!!

Versions

hi I have the following client

import React ,{Component}from 'react'
import { Mutation } from "react-apollo";
import {ADD_RECIPE} from '../../mutations';
import Error from '../Error';
import {withRouter} from 'react-router-dom';
import {GET_ALL_RECIPIES} from '../../queries'

class AddRecipe extends Component {
  ...
  onSubmit(event,addRecipe){
    event.preventDefault();
    addRecipe().then(
      ({data})=>
        {
          this.props.history.push("/")
        }
      )
  }
  render (){
    const{name,category,description,instruction,username} = this.state;
    return(<div className="App">
      <h2>Add recipe</h2>
      <Mutation
             mutation={ADD_RECIPE} 
             variables={{name,category,description,instruction,username}} 
             update={(cache, {data:{addRecipe}}) => {
                  const {getAllRecipes} = cache.readQuery({ query: GET_ALL_RECIPIES });
                  console.log(cache)
                  //console.log('get all recipes',getAllRecipes)
                  //console.log('add recipe',addRecipe)
                  cache.writeQuery({
                    query:GET_ALL_RECIPIES,
                    data:{getAllRecipes:getAllRecipes.concat[addRecipe]}
                  })
                  console.log(cache)
              }} >
      {(addRecipe, {data,loading,error})=>
        
           (<form className="form" onSubmit={event=>this.onSubmit(event,addRecipe)}>
                  <input type="text" name="name" onChange={this.handleChange.bind(this)} placeholder="recipe name" value={name}/>
                  <select name="category" value="breakfast" id="" onChange={this.handleChange.bind(this)} value={category}>
                    <option value="breakfast">breakfast</option>
                    <option value="lunch">lunch</option>
                    <option value="dinner">dinner</option>
                    <option value="snack">snack</option>
                  </select>
                  <input type="text" name="description" onChange={this.handleChange.bind(this)} placeholder="recipe description" value={description}/>
                  <textarea name="instruction" id="instruction" cols="30" rows="10" onChange={this.handleChange.bind(this)} value={instruction}> </textarea>
                  <button type="submit" className="button-primary" disabled = {loading || this.validateForm()}>submit</button>
                  {error && <Error error={error}/>}
            </form>)
        
      }
      </Mutation>
    </div>)
  }
}
export default withRouter(AddRecipe)

and for get Recipe is:

import gql from 'graphql-tag';


//RECIPE QUERY
export const GET_ALL_RECIPIES =gql`
query {
    getAllRecipes{
        name
        _id
        category
        
    }
}
`

and get recipe item is :

import gql  from "graphql-tag";

export const GET_RECIPE_ITEM =gql`
query($id:ID!){
        getRecipeItem(_id:$id){
    				
          _id
          name
          category
          description
          instruction
          createdDate
          likes
          username
		}
}`
@OurMajesty
Copy link

I think this is the same as #3992
Try apollo-cache-inmemory 1.2.10

@benjamn
Copy link
Member

benjamn commented Oct 23, 2018

Please try updating to the latest apollo-cache-inmemory and apollo-client:

npm install apollo-client@latest apollo-cache-inmemory@latest

@masoud-moghini
Copy link
Author

npm install apollo-client@latest apollo-cache-inmemory@latest

tried it and no result achieved.

@vu-bui
Copy link

vu-bui commented Oct 24, 2018

same problem with apollo-client 2.4.3.
reverting back to 2.4.2 works for me.

@bogdansoare
Copy link

having the same issue

@masoud-moghini
Copy link
Author

have the same problem in optimistic ui about delete mutation.
real time page refreshing doesn't work for react-apollo and this is a great shortcoming.

@Akryum
Copy link

Akryum commented Oct 28, 2018

With the latest apollo-client packages, I still have this issue where the watchQuery observer isn't firing a next result call after using writeQuery in a mutation.

I'm using those packages:

apollo-cache-inmemory@1.3.7
apollo-client@2.4.4

I have found a workaround: I need to create a new data object and clone the array and then it works correctly (as demonstrated below).

Here is a reproduction: https://codesandbox.io/s/42lqq8m9q9

@Akryum
Copy link

Akryum commented Oct 28, 2018

Note that it also works for me if I just create a new array using the same data object:

const data = store.readQuery({ query: WIDGETS })
data.widgets = data.widgets.filter(w => w.id !== this.widget.id)
store.writeQuery({ query: WIDGETS, data })

Edit: my bad, it doesn't work. A new data object needs to be created.

Akryum pushed a commit to vuejs/vue-cli that referenced this issue Oct 28, 2018
Akryum added a commit to vuejs/vue-cli that referenced this issue Oct 28, 2018
* feat: basic fonctionality, welcome and kill port widgets

* fix: contrast improvements

* feat: plugin/dep/vulnerability widgets design

* fix: widget add/remove animation

* feat: run task widget

* feat: news + wip resizing

* feat: nuxt

* chore: removed widget example

* fix: visual polish for widget transform

* feat(widget): overlap detection

* fix: news default/max size

* feat(dashboard): sidepane transition

* chore: dev api server port

* fix(widget): configure tooltip

* refactor(widget): generic Movable mixin

* refactor(widget): resizable mixin

* feat(widget): resize transition

* feat(widget): resize improvements

* refactor(widget): zoom factor

* refactor(widget): OnGrid mixin

* refactor(widget): resize handler style moved to global

* chore: remove console.log

* refactor: files structure

* feat: improved design and layout

* fix: content background vars

* fix: status bar / view nav z-indexes

* fix: webpack dashboard grid gap

* feat(news feed): handle errors

* fix(card): dimmed box shadow

* fix: view nav & status bar z-index

* fix: remove (wip)

* feat(widget): style tweaks

* feat(widget): details pane (wip)

* feat: news feed widget improvements

* feat(widget): custom header button

* feat(news): item details pane

* feat(widget): custom title

* fix(news): better cache and misc fixes

* feat(widget): resize left and top handles

* feat(widget): transparent widget while moving/resizing

* feat(news): better "big size" style

* fix(news): media sizes in rich content

* feat(plugin): local plugins support

* fix: scrolling issue in Fx

* fix: colors

* fix(nav bar): more item overflowing

* feat(vuln): frontend

* chore: locale update

* fix: image in suggestion dropdown (dev)

* fix(suggestion): missing custom image

* feat(view): user default plugin logo if no provided icon

* feat(view): better loading UX

* feat(view): button background if view is selected

* feat(view): new nav indicator

* feat(widget): use plugin logo as default icon

* feat(widget): better widget modal

* feat(widget): longDescription

* fix(widget): news validate url param

* feat(widget): filter widgets in add pane

* feat(widget): tease upcoming widgets

* chore: fix merge dev

* chore: yarn install

* chore: sync versions

* chore: update apollo

* docs: widget

* fix(progress): graphql error

* fix(deps): localPath

* perf(plugin): faster local plugin refresh

* fix(nav): center active indicator

* feat(task): improved header

* feat(client addon): custom component load timeout message

* feat(suggestion): ping animation to improve discoverability

* chore: update vue-apollo

* feat(api): requestRoute

* fix(suggestion): hide more info link if no link

* fix(style): ul padding

* test(e2e): fix plugin path

* chore: change test scripts

* chore(deps): upgrade

* fix: build error

* fix(widget): removed moving scale transform

* fix(widget): resize handles style

* chore(deps): unpin apollo-utilities

* chore(deps): lock fix

* test(e2e): fix server

* fix: issue with writeQuery

See: apollographql/apollo-client#4031 (comment)

* test(e2e): fix tests

* test(e2e): missing widgets build

* fix: missing widgets dep
@Akryum
Copy link

Akryum commented Oct 29, 2018

It seems sometimes adding an optimistic response fixes the issue: vuejs/apollo#436

@benjamn
Copy link
Member

benjamn commented Oct 29, 2018

@Akryum Thanks for the reproduction! Interestingly, the problem disappears for me when I apply #4069, though I don't have a complete theory as to why that makes a difference.

benjamn added a commit that referenced this issue Oct 30, 2018
This commit is a more conservative version of
e66027c

We still need to make a deep clone of observableQuery.lastResult in order
to determine if future results are different (see #3992), but we can
restrict the use of that snapshot to a single method, rather than
replacing observableQuery.lastResult with the snapshot.

Should help with #4054 and #4031.
@benjamn
Copy link
Member

benjamn commented Oct 30, 2018

@Akryum's reproduction is fixed by apollo-client@2.4.5, thanks to my PR #4069. Everyone else, please update your packages and try again. If the problem persists, a runnable CodeSandbox reproduction would really help us track it down.

@dsanders11
Copy link
Contributor

dsanders11 commented Oct 30, 2018

@benjamn, can confirm that apollo-client@2.4.5 fixes this issue for me.

However, I do notice that when deleting items, writeQuery isn't necessary to achieve the desired effect, probably because the result of readQuery is not a deep copy, and so modifying it (in this case, removing an item from an array) affects the store. This is with vue-apollo (and in turn apollo-cache-inmemory). Is this a known issue, or should I open a new issue? It seems that readQuery should logically return a deep copy for the in-memory cache, as otherwise behavior would be inconsistent with a different cache implementation.

EDIT: I saw in another issue you mentioned that results from readQuery shouldn't be mutated. As others have pointed out, the official docs do so, and it's not intuitive to make a copy before mutating. Other cache implementations don't necessarily need this rule. apollo-cache-inmemory should either deep copy on readQuery, or freeze, which you mentioned as a possibility, but that seems less ideal. A deep copy just works, where as freeze would need to be enforced at the apollo-cache level for consistency, and would need warnings in the docs since when not in strict mode, mutating a frozen object silently fails, which would confuse a lot of people.

@flytedan
Copy link

@Akryum thanks man! Yes the apollo-client@2.4.5 fixes the problem!

@masoud-moghini
Copy link
Author

I got its function confused with refetchQuery and if someone explains me what is the necessity of updating cache I would be grateful .

@benjamn
Copy link
Member

benjamn commented Nov 1, 2018

@dsanders11 To clarify, modifying result objects returned by readQuery only affects the cached result objects, not the data in the store, which can only be modified by writing new results.

Modifying data and then writing it back to the store is safe, since it invalidates the previous result objects, so readQuery will return new objects next time. That's what the official docs are doing, unless I'm mistaken.

The problem with a deep copy is that it's expensive, both because the copy itself takes time, and because identical results are no longer === identical, which makes equality comparisons in your application (e.g. to determine whether to re-render a component) more expensive. Making a deep copy is something you (as the consumer of the cache) can choose to do, if you want to pay for that convenience / peace of mind. In practice, when you copy your results, you don't usually need a full deep copy, because you can get away with shallow-copying parts of the result and modifying the partial copy without affecting the original object. Since the cache doesn't know what parts of the data you're planning to change, it has no hope of making these decisions correctly. Only you understand the needs of your application. The job of the cache is to enable all use cases, without making assumptions about where you might value safety over performance. If the cache itself always made a deep copy whether or not it was necessary, developers who don't want/need the copying behavior would be stuck with it anyway. In this area of the system, we've chosen to prioritize performance and developer choice over eliminating the possibility of misuse.

For those reasons, I think freezing the result objects is the only viable option here, though that would interfere with valid read-modify-write patterns, and would definitely require changing the documentation.

@ECIZEP
Copy link

ECIZEP commented Nov 18, 2018

@benjamn why are result objects returned by readQuery not consistent with the data in the store?
readQuery don't get data from the store, but return the cached result objects,so if i modify some items, there is no need to writeQuery to update the store,modifying the result objects is just enough。it seems that there are two cache layers,one is the result objects returned by readQuery, the other one is the store. This really makes me confused.
I also think the result objects should be immutable and frozen, or readQuery should return data from the store every time instead of return the result objects.

@charlie632
Copy link

Why is this issue still opened? It was resolved for me by installing the latests versions of apollo-client and apollo-cache-inmemory

npm install apollo-client@latest apollo-cache-inmemory@latest

@timbrandin
Copy link

I'm using these:

apollo-cache-inmemory@1.3.11
apollo-client@2.4.7

=> and updates are not fired when running writeQuery.

@timbrandin
Copy link

I just switched back to ReduxCache, and that resolved my issue, not sure I can trust InMemoryCache right now due to this.

benjamn added a commit that referenced this issue Feb 28, 2019
…bility.

Part of the plan I outlined in this comment:
#4464 (comment)

If we could trust application code not to modify cache results, we
wouldn't have to save deep snapshots of past results in order to implement
isDifferentFromLastResult correctly (see #4069).

Aside: why doesn't the cache just return defensive copies of all results?
#4031 (comment)

While you might agree that immutability is a worthwhile aspiration, it can
be hard to maintain that discipline across your entire application over
time, especially in a team of multiple developers.

This commit implements a new freezeResults option for the InMemoryCache
constructor, which (when true) causes all cache results to be frozen in
development, so you can more easily detect accidental mutations.

Note: mutating frozen objects only throws in strict mode, whereas it fails
silently in non-strict code. ECMAScript module code automatically runs in
strict mode, and most module transforms add "use strict" at the top of the
generated code, so you're probably already using strict mode everywhere,
though you might want to double-check.

The beauty of this implementation is that it does not need to repeatedly
freeze entire results, because it can shallow-freeze the root of each
subtree when that object is first created.

Thanks to result caching, those frozen objects can be shared between
multiple different result trees without any additional freezing, and the
entire result always ends up deeply frozen.

The freezing happens only in non-production environments, so there is no
runtime cost to using { freezeResults: true } in production. Please keep
this in mind when benchmarking cache performance!
@scottie-schneider
Copy link

Same issue, using
"apollo-cache-inmemory": "^1.6.2",
"apollo-client": "^2.6.2",

@leofuturer
Copy link

leofuturer commented Aug 7, 2019

@Akryum Thanks for your reproduction, but the codesandbox you provide does not work well now. It shows an error Error sending the query 'items' TypeError: Failed to fetch , it seems that the server has something wrong.

@JamieKudla
Copy link

Was having the same issue with the UI not reflecting the updated cache. Following the example in the docs, I could not get it working unless I created a new array for data.todos:

    options: {
      update: (proxy, { data: { createTodo } }) => {
        const data = proxy.readQuery({ query });
        // data.todos.push(createTodo);
        data.todos = [...data.todos, createTodo];
        proxy.writeQuery({ query, data });
      },
    },
"apollo-cache-inmemory": "^1.6.3",
"apollo-client": "^2.6.4",

@EdmundMai
Copy link

i had this problem too, then i inspected the record returned by the mutation and noticed that it was missing a field that the records in the cache had. i added that field to the mutation response and it worked

@finaxar-arthur
Copy link

@EdmundMai I thought I have the same problem. But I double checked my fields and created new array as @JamieKudla did.

Still didn't solve my issue.

@crisward
Copy link

crisward commented Oct 4, 2019

Not sure if this helps, but I found it works if my collection didn't have id's returned. When it did have id's I had to write a fresh object.

apollo-client@2.6.4
@apollo/react-hooks@3.1.2
apollo-boost@0.4.4
apollo-cache-inmemory@1.6.3

eg

// works with just re-creating array when no id's
export const GET_BOOKS = gql`{ 
  books { 
    title 
    author
  } 
}`

addBook({
  variables: {input:{ title: title.value, author: author.value }},
  update: (store, { data: { book } }) => {
    const data = store.readQuery({ query: GET_BOOKS }) // read books from cache
    data.books = [ ...data.books, book] // create new array with our book
    store.writeQuery({ query: GET_BOOKS, data}) // write back data
  },
})
// with id, new data object needed
export const GET_BOOKS = gql`{ 
  books { 
    id
    title 
    author
  } 
}`

addBook({
  variables: {input:{ title: title.value, author: author.value }},
  update: (store, { data: { book } }) => {
    const data = store.readQuery({ query: GET_BOOKS }) // read books from cache
    const books = [ ...data.books, book] // create new array with our book
    store.writeQuery({ query: GET_BOOKS, data:{...data,books} }) // write back new object with our books
  },
})

@dokicro
Copy link

dokicro commented Oct 7, 2019

I have the same problem but what is the different first time I add a product it updates but second it stops working...

If I console.log(store.readQuery) it shows correct data just UI is not reflecting it...

So as a workaround because I have to push to production on Friday is this:

const hasTransaction = this.bankAccount.transactions
              .find((transaction) => {
                return transaction.id === response.data.createBankTransaction.id;
              });

            if (!hasTransaction) {
              this.bankAccount.transactions.unshift(response.data.createBankTransaction);
            }

TLDR: If data is not added by write query I add it manually...

@thminggg
Copy link

Hi @crisward & all,
thanks for the comments up there. after your findings, is there any rule to follow to ensure the UI update accordingly?

like, always make new object or something?

the behaviour is so unpredictable, it's a nightmare...

@xhlsrj
Copy link

xhlsrj commented May 21, 2020

FYR,
"@apollo/client": "^3.0.0-beta.49"
When I use update like this, it does not work.

client.mutate({
// mutation,

update(cache){
  // const query;
  const data = cache.readQuery({ query });

  data.array.push({ key: value });

  cache.writeQuery({
    query,
    data,
  });
},

After I set a new object to data like this, it works.

update(cache){
  // ...
  cache.writeQuery({
    query,
    data: { array: [...data.array, { key: value }] },
  });
}

@thminggg
Copy link

Yup, always ensure the original cache object is immutable!
Assign new value by deep clone the object and assign new values will work!

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

No branches or pull requests