Is there an API to add more tracking observables to the existing reaction?

[th0r]

Hey guys!

I'm currently working on an Angular / MobX integration as already existing library (https://github.com/mobxjs/mobx-angular) is not very efficient and I believe can be improved!

But I've faced one challenge - Angular renders component templates in chunks which means wrapping renderComponentTemplate() into a reaction.track() doesn't work as render functions for some inner parts of component template may be called sometime later.

So, the question is whether it's possible to add tracking context to already existing reaction e.g. something like reaction.addTracking(() => { renderNestedComponentTemplate() })?

Some pseudo-code to better explain what I'm talking about:

let reaction = new Reaction('foo', () => {
    scheduleComponentRerender();
});

reaction.track(() => {
    renderComponentTemplate();
});

// ... sometime later ...

reaction.addTracking(() => {
    // All observables accesses in `renderTemplateNestedIntoComponentTemplate` will be *added* to this reaction's dependencies list
    renderTemplateNestedIntoComponentTemplate();
});

// ... sometime later ...

reaction.addTracking(() => {
    renderAnotherTemplateNestedIntoComponentTemplate();
});

Thanks!

[th0r]

@mweststrate @FredyC @urugator @faassen any comment, guys? If there is not such API is there a chance you may consider to implement it as I think it can be really helpful in situations like mine. I could try to implement it myself if I was sure that's something that can be approved and merged. Thanks!

[urugator]

Afaik there is no such API. Personally I don't know how the impl should/could look like or what problems (if any) to expect. @mweststrate could have a better picture.

I was sure that's something that can be approved and merged

I don't think that's possible to guarantee without knowing:

• what problem (in general) does it solve or who (else) can benefit from it and how ("I believe can be improved" is a bit vague as a selling point)
• are there alternative solutions (reaction per chunk?)
• how complex the implementation is
• does it work reliably, does it break anything, what the limitations are

[th0r]

are there alternative solutions (reaction per chunk?)

This question appeared right after I've failed with an attempt to use a separate reaction per template chunk because of some restrictions: after first render, when all reactions have been set up and some change has occurred in one of the nested templates and corresponding reaction triggers "rerendering" of this template and all of its parent templates (that's how Angular's OnPush change detection strategy works) there is no way to identify corresponding reaction for this nested template tin order to call its reaction.track() again.
Without it I would need to dispose all previous reactions and create them again but in this case all @computed props will drop their cached values so it's not the way to go.

On the other hand if reaction had a way to call track multiple times to watch for more observables I could just create a single reaction object and wrap all nested templates into reaction.addTracking(() => renderNestedTemplate()). In this case after some change and "rerendering" happens I would just wrap rendering of the root template into reaction.track() again (which would reset all previous trackings) and again wrap rendering of all nested templates into reaction.addTracking.

[th0r]

how complex the implementation is
does it work reliably, does it break anything, what the limitations are

I guess it's better to ask someone who is more familiar with the MobX code first.

[urugator]

You would probably need a variant of

mobx/packages/mobx/src/core/derivation.ts

Line 163 in 598f571

export function trackDerivedFunction<T>(derivation: IDerivation, f: () => T, context: any) {

that doesn't call:

mobx/packages/mobx/src/core/derivation.ts

Line 167 in 598f571

changeDependenciesStateTo0(derivation)

which will probably break bindDependencies (?)

mobx/packages/mobx/src/core/derivation.ts

Line 186 in 598f571

bindDependencies(derivation)

mobx/packages/mobx/src/core/derivation.ts

Line 210 in 598f571

function bindDependencies(derivation: IDerivation) {

I am a bit worried you will still need to be able to pair observables with particular addTracking call, dunno.

The individual addTracking calls can be time separated right? I mean it's not just about the need to pass single closure, which could be solved eg by exposing startTracking()/endTracking()...

[th0r]

The individual addTracking calls can be time separated right?

Well, in my case individual calls to addTracking are done within a single JS event loop.

I mean it's not just about the need to pass single closure, which could be solved eg by exposing startTracking()/endTracking()...

Sorry, I'm not sure I understand what startTracking/endTracking methods do you mean - couldn't find them in the sources.

[urugator]

These were just hypotetical, I should use "introducing" instead of "exposing". However I realized it probably wouldn't help much anyway so don't worry about it.

[th0r]

I think I've implemented a working proof-of-concept already. Will create a PR in my fork and would be grateful if you or @mweststrate or someone else who knows MobX core well enough could take a look to identify potential problems. 🙏

[th0r]

Here it is: th0r#1

[mweststrate]

Sorry, didn't look at the PR yet in depth, which might be correct as well, but I think it can be solved in the current API's already, by 'inheriting' deps from a previous reaction, and not disposing before starting new reaction to keep deps hot:

const invalidate =  () => {
    scheduleComponentRerender();
}

let reaction = new Reaction('foo', invalidate);

reaction.track(() => {
    renderComponentTemplate();
});

// ... sometime later ...
const newReaction('foo', invalidate)

newReaction.track(() => {
   // register all old deps (exact API might differ, or maybe you can skip getAtom entirely)
   getAtom(reaction).depencencies.forEach(d => d.reportObserved())
    // All observables accesses in `renderTemplateNestedIntoComponentTemplate` will be *added* to this reaction's dependencies list
    renderTemplateNestedIntoComponentTemplate();
});

// (maybe this needs to happen in above track, not sure)
reaction.dispose() // all orig deps have a second dep now, so should not go cold as a result of this dispose
reaction = newReaction;

[th0r]

Thanks @mweststrate! I tried your approach and if I understood it right here is the corrected version:

// ...

newReaction.track(() => {
  reaction.observing_.forEach(observable => {
    (getAtom(observable) as IAtom).reportObserved();
  };
  renderTemplateNestedIntoComponentTemplate();
});

// ...

But there is at least one problem: getAtom for instance of ComputedValue returns itself, but it neither extends Atom not implements IAtom so it doesn't have reportObserved method and I couldn't find a way to call it as reportObserved function itself is not part of public API.

Actually, return type for getAtom is IDepTreeNode and not IAtom so it's technically correct that it may not always have reportObserved method.

Another concern is a performance of such approach but I haven't measured it yet.

[mweststrate]

probably it should just be observable.reportObserved(). I wouldn't worry about the types atm, but just check whether it works :)

[th0r]

I wouldn't worry about the types atm, but just check whether it works :)

The thing is it doesn't - ComputedValue instance doesn't have reportObserved method.

[urugator]

I think computed.get() should do the trick (since it's already observed it should't recompute)