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

Added custom objectCache root in inMemoryCache #5601

Closed

Conversation

morrys
Copy link
Contributor

@morrys morrys commented Nov 19, 2019

Hello everybody,
I would like to take advantage of release 3 to propose this PR that implements a feature that I requested here: apollographql/apollo-feature-requests#154

This change allows you to use a custom object cache in EntityCache.Root.

My main use is in the management of the apollo cache persistence and the management of the offline application: morrys/wora#25

But it can also be used to monitor Apollo's cache.

In the file inMemoryCache.ts the main change is this: https://github.com/apollographql/apollo-client/compare/release-3.0...morrys:154-v3-custom-object-cache?expand=1#diff-add1f566156c3d8d297056b5439e0f01R72

The PR is in draft because I would like to discuss it with you ( @hwillson, @benjamn ).

Thanks,
Lorenzo

@benjamn
Copy link
Member

benjamn commented Nov 20, 2019

Thanks for following the release and taking the initiative to propose and implement this change!

To be completely honest with you, the NormalizedCache interface methods (get, set, etc.) are still very much subject to change before we finalize 3.0, so it might be a good idea to wait until later in the release process to discuss this kind of change.

For example, we are considering changing resultCaching dependencies to use the entity ID plus a field name, rather than just the ID, so the get signature may change from get(dataId: string): StoreObject to get(dataId: string, fieldName: string): StoreValue.

Also, if you're envisioning persisting every individual read or write to IndexedDB or some similar persistent storage API, I would just note that the latency for those APIs is pretty awful (5-10ms+ for single reads), so you should really prefer to write the whole cache.toObject() state at once, periodically. Hopefully this is not news to you, but I think it's important to be aware of this unfortunate reality.

@morrys
Copy link
Contributor Author

morrys commented Nov 20, 2019

Hi @benjamn, thanks for the quick reply.

I proposed a PR in draft because I imagined that cache management was still evolving.

You're absolutely right about the latency in interfacing with storage. This was the reason why my cache library handles reading from storage only when the application starts for state reconciliation... Gets are performed on an inmemory object, and writes are managed through the combination of queue, throttle, merge and write through multiSet & multiRemove.

Obviously in react-native contexts it is remarkably efficient (they implement multiSet / multiRemove), in web contexts the throttle value must be correctly managed (for now).

In the next versions of the library I wanted to give the opportunity to manage the storage writing in a single key (similar to cache.toObject()).

Finally, it can also be used as a simple InMemory cache using the disablePersist parameter.

For a perfect integration with Apollo it would be useful to manage all the data that guarantee the consistency of the cache, the cache data and the references to the persisted queries (rootIds), through an object like objectCache and its possible configuration.

@morrys
Copy link
Contributor Author

morrys commented Nov 22, 2019

so it might be a good idea to wait until later in the release process to discuss this kind of change.

I agree, the PR has been used to inform you of this need and to give my availability to implement it.

To ask for some information do you recommend opening an issue or is there an alternative channel for the v3 release?

Anyway, I created two example projects (one with nextjs ssr & one simple web app) to test apollo-client 3.0 and integration with my libraries so If you have any doubt where I can help, let me know

@morrys
Copy link
Contributor Author

morrys commented Dec 6, 2019

It would be interesting to extend this discussion with the apollo-cache-persist roadmap.

In addition to the possibility of configuring different storage, there are other configurable properties that are often useful:

  • mutateKeys: allows you to filter / modify the keys when reading / writing to storage
  • mutateValues: allows to filter / modify the values during the reading / writing in the storage
  • initialState: allows you to correctly manage the SSR
  • mergeState: asynchronous callback that allows to manage the state reconciliation between the initial state and the persisted state

@hwillson hwillson changed the base branch from release-3.0 to master January 7, 2020 17:48
@codecov
Copy link

codecov bot commented Apr 6, 2020

Codecov Report

Merging #5601 into master will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5601      +/-   ##
==========================================
+ Coverage   95.37%   95.39%   +0.02%     
==========================================
  Files          88       89       +1     
  Lines        3653     3671      +18     
  Branches      903      905       +2     
==========================================
+ Hits         3484     3502      +18     
  Misses        146      146              
  Partials       23       23              
Impacted Files Coverage Δ
src/cache/inmemory/inMemoryCache.ts 100.00% <ø> (ø)
src/cache/inmemory/entityStore.ts 99.53% <100.00%> (+<0.01%) ⬆️
src/cache/inmemory/objectCache.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0aacba4...0508548. Read the comment docs.

@hwillson hwillson modified the milestones: Release 3.0, Post 3.0 Jul 10, 2020
@austinamorusocfc
Copy link

Eta?

@morrys
Copy link
Contributor Author

morrys commented Oct 14, 2020

Hi @benjamn,
as for this PR in draft, now that version 3.x has been released, could you point me to some documentation that will help me better understand the new cache management?

Also, what about apollo-cache-persist do you plan to upgrade it to support apollo v3?

@wtrocki
Copy link
Contributor

wtrocki commented Oct 14, 2020

Also, what about apollo-cache-persist do you plan to upgrade it to support apollo v3?

Apollo-cache-persist is working with Apollo v3 already. We needed to push different package as the community did not have access to the v2 package

Big topic is now support for reactive variables:
apollographql/apollo-cache-persist#361

Looking to some opinions from the core team on it as not sure if makes sense to persist them.

@benjamn
Copy link
Member

benjamn commented Oct 15, 2020

@morrys Make sure you're using the new apollo3-cache-persist package, which has been updated to work with AC3.

@wtrocki
Copy link
Contributor

wtrocki commented Oct 15, 2020

@benjamn do you still plan to include persistence at some point into Apollo-Client. Any cons about adapting persistence into the core?

@morrys
Copy link
Contributor Author

morrys commented Oct 19, 2020

hi @benjamn,
I have updated the PR and everything seems to work correctly, I would have some internal technical questions to ask to better understand the new cache management.

I noticed that in the sources the retain function is called https://github.com/apollographql/apollo-client/blob/release-3.3/src/cache/inmemory/writeToStore.ts#L114 but not the release function nor the function gc. Is this the intended behavior or is it evolving?

In summary, should the management of retain, release, evict and gc be implemented in the application without any automatisms present in the library?

Thanks

@hwillson hwillson removed this from the Post 3.0 milestone Aug 10, 2022
@jerelmiller
Copy link
Member

Hey all 👋! I'm doing a bit of housekeeping on some of our older PRs/issues and noticed this PR. Since we are now well into 3.0 and this is 3 years old, I'm going to close this. Thanks for the discussion and ideas!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 1, 2023
@apollographql apollographql unlocked this conversation Feb 2, 2023
@apollographql apollographql locked and limited conversation to collaborators Feb 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants