-
-
Notifications
You must be signed in to change notification settings - Fork 124
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
keepAlive timeout? #51
Comments
Separately, but related; would it be better if applying keepAlive to a computed does not cause it to be resolved immediately, rather it only kicks in when it first becomes observed? |
Workaround solving both problems: const Mobx = require("mobx");
const MobxUtils = require("mobx-utils");
class Computed {
constructor(fn, disposeAfter) {
this._computed = Mobx.computed(fn);
this._disposeAfter = disposeAfter;
this._timeout = null;
this._atom = new Mobx.Atom("TimeoutedKeepAlive",
() => this._onObserved(),
() => this._onUnobserved()
);
}
_onObserved() {
console.log("observed");
// Prevent possibly scheduled disposer
if (this._timeout) {
clearTimeout(this._timeout);
}
// Establish keep alive with first observer
this._disposeKeepAlive = MobxUtils.keepAlive(this._computed);
}
_onUnobserved() {
console.log("unobserved");
// Set dispose timeout
this._timeout = setTimeout(() => {
console.log("disposing keepAlive");
this._disposeKeepAlive();
this._timeout = null;
}, this._disposeAfter);
}
get() {
this._atom.reportObserved();
return this._computed.get();
}
}
// Test
const computed = new Computed(() => {
console.log("computing");
return "value";
}, 3000);
const dispose = Mobx.autorun(() => {
console.log(computed.get());
});
dispose(); You could probably monkey patch existing computed instance by modifing the
agree |
I think it is trivial to fix this in userland code, so not really needed in this library? function keepAliveWithTimeout(thing, prop, timeout) {
const disposer = keepAlive(thing, prop)
setTimeout(disposer, timeout)
return disposer
} |
@mweststrate He doesn't want to simply dispose keepAlive after a timeout, he wants to dispose keepAlive when the computed is not being observed for a certain amount of time. |
Just repeating what @urugator said. I think I need the hooks mentioned in #992 in order to be able to accomplish something like this without having to start poking around in the MobX internals. Also, this somewhat related comment is perhaps still relevant:
It'd be tempted to have a go at both at once, i.e.: @computed.keepAlive({ timeout: 5000 }) x; But again, I'm not sure the hooks are there for even the basics at present. |
|
There is one feature of 'keepAlive' timeout that is quite tricky to implement. That is 'lazy computation'. Eg. if computed is not observed, I want to keep it alive, but only until it is invalidated. Then, I don't want to re-calculate it instantly as the value is not needed. Current keepAlive is quite like 'autoRun' wrapper which means heavy computed are always 'fresh' even if their value is not needed. |
My implementation of it (quite far from 'trivial'): import { IComputedValueOptions, Reaction, createAtom } from "mobx";
// like setInterval, but batched - if we'll have 1000s of computed values - we'll still create 1 interval.
import { createSharedInterval, sharedDefer } from "./sharedDefer";
export type CachedComputed<T> = {
get(): T;
dispose(): void;
};
export const CACHED_COMPUTED_ALIVE_TIME = 15 * 1000;
/**
* Normally we keep not used computed alive for KEEP_ALIVE_TIME_AFTER_UNOBSERVED time.
*
* Very often, however - we have 'cascade' of computed's one using another. One being disposed instantly causes other one
* to not be observed. In such case we don't want to wait KEEP_ALIVE_TIME_AFTER_UNOBSERVED for each 'cascade' level.
*
* Thus we'll set this flag to true when disposing lazy computed to avoid waiting if other computed becomes unobserved during cascade.
*/
let isDisposalCascadeRunning = false;
export interface CachedComputedOptions<T> extends IComputedValueOptions<T> {
debugId?: string;
onDisposed?: () => void;
}
const sharedDisopseInterval = createSharedInterval(CACHED_COMPUTED_ALIVE_TIME);
/**
* This is computed that connect advantages of both 'keepAlive' true and false of normal computed:
*
* - we keep cached version even if it is not observed
* - we keep this lazy meaning value is never re-computed if not requested
*
* It provided 'dispose' method, but will also dispose itself automatically if not used for longer than KEEP_ALIVE_TIME_AFTER_UNOBSERVED
*/
export function warmComputed<T>(getter: () => T, options: CachedComputedOptions<T> = {}): CachedComputed<T> {
const { name = "CachedComputed", equals, debugId, onDisposed } = options;
// return computed(getter, options);
let latestValue: T;
let needsRecomputing = true;
let currentReaction: Reaction | null;
const updateSignal = createAtom(
name,
() => {
sharedDisopseInterval.remove(dispose);
},
handleBecameUnobserved
);
function handleBecameUnobserved() {
// It became unobserved as result of other lazyComputed disposing. We don't need to wait for 'keep alive' time
if (isDisposalCascadeRunning) {
// Use timeout to avoid max-call-stack in case of very long computed>computed dependencies chains
sharedDefer(dispose);
return;
}
sharedDisopseInterval.add(dispose);
}
// Will initialize reaction to watch that dependencies changed or re-use previous reaction if the same computed used multiple times
function getOrCreateReaction() {
if (currentReaction) {
return currentReaction;
}
currentReaction = new Reaction(name, () => {
// Dependencies it is tracking got outdated.
// Set flag so on next value request we'll do full re-compute
needsRecomputing = true;
// Make observers re-run
updateSignal.reportChanged();
});
return currentReaction;
}
function dispose() {
try {
sharedDisopseInterval.remove(dispose);
// If other computed values become unobserved as result of this one being disposed - let them know so they instantly dispose in cascade
isDisposalCascadeRunning = true;
// It was already disposed
if (!currentReaction) {
return;
}
needsRecomputing = true;
currentReaction.dispose();
currentReaction = null;
onDisposed?.();
} finally {
isDisposalCascadeRunning = false;
}
}
let currentComputingError: null | unknown = null;
const recomputeValueIfNeeded = () => {
// No dependencies did change since we last computed.
if (!needsRecomputing) {
return;
}
let newValue: T;
// We need to re-compute
getOrCreateReaction().track(() => {
// Assign new value so it can be reused. Also we're tracking getting it so reaction knows if dependencies got outdated
try {
newValue = getter();
currentComputingError = null;
} catch (error) {
currentComputingError = error;
}
});
// Inform value is up to date
needsRecomputing = false;
if (!equals) {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
latestValue = newValue!;
return;
}
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
if (!equals(latestValue, newValue!)) {
latestValue = newValue!;
}
};
return {
dispose,
get() {
// This is final 'getter'
// If value is outdated - recompute it now (on demand - in lazy way)
const isObserved = updateSignal.reportObserved();
// ! This value is used, but outside of mobx observers world. Dont initiate reactions to avoid memory leak - simply return value without cache.
if (!isObserved) {
// We already have value - no need to re-compute it even tho it is not observed context
if (!needsRecomputing) {
return latestValue;
}
return getter();
}
recomputeValueIfNeeded();
if (currentComputingError !== null) {
throw currentComputingError;
}
return latestValue;
},
};
} |
What mobx version do you use? This was changed in 5.6.0, it's not recalculating until there is a "real" consumer: |
I've started using
keepAlive
to get around an issue where my observable objects become momentarily unobserved, and avoid complex computations (including those that result in network requests) re-running. Sometimes this can be between frames, other times I might want something to hang onto the computed value for a longer time.So does a
keepAlive
timeout make sense? I was going to have a go at this, but I don't know how I can hook into the 'I'm now the last observer, I'll drop my observer in X seconds' situation so I thought I'd float the idea first.The text was updated successfully, but these errors were encountered: