n1ru4l / envelop

Envelop is a lightweight library allowing developers to easily develop, share, collaborate and extend their GraphQL execution layer. Envelop is the missing GraphQL plugin system.
https://envelop.dev
MIT License
771 stars 119 forks source link

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

Open Kendalor opened 1 month ago

Kendalor commented 1 month ago

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.

EmrysMyrddin commented 1 month ago

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.

Kendalor commented 1 month ago

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: #https://github.com/n1ru4l/envelop/blob/de4d13b7fb95b2a6c0f96126c1fd4de210284443/packages/plugins/response-cache/src/plugin.ts#L364-L365

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 commented 1 month ago

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 commented 1 month ago

Would you create a PR?

Kendalor commented 1 month ago

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

Kendalor commented 1 month ago

created pull request

Kendalor commented 1 month ago

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