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

[useResponseCache] Ignore specfic schemaCoordinates for IdFields which are not the root of a subgraph #2247

Open
Kendalor opened this issue Jun 4, 2024 · 7 comments · May be fixed by #2249
Labels
kind/bug Something isn't working stage/0-issue-prerequisites Needs more information before we can start working on it

Comments

@Kendalor
Copy link

Kendalor commented Jun 4, 2024

Is your feature request related to a problem? Please describe.

In our schema we have fields named "id" which are not part of a subgraph, just a nested field named "id". This causes the caching-plugin to treat this nested field like a subgraph, which results in the nested field always beeing null.

Describe the solution you'd like

Add a new optionalList of schemaCoordinates to the useResponseCacheParamters, which is used to exclude fields on specific objects in the mapSchema step. Can provide a pull request.

Alternative solution which we tried (and worked) was renaming all "id" fields which are not a root of a subgraph in our schema. Which sadly is not a solution wie can deploy.

@Kendalor Kendalor changed the title [use-response-cache] Ignore specfic schemaCoordinates for IdFields which are not the root of a subgraph [useResponseCache] Ignore specfic schemaCoordinates for IdFields which are not the root of a subgraph Jun 4, 2024
@EmrysMyrddin
Copy link
Collaborator

Hi! Thank you for reaching out on this!

Can you provide a simple reproduction of this behavior ?

The response cache is using IDs for its automatic cache invalidation and should not impact the other field of the entity.
The idea is that if an entity (with a given ID) is seen in a mutation's response, every query response containing an entity with the same type and ID will be invalidated.

This mechanism works well most of the time, but if some cases doesn't fit this paradigm, you can manually invalidate response by calling invalidate on the cache instance provided to the plugin.

@EmrysMyrddin EmrysMyrddin added kind/bug Something isn't working stage/0-issue-prerequisites Needs more information before we can start working on it labels Jun 4, 2024
@Kendalor
Copy link
Author

Kendalor commented Jun 4, 2024

Hey, thanks for the fast response
will be working on a test case for this behavior but honestly no clue if i can reproduce it easily, will try.

For clarification, this has nothing todo with invalidation. The object is not even cached in this case as ttl = 0.
Object looks like this:
{ "id":"foobar", "header":{ "id":{ "fieldA":"a", "fieldB":"b", "fieldC":"c" } }, "otherFields":"otherField" }

In the Step: #

if (idFields.includes(fieldName) && !idFieldByTypeName.has(typeName)) {
idFieldByTypeName.set(typeName, fieldName);

the field "id" is picked up for the "header" Object. And is therefore resolved as if header is a cached item ( which it isn't, its just common field for all our Objects). This causes the header object which is returned from our data source to be overwritten and returned as null. If the query does not contain the header(id) field, the Header.Id coordinate is not added to 'idFieldByTypeName' all is resolved normally.

@Kendalor
Copy link
Author

Kendalor commented Jun 5, 2024

Update:
Added a UnitTest which reproduces the Issue to a fork:
#https://github.com/Kendalor/envelop/blob/dbd45933a74bce9ae48f2cc1d3c2e4425b12ea9b/packages/plugins/response-cache/test/response-cache.spec.ts#L4098

Hope this helps, the fork has the "fix" for this issue. While coding I encountered the "ignoreTypes" parameter which ignores the caching for given types. I do wan't the object to be cached though.

@ardatan
Copy link
Collaborator

ardatan commented Jun 5, 2024

Would you create a PR?

@Kendalor
Copy link
Author

Kendalor commented Jun 5, 2024

sure, will finish my tests by tomorrow and create the PR

@Kendalor
Copy link
Author

Kendalor commented Jun 6, 2024

created pull request

@Kendalor
Copy link
Author

Pull request provided a reproduction and fix to the Issue. Can I get an update @ardatan ? On what to do next ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working stage/0-issue-prerequisites Needs more information before we can start working on it
Projects
None yet
3 participants