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

Add configurable WeakCache implementation based on FinalizationRegistry #599

Closed
wants to merge 4 commits into from

Conversation

phryneas
Copy link
Contributor

@phryneas phryneas commented Oct 6, 2023

This would be an alternative approach to #132 - and adds a second implementation of Cache that only allows object keys and is based on WeakMap, with automatic cleanup.

I tried integrating it with the original Cache at first, but it

  • made the implementation hard to reason about
  • changed the public api of Cache (there cannot be a guarantee that dispose is called with a key!)
  • felt a lot less intentional - it would end up as a cache that accepted both strong and weak keys, which could invite bugs in downstream code. I prefer this option of a "deliberately weak-only" alternative implementation.

The new WeakCache would be incredibly useful in Apollo Client in multiple places, and could also be plugged in quite a lot of our wrap usages.

This implementation polyfills WeakMap with Map and FinalizationRegistry and WeakRef with dummy functions, so it will work in e.g. React Native, but it won't collect weak values.

I haven't finished tests on cleanup behaviour yet, as I wanted to wait for feedback if this is interesting at all first.

For review purposes, it can make sense to look at commit 1e26a2c independently - the first commit in this PR only copies over the existing Cache, so it adds a lot of clutter.

@phryneas
Copy link
Contributor Author

Superseded by #615 and benjamn/wryware#568

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant