apollographql / react-apollo

:recycle: React integration for Apollo Client
https://www.apollographql.com/docs/react/
MIT License
6.85k stars 790 forks source link

Apollo cache executes local property resolvers for all objects even if they are added directly to the cache #3653

Open komyg opened 4 years ago

komyg commented 4 years ago

The Issue: Hi, I am creating a web app using React and Apollo that allow users to make orders at a given restaurant. In order to to this I created a shopping cart object that works as follows:

1 - When the user opens the web app, the restaurant menu is downloaded from our backend server and I added a few local fields to each product object to represent the quantity that the user chose for each one.

So, in my local schema I extended the Product type that came from the backend and added a few fields:

extend type Product {
  chosenQuantity: Int!
  isValid: ProductValidationStatus
}

And I created a local resolver to initialize them:

Product: {
    chosenQuantity: () => 0,
    isValid: () => 'UNKNOWN',
},

2 - Once the menu is downloaded, the user can make his choice of products which will change the @client / local variables above. So, if I choose a cake, than the cake chosenQuantity will be increased from 0 to 1 in the Apollo Cache.

3 - Once the user is satisfied with his choices, he can add the product to the shopping cart. When he does that, I create a duplicate of all the products he chose and add them to the local cache under a shopping cart object, during this process I also add a new unique ID to each product so that the user can chose different combinations of the same product. For example, if I add my cake to the cart, I will have this on the cache:

Before adding the cake to the shopping cart:

{
    "products": [
        {
            "name": "cake",
            "id": 1
            "chosenQuantity": 1,
        "isValid": "VALID",
        }
    ],
    "shoppingCart": []
}

After adding the cake to the shopping cart:

{
    "products": [
        {
            "name": "cake",
            "id": 1
            "chosenQuantity": 0,
        "isValid": "UNKNOWN"
        }
    ],
    "shoppingCart": [
        {
            "name": "cake",
            "id": 2
            "chosenQuantity": 1,
        "isValid": "VALID",
        }
    ]
}

So I end up with a "new" product on the Apollo Cache when this happens.

The problem happens when I query the shopping cart with the new product. When the query is executed Apollo returns to me the product with chosenQuantity: 0 and isValid: UNKNOWN instead of chosenQuantity: 1 and isValid: VALID.

For example if I execute the following query:

query GetShoppingCartProducts {
    shoppingCart @client {
        id
        chosenQuantity @client
        isValid @client
    }
}

I expected to receive:

"shoppingCart": [
    {
        "name": "cake",
        "id": 2
        "chosenQuantity": 1,
        "isValid": "VALID",
    }
]

Howerver the actual outcome is:

"shoppingCart": [
    {
        "name": "cake",
        "id": 2
        "chosenQuantity": 0,
        "isValid": "UNKNOWN",
    }
]

Possible root cause and work around:

After some investigation, I found out that this is happening because the Product resolvers (chosenQuantity: () => 0 and isValid: () => 'UNKNOWN') are being executed for the new product and resetting these values.

Is this the expected behavior of the Apollo and the Apollo Cache?

As a work around to my problem I created a resolver that check if there is a previous value assigned to these variables before updating the value:

Product: {
    chosenQuantity: setChosenQuantityInitialValue,
    isValid: setIsValidInitialValue,
}
export function setChosenQuantityInitialValue(
  root: Product,
  variables: any,
  context: { cache: InMemoryCache; getCacheKey: any },
  info: any
): number {
  let chosenQuantity = 0;
  if (root.chosenQuantity) {
    chosenQuantity = root.chosenQuantity;
  }

  return chosenQuantity;
}

export function setIsValidInitialValue(
  root: Product,
  variables: any,
  context: { cache: InMemoryCache; getCacheKey: any },
  info: any
): ProductValidationStatus {
  let isValid = ProductValidationStatus.Unknown;
  if (root.isValid) {
    isValid = root.isValid;
  }

  return isValid;
}

Is this the best soluton for this case? Or is there a better way to solve this problem?

Thanks, Felipe

Version

System: OS: Windows 10 Binaries: Node: 10.16.0 - C:\Program Files\nodejs\node.EXE Yarn: 1.19.0 - C:\Program Files (x86)\Yarn\bin\yarn.CMD npm: 6.9.0 - C:\Program Files\nodejs\npm.CMD Browsers: Edge: 44.18362.329.0 npmPackages: apollo-cache: ^1.3.2 => 1.3.2 apollo-cache-inmemory: ^1.6.2 => 1.6.3 apollo-client: ^2.6.3 => 2.6.4 apollo-link: ^1.2.12 => 1.2.13 apollo-link-context: ^1.0.18 => 1.0.19 apollo-link-error: ^1.1.11 => 1.1.12 apollo-link-http: ^1.5.15 => 1.5.16 apollo-link-rest: ^0.7.3 => 0.7.3 apollo-utilities: ^1.3.2 => 1.3.2

lensbart commented 4 years ago

I think you might want to model your data differently, so that you don’t have to duplicate entries etc. Apollo uniquely identifies entries based on their __typename and id. This can be configured by looking up dataIdFromObject in the docs.

I see you have two product arrays: one named products and another one named shoppingCart. This is redundant, how about just having one array with products and giving each an additional property inCart: Int! which is 0 by default? This way, Apollo won’t be confused between the same entry appearing in both arrays.

This inCart property can be added to the query for the shopping basket only, it doesn’t need to be there for the menu.

On the other hand, if the data returned by your menu query isn’t exhaustive (i.e. doesn’t contain all possible products), this approach may need to be tweaked. For example, if the user can navigate between paginated product lists, you can’t count on the shopping basket items to be present in the product menu array.

In that case, but this seems like a less elegant solution, you could change the dataIdFromObject function for the Product type, and have it include some extra field that indicates whether the product is in the cart.

Remember that the state should always be the minimal representation of the data you need.

komyg commented 4 years ago

Unfortunately I don't think that your suggestion would work for me, because each product can have children and grandchildren which represent the user choices, for example, the user can choose between different toppings on his cake or he can add a beverage to make a combo.

Since he can add the same parent product one or more times to the shopping cart, but with a different combination of its children, then I thought it would be better to duplicate the whole thing in the cache.