[v3] cache.modify with optimistic result does not broadcast change

[martaver]

I have a schema where a Taxonomy is a self referencing node, in a one-to-many parent child relationship:

type Taxonomy {  
  id: ID!
  title: String! @search(by: [fulltext])
  parent: Taxonomy
  children: [Taxonomy] @hasInverse(field: parent)
}

The @hasInverse directive is dgraph-specific, and sets up a two-way relationship, that updated the inverse edge when one is modified.

I am attempting to optimistically add a Taxonomy node under a parent in the following mutation:

mutation AddTaxonomy ($title: String!, $parentId: ID!) {

    addTaxonomy(input: [{
        title: $title,
        parent: {
            id: $parentId
        }
    }]) {
        taxonomy {
            id,
            title,
            parent {
                id
            }            
        }
    }
}

I want the UI to update optimistically, inserting a temporary object until the query is completed, so I specify a compatible optimistic result.

Apollo doesn't know anything about the @hasInverse directive, so I create options that provide an update function to modify the cache and keep the parent's children up-to-date in each case.

    const id = getTmpId();    

    const options = {
        variables: {
            title,
            parentId: parentId
        },
        optimisticResponse: {
            __typename: "Mutation",
            addTaxonomy: {
                __typename: "AddTaxonomyPayload",
                taxonomy: [{
                    __typename: "Taxonomy",
                    id,
                    title: "TEMP",
                    parent: {
                        __typename: "Taxonomy",
                        id: parentId
                    }
                }]
            }
        },
        update: (cache, { data: { addTaxonomy } }) => {
            const added = addTaxonomy.taxonomy[0];
            const parent = added.parent;

            cache.modify({
                id: cache.identify(parent),
                fields: {
                    children: (existing: Reference[]) => {   
                        const newTaxonomy = cache.writeFragment({
                            data: added,
                            fragment: gql`
                              fragment NewTaxonomy on Taxonomy {
                                id
                                title
                                parent {
                                    id
                                }
                              }
                            `
                          });
                        return [...existing, newTaxonomy]
                    }
                }
            });
            
        }
    }

Both results (optimistic and actual) reach the update function. Both results are the same shape. Only the actual result from the query emits in a change in the query data shown in my UI. The temporary Taxonomy is nowhere to be seen.

I might be missing something, but it sure seems like a bug to me.

More, after I delete and add the node a few times in succession, eventually the add fails silently, and the result data from the add payload gets emitted to the UI as 'undefined'.

Not sure what's going on there...

Is AC3 still ironing out bugs with this kind of stuff?

[benjamn]

@martaver Can you put this code into a runnable reproduction using either this repo or this CodeSandbox? You seem to be using cache.modify correctly, but I can't tell from the code you've provided what might be interfering with the broadcast. I promise more concrete suggestions in exchange for a reproduction!

[martaver]

Hey @benjamn thanks, these are quite handy :D

I'm trying to put together a repro here: https://codesandbox.io/s/dazzling-morse-dlg23?file=/src/App.js

But I'm running up against a brick wall... I added a mutation to add a child person to a parent, and an update function to add the response to the parent's children. The field modifier doesn't run, either for the optimistic response or the response from the link.

So obviously I've got something very wrong here... Also, when I try to check the contents of the cache, all the keys are there, but the values are all null.

Am I missing something very obvious here?

Sorry, it seems I'm much greener to GraphQL than I gave myself credit for, and I've had to eat a bit of humble pie :-/

[martaver]

The problem with all cached keys being null also exists in the vanilla repro CodeSandbox: https://codesandbox.io/s/divine-river-fggny?file=/src/App.js

I'm not sure how to proceed now. I kind of need the cache working to be able to check myself in the repro. Or if it is working as intended, then I'm so confused.

[benjamn]

@martaver Ok, thanks very much for sharing (the beginnings of) a reproduction! We'll take a look, and hopefully either identify/fix the problem or clarify how things are supposed to work.

[jcreighton]

@martaver I've been able to resolve the issue with the above CodeSandbox repros here. Press the Add friend button to run the mutation with an optimistic result. I see the optimistic result in the UI but there's a chance I didn't re-implement this as close to your use case above. If so, please feel free to modify it!

[martaver]

Oh geez guys, I'm sorry, I gave you the wrong repro link... here's what I meant to send you:

https://codesandbox.io/s/dazzling-morse-dlg23?file=/src/App.js

Please note in it the schema, my use of the link to provide peopleData instead of the resolver in the query, and the fact that when I console.log the local cache, all values are indexed, but are null.

@jcreighton thanks for your repro, but I think it might be side-stepping the issue that I'm trying to point out. Notice that you are returning peopleData in your AllPeople query's resolver. The peopleData variable is also mutated directly by the AddPerson and AddFriend mutations. Unless I'm misunderstanding how the client works, I think your approach skips the cache all together and simply returns peopleData all the time which, of course, is correct because you mutated it directly.

In my case, I am trying to act on values that are returned from a link in response to AllPeople (check my repro and note that I'm not using resolver in my AllPeople query, instead I'm returning peopleData through the link).

My understanding is that, based on the schema defined, apollo client should normalise the payload returned from the link and split it into independent people entries in the local cache. Those entries should contain a children property with refs to one-another as appropriate for their parent-child relationships.

I know that only one side of that relationship will be indexed by apollo client, so I want to write an update function so that updates children when a new parent is set. For this I create an AddChild mutation, which you can trigger with the 'test' button. It's supposed to add a child person to Person with id '1'.

