calmm-js / karet.util Goto Github PK

Most functions under Kefir section return observables no matter what their input is.

It's especially troublesome for very generic type of functions like U.when or U.ifElse which are likely to be used extensively throughout your codebase. They are usually better alternatives to R.when and R.ifElse due to them working with values instead of functions.

I'm doing some optimization for our app, and this has turned out to be one of the biggest peformance issues for us. It's especially problematic for large tables which use generic components like inputs or dropdowns which use the functions grouped under Kefir, causing slowness due to thousands or tens of thousands of Kefir subscribers.

Would it be okay to make a PR so that these functions will not return observables unless needed? Conditionals should be quite straightforward but perhaps some combinators need some extra thinking.

It seems that currently sink and consume only accept Properties.

Consider the following example from @rikutiira:

U.cond([
  [U.equals('test'), () => Kefir.constant('foo')] // <- Note that the result is an observable
])('test').log('primitive') // foo

U.cond([
  [U.equals('test'), () => Kefir.constant('foo')]
])(Kefir.constant('test')).log('observable') // observable

The behavior is currently as originally designed, but going a flatMapLatest inside cond might actually be a useful simplification. The same might apply to other higher-order functions such as ifElse.

Would it make sense for U.seq to lift functions automatically like U.pipe?

Right now this fails:

const value = U.atom(1)

const obsSeq = U.seq(value,
    U.defaultTo(0),
    U.gt(U.__, 0)
) // false

While this works:

const value = U.atom(1)

const obsSeq = U.seq(value,
    U.defaultTo(0),
    U.lift(U.gt(U.__, 0))
) // observable

Alternatively if it's possible, maybe lifted Ramda functions could return lifted functions when used with placeholder values.

Before doing a PR about this feature, I'd like to discuss how curried Kefir.combine should be implemented in Karet Util.

Personally, I believe this would be the best approach:

U.combine(['foo'], value)
U.combinePassive(['bar'], value)

U.seq(value,
    U.combine(['foo']),
    U.combinePassive(['bar'])
)

But I'd like to double-check if this approach is OK with you @polytypic ? It's worth discussing over because:

1. There is no longer a single function you can combine active and passive values with. Personally I think it's not a problem.

2. The bigger issue might be that it's inconsistent with how Kefir.merge is currently implemented as U.parallel, it's not curriable and therefore not directly usable when piping functions like with U.seq. But on the other hand, you can't simply write U.combine([...observables]) with this suggested approach.

Any opinions?

I'm still unsure if the lifted function semantics are good as they are now. As @polytypic mentioned in another issue, lifted functions have following functionality:

• they give properties that skip identical values
• they perform deep lifting of their arguments

My issue is mainly with the automatic skipping of identical values. That mostly works fine for properties but it does not work with streams at all. I understand that the Calmm way to do things is to use properties and avoid side-effects but sometimes streams are necessary. Karet.util even has a bus function for creating streams which you can push values to. I have ran into weird bugs with streams because I'm using karet.util functions and they skip identical values. The latest issue was with my cancel feature of Form: it's a stream which tells to cancel the current state of form, but only the first cancel worked because the stream is always emitting the same value.

I also ran into a weird bug with deep lifting of the arguments but I simply didn't remember that lift was doing that. As long as it's well documented, I feel that's a fine semantic.

But question is: does skipping identical values automatically cause more issues than it solves? At least I'm not personally fan of not being able to use karet.util for streams, even when karet.util has utilities for working with streams.

Current implementation of U.view:

export const view = I_curry((l, xs) => {
  if (xs instanceof AbstractMutable) {
    return template(l) instanceof Observable
      ? new Join(combines(l, l => xs.view(l)))
      : xs.view(l)
  } else {
    return combines(l, xs, L.get)
  }
})

I have an use-case like this:

var template = {
	foo: U.variable(),
	bar: U.atom('bar')
}

var bar = U.view('bar', template)

bar.log()

bar.log() never logs anything because foo doesn't have a value.

Perhaps change the view implementation to take this into consideration? Same issue happens with lifted Ramda R.prop (and probably same applies to other Ramda lens functions).

In case of U.view, perhaps something like this would suffice?

export const view = I_curry((l, xs) => {
  if (xs instanceof AbstractMutable) {
    return template(l) instanceof Observable
      ? new Join(combines(l, l => xs.view(l)))
      : xs.view(l)
  } else if (xs instanceof Observable) {
    return combines(l, xs, L.get)
  } else {
    return combines(l, (l) => L.get(l, xs))
  }
})

Consider:

onClick={U.doSet(U.view(observable, atom), value)}

The above does not currently work, because U.view(observable, atom) is not subscribed to. This should be fixed by making it so that U.doSet (and friends) return an observable that subscribes to the atom (which is required when it is a Join atom).

• Simple destructuring:

  const {foo, bar} = U.destructure(observable) // Using a non-recursive Proxy

• Lazy conditional operations:

  U.and(...booleans) // With lazy left-to-right evaluation!
  U.or(...booleans) // With lazy left-to-right evaluation!
  U.not(boolean)

• Actions with value from event:

  U.doPushEvent(lens, bus) // () => bus.push(L.get(lens, event))
  U.doErrorEvent(lens, bus) // () => bus.error(L.get(lens, event))
  U.doSetEvent(lens, atom) // () => atom.set(L.get(lens, event))

• Allow buses in getProps:

  U.getProps({files: filesBus})

Not to be done:

• Not necessary as actions can be made to perform actions while holding (#30).

  U.doHolding(() => {...}) // () => U.holding(() => {...})