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

ComputedFn (and createTransformer) can leak memory in certain scenarios #319

Open
realyze opened this issue Jun 15, 2023 · 1 comment
Open

Comments

@realyze
Copy link

realyze commented Jun 15, 2023

computedFn uses _isComputingDerivation() to determine whether it should cache or not. However, when getting value of a computed from within an action, _isComputingDerivation() will return true and computedFn will cache the arguments it's called with. But since we're in an action, onBecomeUnobserved will never get called to evict the arguments from the cache.

Here's the smallest reproducible scenario I can come up with: https://codesandbox.io/s/computedfn-memory-leak-5xvsg6

I think it's similar to #116 (which has been mostly—but apparently not completely—fixed by #228).

Should there be a more reliable way than _isComputingDerivation() to determine whether we're running inside an action?

@urugator
Copy link
Contributor

urugator commented Jul 8, 2023

But since we're in an action, onBecomeUnobserved will never get called to evict the arguments from the cache.

Could also relate to this mobxjs/mobx#2309 (comment)

determine whether we're running inside an action?

I think you can (and probably should) do it the other way around - by checking you don't run in Reaction, in which case globalState.trackingContext === null IIRC.

Or you may consider caching the value for the duration of the batch, like computed does, but I don't know how you would go about that.

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

No branches or pull requests

2 participants