Do not use global cache for createCache() to avoid memory leaks #3110
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What:
Currently
createCache()
seems to use a global cache that is reused even if you create multiple caches. In an SSR scenario this means that even if you follow the recommendations to create a cache per render you will end up having a shared cache.Also currently an accessor function is returned from
getServerStylisCache
But later it is just used as if it would be an object
Why:
I think the current approach can cause memory-leaks in bigger applications. Since we additionally just use the
serverStylisCache
as an object, I think the cached values will never be garbage collected. This causes issues in apps that have a lot of css and/or use a lot of dynamic styles.How:
I want to change the return type of
getServerStylisCache
to a simple cache object. This is how it is already used. We could also try to use theaccessor
function. But I wouldn't know how to write to the cache then.Also I want to move the call to
getServerStylisCache
inside the function body ofcreateCache
. This way we will get a new instance of the cache for every request.I'm not sure how to add tests for this. And I'm also not sure if my assessment of what the current code is doing is correct. Maybe someone can help to shed some light on it.
Checklist: