createTransformer leaks memory when run outside a reactive context

[kring]

The docs say:

Always use transformations inside a reaction like @observer or autorun. Transformations will, like any other computed value, fall back to lazy evaluation if not observed by something, which sort of defeats their purpose.

But it's worse than that. A transformer used outside a reactive context will leak memory each time it is called with a new value. The object created by createView will be stored in views for each invocation, but because onBecomeUnobserved will never be called, that view will never be removed. For example, this code causes well over a gigabyte of memory to be allocated:

import { createTransformer } from 'mobx-utils';

const f = createTransformer((a: number) => a);

let sum = 0;
for (let i = 0; i < 10000000; ++i) {
    sum += f(i);
}

console.log(sum);

As the documentation says, using a transformer in this way sort of defeats its purpose. But the same could be said of computed. If your property is meant to be called outside a reactive context, why use a computed observable at all? Just use a regular property. But there are, of course, situations where it's convenient to have a single property that is used both inside and outside reactive contexts, and it's a useful feature of mobx that computed observables, when called outside a reactive context, or no worse than regular property getters. In my opinion, the same should be true of createTransformer.

I'd further argue that the transformer's cleanup function should be called immediately when the function is invoked in a non-reactive context.

[mweststrate]

Any memoization mechanism suffers from a similar faith semantics. While using computed outside reactive context is in principle harmless, createTransformer is not. I think the proper solution to this problem is too simply throw (PR is welcome!). cleanup could work as well, but it means that users might totally not get any benefits from use createTransformer at all, and is probably not what he intended.

[kring]

Thanks @mweststrate. Perhaps whether MobX throws or calls cleanup immediately in this scenario could be controlled by computedRequiresReaction or a perhaps a new createTransformerRequiresReaction?

[kring]

By the way, is it possible for something outside core MobX (because createTransformer is in mobx-utils now) to determine whether it is happening inside a reaction? If I remember correctly from when I looked at this before, it did not appear to be possible. We would need to be able to determine that in order to throw, of course.

[n9]

@mweststrate Should this be implemented by !globalState.inBatch? (If yes, it seems that globalState is internal to mobx.)

[fschwiet]

I think the proper solution to this problem is too simply throw (PR is welcome!). cleanup could work as well, but it means that users might totally not get any benefits from use createTransformer at all, and is probably not what he intended.

I'm using createTransformer for a method that takes a small range of possible arguments. In some cases the result is observed, but not in all cases. The leak is bounded by the input range. So we are getting the effect we intended, an exception would be bad for us, and cleanup would be fine.

[vivainio]

Does computedFn suffer from this?

[mweststrate]

@vivainio, no it shouldn't: as:

By default the output of a function call will only be memoized as long as the output is being observed.

Also, it will print a warning if used outside a reactive context and not memoize at all (unless the keepAlive flag is set, in which case it leaks-by-design)