[WIP] Fix various apollo-cache-inmemory 1.3.x bugs.

[benjamn]

Since publishing [email protected] officially, several bugs have been reported, so we have removed the latest tag from 1.3.0 and restored the old 1.2.10 version as latest. The most recent 1.3.x release is now tagged as next, so you can install it by running

npm install apollo-cache-inmemory@next

The more information we include in the cache keys used by the InMemoryCache, the more different cached results can be distinguished from one another. It's a bug if a change in some input value can change the result without also producing a different cache key. The typical solution is to include the relevant input data in the cache key returned by makeCacheKey. Note that not all input data needs to be included explicitly in the cache key, since some inputs are simply derived from other inputs.

Related bugs, possibly also fixed by this PR:

• After updating query data with subscribeToMore the query data in the component is empty #3962
• GraphQL query throws 'QueryKeyMaker cannot handle circular query structures' with simple React Native setup react-apollo#2442
• [apollo-cache-inmemory] 1.2.x -> 1.3.x can't perform introspection queries #3970

[carllippert]

So coming here from the slack channel to show whats going on. Mind you I don't even like the code but its where the bug lives. Having trouble making good looking code with apollo-link-state for interconnected state. Anyways.

This function amount other things is meant to add or remove an item from a list. It reads a fragment and determines what to do. It works the first time it is called. But the second time it is called the readFragment always returns the first fragment it ever returned. So the first ID passed to it will always be returned.

`setActiveBatchPen: (__, { pen_id }, { cache }) => {

            let id = `__Pen:${pen_id}`;
            let mountedID = `MountedPen:${pen_id}`;

            let pen = cache.readFragment({ fragment: penFragment, id: id});
          
            let mountedPen = cache.readFragment({ fragment: mountedPenFragment, id: mountedID});
           
            let previousActivePens = cache.readQuery({ query: activeBatchPens });
           
            let data = {};

            let selectedPen = previousActivePens.activeBatchPens.find((pen) => { return pen.pen_id == pen_id });
            
            //if the pen was selected before do 
            if(selectedPen) {
                
                //filter out the old pen that we are unselecting
                data.activeBatchPens = previousActivePens.activeBatchPens.filter((pen) => { return pen.pen_id != selectedPen.pen_id});
              
                //set selected in mounted pen for use in modals
                if(mountedPen){
                    cache.writeFragment({fragment: mountedPenFragment, id: mountedID, data: { ...mountedPen, selected: false }});
                }

            } else {

                let farmQuery = cache.readQuery({ query: activeFarm });

                 farm_id = farmQuery.activeFarm.farm_id;
           
                let feedingEvents = cache.readQuery({ query: feedingEventsQuery, variables: { farm_id }});
                let feedingEventsBlob = getFeedingEventsForPenForFeeding(feedingEvents.feedingEvents, pen_id);

                //add to activeBatchPens

                let newPen = Object.assign({}, 
                                            {...pen},
                                            {
                                            id: pen_id,
                                            __typename: 'ActiveBatchPen',
                                            adjustmentValue: 0,
                                            batchSizeError: 0,
                                            newTotalFedWeight: 0,
                                            dropOrder: 0,
                                            dropStartWeight: 0,
                                            plannedWeight: 0,
                                            dropEndWeight: 0,},
                                            {feeding_event_id: genUUID()},
                                            { feedingEvents: feedingEventsBlob.suggestedEvents });

                ///set inital weight for pen depending on a bunch of conditions. 
                //set totalWeight if we have suggestedEvents.
                if(feedingEventsBlob.suggestedEvents.length > 0) {
                   newPen.plannedWeight = feedingEventsBlob.suggestedEvents[0].weight;
                   newPen.newTotalFedWeight = feedingEventsBlob.suggestedEvents[0].weight;
                } else {
                     //if no suggestedEvents set num_of_hed * rationWeight
                     //get ration
                     let ration = cache.readQuery({query: selectedRation});

                     if(ration.selectedRation.length > 0){
                         //sum the ingredientweights to get a total ration weight
                         let ingSum = 0;

                         ration.selectedRation[0].currentVersion.ingredients.forEach((ing) => {
                            ingSum += ing.weight;
                         });

                         let penTotal = ingSum * pen.num_of_head;

                         newPen.plannedWeight = penTotal;
                         newPen.newTotalFedWeight = penTotal;

                     } else {
                         //no ration  so leave the weights at zero. 
                     }
                }
               
                data.activeBatchPens = previousActivePens.activeBatchPens.concat([newPen]);


                if(mountedPen){
                    cache.writeFragment({fragment: mountedPenFragment, id: mountedID, data: { ...mountedPen, selected: true }});
                }
            }

            //write some metadata about the pens and ingredients all togethor

            let totalBatchWeight = 0;

            data.activeBatchPens.forEach((pen) => {
                totalBatchWeight += pen.plannedWeight;
            });

            //write total batch weight to cache
            cache.writeData({ data: { totalBatchWeight: totalBatchWeight}});
            
            //write pens with feeding events to cache
            cache.writeQuery({ query: activeBatchPens, data });
        
            return null;    
        },`

You can see in this pic that the second time around calling this when a new fragment ID, in this case '__Pen:xxx-xxx' you get a different fragment. The fragment of the id passed on the previous call.

I updated to apollo-cache-inmemory 1.3.2 still this bug persists unfortunately.
Other deps.
react-apollo 2.2.3
apollo-client 2.4.2
react-native 0.57.1

I also rely on _.merge from lodash to connect all my resolvers. The only reason I bring that up is for some reason it seems occasionally my debugger will show undefined defaults or consts from other resolvers into debugger scope. Unsure what thats about.

[jelteliekens]

I'm experiencing exact the same problem. The first time it reads the correct fragment but the next time it will always return the first fragment even if the id is different.

Updating the package (reverts to 1.2.10) did resolve the problem for now.

[benjamn]

@jelteliekens Can you confirm you were using [email protected] rather than 1.3.0?

[benjamn]

So, I thought I had a reproduction, but I was mistakenly still using [email protected] instead of 1.3.2, and the problem went away for me after updating to 1.3.2. Granted, my reproduction was pretty simplistic (two consecutive client.readFragments with different IDs), but I would still like to make absolutely sure you're not somehow using 1.3.0, like I was!

[benjamn]

If [email protected] didn't fix the problem originally reported by @carllippert, I'm not sure the changes in 1.3.3 will help, but it might be worth a try, if you have a moment to run

npm install [email protected]

[jelteliekens]

@benjamn Apparently I also was using version 1.3.0. After updating to 1.3.3 this morning the problem went away so I think it's fixed now. Thanks!

[carllippert]

DUDE. It worked! Fragment match bug is fixed in 1.3.3 🎉 Pushing fix to our alpha channel right now to really see how the performance improvements are with 1.3 ( since this all started trying to improve performance ). Thanks so much for the fix!

[benjamn]

Awesome! By the way, the QueryKeyMaker issue is being tracked in apollographql/react-apollo#2442, in case you see that again.

[fbartho]

@benjamn -- Please let me know if you need any changes from apollo-link-rest or apollo-link-state to be more "spec-friendly" for these performance changes!

[benjamn]

@fbartho I don't think I have any warnings or suggestions for you right now. The problems with the Map polyfill are React Native-specific, and should be fixed soon.

[fbartho]

@benjamn Speaking as a user & contributor to apollo / apollo-link-rest / apollo-link-state on react-native, I'm invested 😉 -- My question was actually also more about if you need help testing any of these changes with apollo-link-rest.