Only neither the optimistic result nor the response from the link are correctly ingested by the cache.

When I serialize the local cache to console.log, I see that there are entries for each Person, including the new child, but they are all null. I was hoping to be able to debug my problem by inspecting the cache...

So I don't really know what's wrong here... Am I handling my query and mutation in the link incorrectly...? Is this a bug in apollo (unlikely, I hope...?)

The end result I am looking for is that the child appears immediately underneath Person with id '1', optimistically, and then is updated with the result from the link.

[jcreighton]

@martaver Thank you for the updated repro link! I've made some modifications and it's working, check it out: https://codesandbox.io/s/green-cdn-hef5f?file=/src/App.js

These are the changes:

• Add typename to the parent object in both App.js (line 39) and link.js (line 56). This is required for cache.identify (App.js, line 49) to create the correct ref.
• Send the correct data to writeFragment (App.js, line 53). Previous to this, the data object referenced here was the data from the AllPeople query. You can see this on line 47 where I’m logging out the data. This meant that writeFragment was unable to identify the correct ref for the child.
• Changed “title” to “name” (App.js, lines 38 & 57, Link.js, line 55). Without this change, the UI didn’t render the new child.

This was tricky to resolve and would've been easier to catch if writeFragment didn't return undefined when it can’t identify the object. writeFragment should throw an error in that case. @benjamn Cool if I assign myself to tackle that? (And should that be a new issue or keep it as part of this one?)

[benjamn]

writeFragment should throw an error in that case. @benjamn Cool if I assign myself to tackle that?

@jcreighton Go for it (in a new PR please)!

[martaver]

@jcreighton thanks for the update, this looks like its working fine :)

Can I ask a few questions about your implementation?

Why is writeFragment necessary? Shouldn't the optimistic result and then the query result automatically be normalised and added to the local cache?

Or does using the update function replace the default cache behaviour in some way?

Ideally, I would just like to leave apollo to work 'as normal', and simply be able to tweak the parent's children, leaving everything else as is.

[benjamn]

@martaver The reason writeFragment is necessary is that cache.modify by itself doesn't do any normalization or run read or merge functions. It's a lower-level API that gives you internal cache data and expects you to return the same. An alternative is to use cache.readFragment to get the current children array, append a non-Reference object to it, and then use cache.writeFragment to put that array back into the cache (which is what you would have to do in AC2), but that ends up being a little more computationally expensive and verbose than cache.modify in this case.

[martaver]

Hi,

Sorry for pressing this - I'm just really trying to understand the reasoning behind how this caching works, and there are things that aren't making sense for me.

So I'm looking at @jcreighton 's solution here again: https://codesandbox.io/s/green-cdn-hef5f?file=/src/App.js and now writeFragment is missing from it. I'm guess it was updated?

So it looks like writeFragment isn't necessary?

Also, I'm trying to use cache.modify as in this example in another app, also with an optimistic response. In my other app, the optimistic result does not render in the UI, even though the mutation result does. If I inspect the cache throughout all of this, I can see the optimistic record and actual record added to the cache. If I watch the parent's collection of children, both the optimistic record and the actual record are added to the list of children (the optimistic record being replaced by the actual one, correctly). However, in the UI, only the actual result renders - there is no render update for the optimistic result.

One thing to note is that after cache.modify is done, the actual JSON object is added to the list of children, not a ref to it.

I don't think that's related to the problem - but it was odd, at any rate - I kind of assumed that I could rely on apollo to keep anything with a __typename and id normalized for me.

Anyway, finally I found the culprit causing my optimistic response to fail - a missing field in my optimisticResponse - which was returning from the api as null anyway!

This 100% should not have failed silently. If there is something wrong with either payload... from the optimistic response or the server, which can cause the client to withhold rendering or otherwise behave erratically - this needs to be surfaced to the user immediately.

I think any of these read/write errors should be, if they aren't already!

[jcreighton]

@martaver I accidentally introduced that change when reproducing something else. I reverted it.

[martaver]

Hi, okay...

So I'm confused now... I'm assuming that apollo client has default behaviour for consuming payloads from mutations - where the payload is normalized and cached by typename and key.

Does defining an update function circumvent that default behaviour? Is that why writeFragment is necessary?

In that case - why did the example still work fine when writeFragment was removed? Is that because the whole object ends up added to the children array instead of just a reference?

If that's not the case, and the default behaviour still happens - why do we need the writeFragment code? Shouldn't I just be able to look up the entry from the default behaviour and patch children?

Ideal scenario would be to accept default behaviour without having to write a fragment, and simply patch children.

Actually, the most ideal behaviour would be to be able to define a TypePolicy somehow that can automatically keep parent and children fields in sync. After all, these relational rules should be universal!

[martaver]

Actually I figured I'd try the type policy approach. It would be hand if the cache knew how to update children on the parent automatically whenever the child's parent property was set. And vice versa.

It seems that's not possible though because there's no way that I can get a reference to the present object being added when inside the field merge function for 'parent'. E.g.

{
    parent: {
              merge: (existing, incoming, {readField}) => {
                console.log('own id: ', readField<string>('id')); // This is undefined
                console.log('parent id: ', readField<string>('id', incoming)); // But this returns the correct id
                return incoming;
              },
            }
}

Here's a reproduction: https://codesandbox.io/s/tender-mountain-hq3li?file=/src/index.js

Am I missing something obvious here? Shouldn't I be able access the current object being merged?

[jcreighton]

Closing this as the initial issue was resolved.