Suggestion: Trackers through arguments

[voliva]

This is a cool and fun idea indeed. Thanks for this!

I'd like to suggest an alternative API that maybe would make more explicit what $ and _ do, and make the implementation slightly easier.

Instead of importing $ and _ from the package, what if these two trackers could be passed as arguments to run? So instead of:

import { run, $, _ } from 'rxjs-autorun';

const a = new BehaviorSubject('#');
const b = new BehaviorSubject(1);
const c = run(() => _(a) + $(b));

You could:

import { run } from 'rxjs-autorun';

const a = new BehaviorSubject('#');
const b = new BehaviorSubject(1);
const c = run((track, read) => read(a) + track(b));

This would also let the consumer name $ and _ as they want (in this case, track and read respectively), but they could also go with a smaller t and r.

[kosich]

Hey, Víctor! You've actively participated in this, so big thanks goes to you 🙂 🙌

Yeah! Actually this was my initial API: run($ => $(o)) and I think it was nice and conscious (user wouldn't try to call those functions outside run). Though particular use-case for Ryan, as I understood, required those track/read functions to be available outside the run for integration purposes.

And it makes sense if you want to integrate this with, say, a custom state Observable, e.g.:

// pseudocode
// state.js
import { $, _ } from 'rxjs-autorun';

export const State = v => {
  const subj = new BehaviorSubject(v);
  return Object.assign(subj, { get _: _.bind(null, subj), get $: $.bind(null, subj) });
  // not sure it'l work, but you got the idea
}

// index.js
import { run } from 'rxjs-autorun';
import { State } from './state';

const state = State(0);
run(() => state.$ + ' 🐑'); // > 0 🐑
state.next(1); // > 1 🐑

Which wouldn't be possible with run($ => $(o)).

But maybe we could have both options available for the user? Would that be fine or confusing?

[voliva]

Ahh, that makes sense, thank you for explaining.

But maybe we could have both options available for the user? Would that be fine or confusing?

Although I guess it wouldn't hurt (as users can just ignore one of both ways), it's not really needed. If anyone needs to use the suggested overload, it's fairly trivial to write a util function that exposes that API.

I'll close this issue - feel free to reopen it if you think it's worth keeping it in.

[kosich]

I think, you're right that it's a good alternative and it wouldn't hurt
Let's keep this issue open, maybe actual use-cases (ha-ha!) will advice us how to act 🤔
Truly, it's a one-line fix 🙂

Also, I guess, there's a side-point that $ and _ are not the best names for those functions, will create a separate issue to rename/add aliases.

[Jopie64]

I like to add my 2ct here as well.
It could be argued that another downside of passing $ and _ as arguments of the callback function could be that when you do a nested run, $ and _ would be shadowed, and the compiler would warn you. But I consider this a minor thing because:

1. I think nested runs are not gonna be used that often because I expect that would lead to awful expressions...
2. You can easily mitigate that by using different names in the nested run (e.g. $2 and _2)

But an upside is that you don't have to import $ and _. $ is still used for jquery a lot, and _ is often by convention used as unused argument.

So I would very much argue in favor of passing $ and _ as arguments as well.

[loreanvictor]

Follow up to this discussion, I would also like to argue in favor of $ and _ being passed as arguments:

• In case of provided example I am a bit confused as to the benefit here, since $ => $(state) + ' 🐑' would achieve the same thing. So I am particularly unsure of the general situation that such integrations would be useful and how they would become a serious hurdle if $ and _ were passed as arguments.

• As mentioned on the reddit discussion, I believe not having an import side-effect (i.e. the context which is the basis of global $ and _) is much more tree-shaker friendly. I am unsure of current state of tree-shakers and how well they can handle Ok side-effects such as this.

• Ergonomically, the ability to seamlessly rename would also be pretty cool. Imagine this:

() => `${${a}} 🐑`

vs this:

_ => `${_(a)} 🐑`

(although the best option might be an added feature like this:

() => rx`${a} 🐑`

)

• Also I am personally in favor of strict separation of responsibilities and code simplicity as that has a drastic effect on long term maintainability of the code (lower maintenance cost basically). Providing $ and _ actually do add considerable complexity to the code-base, so I generally would recommend taking it out unless there are common use-cases that do indeed fall in the scope of responsibilities of this util and are pretty inconvenient without this.

[kosich]

I want to try some integrations to see what are the limits of the two approaches. E.g. I want to try autorun with rxjs-proxify or a simple state based on it. Unlike pipe(pluck('prop')), both have stable access to subproperties. Maybe something like:

const state = new State({ a: 1, b: { c: 2 } });
computed(() => state.a._ + state.b.c.$); // > 3
state.b.next({ c: 3 }); // > 4

Also, a.$ + b._ notation to me looks a bit less noisy than $(a) + _(b).

--

() => rx${a} 🐑

Nice! Maybe it's a candidate for an API. I also wonder if can we achieve this via external code now and would we be able to do that with $_ as arguments.

Btw, @voliva did an interesting string template-based approach

--

Now, back to the issue: currently, I see this change as not critical (unless it forbids tree shaking)
And therefore would like to do more research on integrations and explore the API (#3), as those might give us a better understanding of this feature

And keep sharing your thoughts and findings!
Also, if you still consider this to be a critical issue that needs urgent resolution — do elaborate on this

Cheers

[kosich]

So, I've been typing-thinking. Guess, if we pass params as args, we can still achieve the separation:

Warning: pseudocodish

// shared.ts
import * as rxjsAutorun from 'rxjs-autorun';

export function unwrapComputed(){
    const c = { _(){ throw 'ERR'; }, $(){ throw 'ERR'; } };

    return [ o => c.$(o)
           , o => c._(o)
           , fn => rxjsAutorun.computed(($,_) => c.$ = $, c._ = _, fn())
           ]
}

So that we can use it in 3rd party API:

// state.ts
import { unwrapComputed } from './shared';

const [$,_, computed] = unwrapComputed();
const State = v => {
  const subj = new BehaviorSubject(v);
  return Object.assign(subj, { get _: _.bind(null, subj), get $: $.bind(null, subj) });
  // not sure it'l work, but you got the idea
}

export { State, computed };  // < NOTE exporting computed from state

And then:

// index.js
import { State, computed } from './state';

const state = State(0);
computed(() => state.$ + ' 🐑'); // > 0 🐑
state.next(1); // > 1 🐑

But I suspect that in this case we are locked to computed only from this API: in index.js computed from rxjs-autorun won't work. Neither would other 3rd party integrations if we have such.

Am I right in this assumption? What do you guys think?

[loreanvictor]

How about providing the$ and _ as arguments to computed() alongside making them available globally? This way we would have some of the good parts of both worlds:

• Easy renaming
• Async invocation of $ and _
• Still supporting the state.$ syntax (though with the added complexity that async invocation wouldn't work here)

[kosich]

This is definitely an option! I'm holding this because ($,_) => notation might limit us with alternative APIs we're exploring and this duality might confuse users. As we discussed, async invocation is doubtful, so I think we can deprioritize it.

Since the current approach works fine now, I'd like to postpone this decision until we got more practical cases.
Unless we discover it's critical, surely.