Add support for loading extra operators

[Widdershin]

It would be nice if we could allow users to specify additional operators to be available with @cycle/time, so that users could create their own operators or so that we could ship some operators that are uncommonly used but can be added as needed.

One possible way we could do this is to create a makeTimeDriver function that takes an extras object:

import {makeTimeDriver} from `@cycle/time`;
import tween from `@cycle/time/extra/tween`;

const drivers = {
  Time: makeTimeDriver({extras: {tween}})
}

mockTimeSource could also take this extras object.

An extra would need to be a function that takes in the schedule object (used for adding to the schedule, like a listener), the currentTime() function, and possibly a function to get the time source (to allow building on top of core operators). It would need to return a function.

One thing I am unsure of is how to make this play nicely with TypeScript. Perhaps there is a way to tell TypeScript that there will be extra operators in the TimeSource from the provided extras object but I am unsure.

This is a prerequisite for #13.

[wclr]

I think the whole approach to scheduling and the virtual timing could be simplified. If xstream doesn't want it yet to introduce virtual time ok, then I would do the following:

1. Follow simplified singleton approach that cyclejs prefers (for example with adapt)
2. Create virtual setTimeout which could be imported from import { setTimeout } from '@cycle/time'
3. In normal situation this setTimeout would be a normal setTimeout
4. Create import { setVirtual } from '@cycle/time', when you call setVirtual(), it makes setTimeout to use virtual time.
5. Take all needed operators's code from xstream replace there setTimeout with setTimeout from '@cycle/time' (actually just add import { setTimeout } from '@cycle/time' on top of each file and fix imports of xstream's things.

And use those operators in the manner you like import {delay} from '@cycle/time' or Time.delay.., maybe moved to extra, whatever.

When running test just call setVritual() and all time in @cycle/time operators would become virtual, this would also allow to use this virtual setTimeout singleton in tests for doing imperative timeout stuff.

[Widdershin]

@whitecolor one thing I can see being hard with this approach is how would we support RxJS and most.js?

[wclr]

Well as for Time.delay(..) things may support just via adapt What is the problem?

[wclr]

@staltz what do you think about my comment?

[Widdershin]

@whitecolor your suggestion would probably work, but I'm not sure I like it.

Follow simplified singleton approach that cyclejs prefers (for example with adapt)

I disagree that Cycle prefers a global singleton approach. A clear design principle of Cycle is to use drivers to control side effects and global state. There are two parts of global state in Cycle that I know of. One is setAdapt and the other is the internal counter in isolate.

If we used global singleton for this, why not the same for the DOM driver?

It's also problematic for cycle-restart, which actually swaps out the time driver each time the application is restarted, so there is a period where two co-exist.

I'll grant that it would simplify things a bit and make it easier to build new operators and adapt old ones, but I really, really dislike global state.

[wclr]

There are two parts of global state in Cycle that I know of. One is setAdapt and the other is the internal counter in isolate.

isolate I believe is a different thing it is application logic level. While setAdapt is the framework level thing, it is a singleton in the context of application runtime environment.
I too didn't (don't?) like but. I as applications/service tend to be smaller in scope, it might be ok, global setAdapt would be a rub if you would need/want to use multiple stream libraries in one cycle application/environmental context. And it simplifies things kind of.

So agreeing with setAdapt means agreeing with using only one stream library in cycle application.

I'll grant that it would simplify things a bit and make it easier to build new operators and adapt old ones, but I really, really dislike global state.

This only one part of my suggestion global singletons like setVirtual or setTimeout, agreeing with it means agreeing that you only use one time scheduler in the application - in practice I believe it corresponds well to setAdapt approach and it is too framework level thing.

And this thing could introduce in xstream itself. Agreeing with it would mean that you use only xstream in you app and only one time scheduller real or virtual.

But if xstream doesn't implement scheduling itself yet, another part of my suggestion was no take the implementation of operators from cycle, and not implement them again with some new approach (as they are implemented now in cycle-time) - this would just be less work and less bugs eventually. And returning to global singleton problem - potentially this could be implemented without it, but it would like do setAdapt not a singleton thing - maybe not so needed.

I would really encourage @staltz to think about introducing virtual timing in xstream something like I described. Maybe then special virtual time driver would be not so needed.

For example, for Rxjs application I would prefer standard test/virtual scheduler approach.

[Widdershin]

This only one part of my suggestion global singletons like setVirtual or setTimeout, agreeing with it means agreeing that you only use one time scheduler in the application - in practice I believe it corresponds well to setAdapt approach and it is too framework level thing.

It's also problematic for cycle-restart, which actually swaps out the time driver each time the application is restarted, so there is a period where two co-exist.

There is not always a guarantee that there is one time scheduler in play at once.

this would just be less work and potential bugs

It would be more work, as the time based operators in @cycle/time exist and work. In fact, there are some bugs present in xstream's time based operators that are fixed in @cycle/time, like periodic getting out of sync in background tabs.

I'm not going to say that there are no bugs in the @cycle/time operators, but I have done a lot of testing and they work well. If you know of any defects, please report them.

Personally I am not a fan of the setAdapt approach. It makes some things easier but some things harder, and personally I was fine with using adapters explicitly.

For example, for Rxjs application I would prefer standard test/virtual scheduler approach.

Which means you either need to use global state or inject a scheduler, which has the same tradeoffs as here. Except if you want to change stream libraries you have to rewrite your entire test suite.

I really dislike global state. I think it causes surprising behaviour, makes it harder to compose applications, and setting a further precedent for it might mean we're more likely to use it in future.

[Widdershin]

Having a single virtual scheduler would also make it impossible to parallelize tests as well.

[wclr]

Having a single virtual scheduler would also make it impossible to parallelize tests as well.

Well, yes, but:

1. tests usually are run in different runtime context, for example, I run browser application test in the node, because good cycle tests would use mocked driver sources abstracted from real effects.

2. if you want for example to run tests in the browser in the same window you still can implement it in a different context:

• for example, web/service worker.
• implement using completely separate bundle, in this case, singleton modules will be global NOT for browser window but for module context (unless they explicitly use window/global object).

So As I said this is a limitation that can be taken. And it seems to me quite reasonable to take if it really simplifies dev experience.

And really I wouldn't advise running tests in the same context with running app. And this not popular approach anyway.

[Widdershin]

https://github.com/avajs/ava runs tests concurrently, which would break with a singleton scheduler.

You can also run tests concurrently with mocha.

And this not popular approach anyway.

If it speeds up a large test suite, why wouldn't you do it?

I'm somewhat tired of discussing this. I've thought about it quite a bit and I prefer the tradeoffs of dependency injection over globals.

I've listened to your opinion, and I have taken it into consideration. I want to attempt to implement extras as I originally conceived, and if that doesn't work I will explore alternative avenues.

[wclr]

I don't get why should parallel tests be broken by single virtual sceduller that just abstracts time? What is the difference for scheduler where schedule requests come from one tests or multiple tests? Of course, it may depend on scheduler implementation, but I see currently no problem with implementing a universal scheduler that should not be coupled to a particular cycle app or test running.

[staltz]

I'm late for the discussion, but I just read everything. xstream won't get its own virtual scheduler because @cycle/time is precisely xstream's (upcoming) virtual scheduler. It all comes from the realization that time is an effectful I/O (think of it as querying the system clock inside the computer). So import {delay} from 'xstream/extra/delay'; doesn't make the dependency to effectful I/O explicit. But sources.Time.delay(100) does.

On a slightly different subject, @Widdershin what if extras would work like this?

import whatever from 'my-extra-whatever-operator';

function main (sources) {
  // ...
  const b$ = a$.compose(whatever(sources.Time, 100 /* ms */))
  // ...
  return sinks;
}

That would solve TypeScript usage, since normal extra xstream operators also work like that, and respect TypeScript type inference.

[Widdershin]

I like that @staltz, let's do it!