V3 InMemoryCache: Object Fieldname Characters too Restrictive

[jamshally]

I have a nested object that uses Guids as the fieldname:

var user = {
  id: '1',
  nestedObject: {
     'd7eb901a-cc21-4f8f-93f4-0408406e6a00': 'some value'
  }
}

I use cache.writeData to update the nestedObject:

cache.writeData({
  id: defaultDataIdFromObject(user)
  data: { 
    nestedObject: {
      'd7eb901a-cc21-4f8f-93f4-0408406e6a00': 'a different value'
    }
 },
});

Intended outcome:
I am expecting the imMemoryCache to update the User to the following value:

{
  id: '1',
  nestedObject: {
     'd7eb901a-cc21-4f8f-93f4-0408406e6a00': 'a different value'
  }
}

Actual outcome:
In actuality, the keys for the nested object get messed up (seem to get doubled):

{
  id: '1',
  nestedObject: {
     'd7eb901a-cc21-4f8f-93f4-0408406e6a00-d7eb901a-cc21-4f8f-93f4-0408406e6a00': 'a different value'
  }
}

How to reproduce the issue:
Follow steps above to reproduce

Notes

• This is possible due to the limited characters in the FieldNamePattern variable in cache/inMemory/helpers.ts
• There are a lot more valid JSON key characters that are missing. However, the hyphen was the only one I was running into in this example
• Feature Request Sidenote: The ability to pass in a custom merge algorithm could be useful
• Possible Bug sidenote: Due to the merge algorithm, it seems like it is not possible to reset a field back to undefined. Is this correct, or intended behaviour? And, if intended, is there a way to set a field back to undefined in cache?

Versions
System:
OS: macOS 10.15.2
Binaries:
Node: 12.12.0 - /usr/local/bin/node
Yarn: 1.19.1 - /usr/local/bin/yarn
npm: 6.9.0 - /usr/local/bin/npm
Browsers:
Chrome: 79.0.3945.88
Safari: 13.0.4
npmPackages:
@apollo/client: ^3.0.0-beta.10 => 3.0.0-beta.16
@apollo/react-hooks: ^3.2.0-beta.0 => 3.2.0-beta.0
apollo-link-debounce: ^2.1.0 => 2.1.0

[dylanwulf]

According to the GraphQL Specification, field names cannot have hyphens.
https://graphql.github.io/graphql-spec/June2018/#sec-Names

[jamshally]

@dylanwulf - OK, thanks, I didn't know that. Do you have any feedback on the remaining points points (below)? I can create a separate issue for each if useful, but won't bother if these are also non-issues.

1. Possible Bug: Due to the merge algorithm, it seems like it is not possible to reset a field back to undefined. Is this correct, or intended behaviour? And, if intended, is there a way to set a field back to undefined in cache?
2. Feature Request Candidate: Add option to pass a custom merge algorithm to writeQueryToStore
3. Feature Request Candiate: Throw error when fieldnames do not comply with Graphql Spec. At present, non GraphQl fieldnames can result in subtle and hard to find bugs (as per above example)

Thanks

[dylanwulf]

1: I don't know, sorry. You could create an issue for that, or maybe browse through #5116 to see if you find any useful information.
2 and 3: Feel free to create feature requests on the apollo-feature-requests repo

[benjamn]

You can evict specific fields with

cache.evict(
  cache.identify(object),
  fieldName,
);

but this only works if the object has an ID, so cache.identify(object) returns a string. You might have to resort to evicting the whole nestedObject field from the parent object, instead.

[benjamn]

It's also fine to embed JSON data containing arbitrary keys in your query results. You just can't apply a nested selection set to that data, because that implies the data is a GraphQL object with a __typename and fields with legal names.

This may point to a problem with cache.writeData, which tries to make up fake selection sets for the data you provide. This behavior has always been somewhat dicey, so I would recommend using cache.writeQuery or cache.writeFragment instead.

[jamshally]

@benjamn - Thanks. I guess it is not my day, because I can't seem to get the evict working either, even for the parent object. When I run the following code, I would expect the second cache.readFragment to return undefined, which it doesn't:

  // Evit Routine
  const key = apolloClient.cache.identify(user);

  console.log(apolloClient.cache.readFragment({ id: key, fragment: UserFragment }));
  console.log(`cache.evict success: ${apolloClient.cache.evict(key)}`);
  console.log(apolloClient.cache.readFragment({ id: key, fragment: UserFragment }));

  // Output
  {__typename: "user", id: "1", nestedObject: { 'prop1': 'somevalue' }}
  cache.evict success: true
  {__typename: "user", id: "1", nestedObject: { 'prop1': 'somevalue' }}

Am I missing something obvious?

In case relevant:

• I have tried both removing the whole parent object, and just the nestedObject field, with the same results
• nestedObject is an @client property
• I just need to figure out a way to remove nestedObject from the parent object so that the Resolver can regenerate it on the next query
• I am using the latest version of the Apollo V3 Client (3.0.0-beta.19).

[benjamn]

I would think that code should work, so there might be a bug here. That said, you shouldn't have to evict the whole user object, if you do apolloClient.cache.evict(key, "nestedObject").

Another way to inspect the cache after evicting data is to do console.log(cache.extract())—more data, but maybe somehow different from readFragment?

[jamshally]

I just added console.log(cache.extract()) to the logging. The evicted User entity still shows up both before and after the eviction.

So now I am a bit stuck. I guess my next step is to create a small test project to see if I can reproduce this issue in a way I can share. Do you know of any resources for apollo-client v3 (boilerplate projects or codepens) that would make that rampup quicker?

[benjamn]

@ahrnee I just updated the error template with an AC3-based branch (called ac3): https://github.com/apollographql/react-apollo-error-template/tree/ac3#reproduction-creation-steps

[jamshally]

@benjamn - I haven't managed to recreate evict issue in the error template (yet), but I'll spend a little more time on it. In the meantime I have narrowed it down and also noticed a few of behaviours. I'm going to share now, in case it sheds any more light on the issue. I'll update this thread as/if I get further info:

1. My issue of the cache evict apparently not working (mentioned above) occurs when the evict is run immediatly after a delete mutation (on a different type) that is not awaited. If I await the prior mutation OR don't run the mutation at all, the cache evict seems to work as expected. When I get a chance, I'll continue to try to recreate in my ac3 error template fork. However, I'll need to figure out how to add a mutation in schema.js (which I currently am not familiar with - so any pointers there would help)

2. Even when the cache evict works, the Apollo Client Developer Tools (Chrome Plugin) does not seem to update to remove the evicted field. Not sure if this is a bug or expected behaviour, but it makes for a confusing developer experience

3. When evicting a field on a entity (instead of the entire entity) the evict method will always return false, even when the eviction succeeds

I am aware that this thread has strayed a long way from the original subject. I can create new ticket(s) as needed.

[benjamn]

@ahrnee I think all of these problems can be solved by making the evict method more powerful: #5773

[hwillson]

@ahrnee did #5773 help address things? Do you think this issue can be closed?

[jamshally]

@hwillson - yes, thanks, these are all resolved for me now as of v3.0.0-beta.24