Open jimbuck opened 3 years ago
So I've tried to implement local work arounds for this, but I've found other areas where changes will need to be made in order to support non-string data. For example, the Persistor
class can't purge when the maxSize
is reached since the size is calculated again manually (rather than calling getSize()
). I will try to put a PR together to address all of this, but not sure about timeline.
@jimbuck As you have mentioned this could be a tricky change. For the moment it is easy to say that for serialize false limits will not be enforced. Code we have in repo is not the best as typically we should have storage limits enforced and event that handles storage limit and wipes it out. Calculating size is very heavy operation and totally redundant when indexed db database have native methods to enforce size and handle overflows.
That being said. I think for advanced cases we can recommend to avoid using internal method and implement your own.
@jimbuck any thoughts on the previous comment? If you’ve gone a different direction or have found a solution I’ll close this issue.
We did come up with a work-around but it just replaces some methods on the persistor and storage instances (this is from 0.10.0):
this.persistor = new CachePersistor({
cache: this.cache,
storage: this.localStorage,
serialize: false,
maxSize: 4.5 * MEGABYTES,
debounce: 5 * SECONDS,
});
this.persistor.storage.getSize = async () => {
return this.localStorage.getSize();
};
// Directly override the persist method to use the getSize method on our localStorage service.
this.persistor.persistor.persist = async () => {
try {
if (this.persistor.persistor.paused) return;
const data = this.cache.extract();
const size = await this.localStorage.getSize();
if (size > (this.persistor.persistor.maxSize ?? 0)) {
console.log(`Purged cache of size ${size} characters`);
await this.persistor.purge();
this.persistor.persistor.paused = true;
return;
}
await this.persistor.storage.write(data);
console.log(`Persisted cache of size ${size} characters`);
} catch (error) {
console.error('Error persisting cache', error);
throw error;
}
};
this.localStorage
is a custom class that includes a dedicated getSize
function. It is technically checking the current size of the persisted data, rather than the size of the data to be persisted. We've accounted for this by just lowering the maxSize
so it that it still saves but will get purged on the next persist (4.5MB
limit when local storage is typically 5MB
).
I am trying to put together a PR that will show it more officially implemented. It should be up soon as a better reference.
When I created our work-around, that was in 0.10.0
, which didn't have the persistenceMapper
. I will have to rethink our approach to the maxSize
check since it kinda changed how maxSize
works at a fundamental level. But the draft PR I created will at least let CachePersistor#getSize
work correctly for custom storage instances with a getSize
function.
I'm utilizing a custom JSON serialization configuration so I've set
serialize
tofalse
and supplied my ownstorage
instance. Unfortunately the built-inStorage
class is written to expect a JSON string for calculating size. That means it always returnsnull
if the result is not a string. Which means the auto-purge doesn't work correctly.https://github.com/apollographql/apollo-cache-persist/blob/7bcb322a32aafb088afcf2c21c1ebe793409d8e3/src/Storage.ts#L30-L38
It would make sense to let storage implementations provide their own
getSize
function, but I figured I would file this and get some feedback before starting on a PR.I'm thinking something simple like this:
It's optional, backwards compatible, and gives full control over how size is calculated. Any thoughts?