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

Parent fields are not available from readField in merge functions #374

Open
ab-pm opened this issue Jul 15, 2021 · 3 comments
Open

Parent fields are not available from readField in merge functions #374

ab-pm opened this issue Jul 15, 2021 · 3 comments
Labels
📕 cache Feature requests related to the cache

Comments

@ab-pm
Copy link

ab-pm commented Jul 15, 2021

The docs at https://www.apollographql.com/docs/react/caching/cache-field-behavior/#handling-pagination state

Note that if you call readField(fieldName), it returns the value of the specified field from the current object. If you pass an object as a second argument to readField, (e.g., readField("id", task)), readField instead reads the specified field from the specified object.

I was surprised that the first form does not work in the readField function passed to a custom merge function - since apollographql/apollo-client#5722 the options should work the same in both read functions and merge functions.

const cache = new InMemoryCache({
    typePolicies: {
        MyEntity: {
            fields: {
                myProperty: {
                    merge(existing, incoming, {readField, args}) {
                        console.log(`Merging MyEntity(${ readField('id') }).myProperty(${ JSON.stringify(args) })`);
//                                                       ^^^^^^^^^^^^^^^
                        
                    }
                },
            }
        },
    }
});

The use case might be to use the parent object's information, in addition to the field arguments, to figure out whether two objects should be merged or not. (I admit it's probably a bad idea to do that).
It's also useful for basic logging (while debugging) as in the snippet above. Unfortunately, the call just returns undefined instead of the entity id.

I'm not quite sure whether this should be a bug report against the library, a feature request (as it would be considered a new feature), or a bug report against the documentation.

@benjamn
Copy link
Member

benjamn commented Jul 19, 2021

@ab-pm Here's a comment from the source code that attempts to rationalize this decision:
https://github.com/apollographql/apollo-client/blob/971ecfcc20be748084c4b769c2a7518845c3919f/src/cache/inmemory/policies.ts#L813-L826
In short, readField('id') is ambiguous in merge functions, whereas there's only one thing it could mean in read functions. That's not an absolute and final answer, but it's a challenge to consider and solve.

The current recommendation is to use the readField("id", ref) style, but I realize it might be tricky to obtain that ref, especially if you can't use readField.

I would be open to exposing an options.entity property to read and merge (and cache.modify) functions, which would always be populated with a Reference or StoreObject representing the parent/enclosing object, making options.entity suitable to be passed as the second argument to readField. Calling readField("id", options.entity) would always read from existing data, and that data might be unreliable depending on the order of cache updates, but informational inspection/logging (as in your example) seems like a fairly harmless use case that would be worth supporting.

What do you think about that?

@ab-pm
Copy link
Author

ab-pm commented Jul 23, 2021

it's not clear what the current object should be for merge functions: the (possibly undefined) existing object, or the incoming object?

I don't understand how those would be choices. It should be the parent object of the field, which is not ambiguous. One can already easily access field values of the existing and incoming objects through readField('id', existing)/readField('id', incoming) (with guards against undefined and array values).

If you think your merge function needs to read sibling fields in order to produce a new value for the current field, you might want to rethink your strategy, because that's a recipe for making merge behavior sensitive to the order in which fields are written into the cache.

Oh, that's a very good point I had not though of. However, I was intending to only read constant properties with primitive values from the parent object, which would never be updated.

I would be open to exposing an options.entity property to read and merge functions, which would always be populated with a Reference or StoreObject representing the parent/enclosing object. What do you think about that?

I'd be very happy to have that available! And it would seem cleaner to always explicitly pass that as the second argument to readField. However, I think that for consistency, the readField(name) (without a reference) should always fall back to that entity, not only inside read functions but also in merge functions.

@MichaelGoff
Copy link

I've run into a similar issue in a merge function where I'm getting Undefined 'from' passed to readField with arguments ["id"] when trying to readField('id') without a ref.

Having access to an entity field in the options would resolve my issue as I'm trying to read the parent node out of the cache. In my case, I don't care if this information is stale as the id field will never change.

@alessbell alessbell transferred this issue from apollographql/apollo-client Apr 28, 2023
@alessbell alessbell added the project-apollo-client (legacy) LEGACY TAG DO NOT USE label Apr 28, 2023
@jerelmiller jerelmiller added 📕 cache Feature requests related to the cache and removed project-apollo-client (legacy) LEGACY TAG DO NOT USE labels Jan 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📕 cache Feature requests related to the cache
Projects
None yet
Development

No branches or pull requests

5 participants