Skip to content
This repository has been archived by the owner on Dec 18, 2019. It is now read-only.

Support Cycle Unified #18

Merged
merged 4 commits into from
Apr 11, 2017
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 16 additions & 12 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {DevToolEnabledSource} from '@cycle/run';
import xs, {Stream, MemoryStream} from 'xstream';
import dropRepeats from 'xstream/extra/dropRepeats';
import { adapt } from '@cycle/run/lib/adapt';

export type MainFn<So, Si> = (sources: So) => Si;
export type Reducer<T> = (state: T | undefined) => T | undefined;
Expand All @@ -14,23 +15,25 @@ export type Lens<T, R> = {
}
export type Scope<T, R> = string | number | Lens<T, R>;

export function pick(selector: Selector | string) {
export function pick<T>(selector: Selector | string) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @goodmind, thanks for the PR.
Could you revert the changes in this part? I know it makes the typings generic for any stream library, but I don't want to make these typings less useful for xstream users. I think the approach should be like in Cycle DOM or Cycle HTTP, where RxJS users import rxjs-typings.d.ts. If that's not possible, then RxJS and most.js users should just use any casting.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can use here generic defaults microsoft/TypeScript#13487

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be even better if TypeScript supports Higher Kinded Types

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can when TypeScript 2.3 is out. https://github.com/Microsoft/TypeScript/milestone/38
We can either revert these changes, and later update to 2.3, or wait until 2.3 is out.

if (typeof selector === 'string') {
return function pickWithString(sinksArray$: Stream<Array<any>>): Stream<Array<any>> {
return sinksArray$.map(sinksArray => sinksArray.map(sinks => sinks[selector]));
return function pickWithString(sinksArray$: any): T {
return adapt((xs.fromObservable(sinksArray$) as Stream<Array<any>>)
.map(sinksArray => sinksArray.map(sinks => sinks[selector])));
};
} else {
return function pickWithFunction(sinksArray$: Stream<Array<any>>): Stream<Array<any>> {
return sinksArray$.map(sinksArray => sinksArray.map(selector));
return function pickWithFunction(sinksArray$: any): T {
return adapt((xs.fromObservable(sinksArray$) as Stream<Array<any>>)
.map(sinksArray => sinksArray.map(selector)));
};
}
}

export function mix(aggregator: Aggregator) {
return function mixOperator(streamArray$: Stream<Array<Stream<any>>>): Stream<any> {
return streamArray$
export function mix<T>(aggregator: Aggregator) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

return function mixOperator(streamArray$: any): T {
return adapt((xs.fromObservable(streamArray$) as Stream<Array<Stream<any>>>)
.map(streamArray => aggregator(...streamArray))
.flatten();
.flatten());
}
}

Expand Down Expand Up @@ -105,7 +108,7 @@ export class StateSource<T> {

constructor(stream: Stream<any>, name: string | null) {
this._name = name;
this.state$ = stream.compose(dropRepeats()).remember();
this.state$ = adapt(stream.compose(dropRepeats()).remember());
if (!name) {
return;
}
Expand All @@ -115,7 +118,7 @@ export class StateSource<T> {
public select<R>(scope: Scope<T, R>): StateSource<R> {
const get = makeGetter(scope);
return new StateSource<R>(
this.state$.map(get).filter(s => typeof s !== 'undefined'),
xs.fromObservable<T>(this.state$).map(get).filter(s => typeof s !== 'undefined'),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can avoid this one since RxJS/xstream/most.js all have map and filter. It should save us something in performance.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to #26
This stream passed to constructor where compose called on it

null,
);
}
Expand All @@ -134,7 +137,8 @@ export default function onionify<So, Si>(
.drop(1);
sources[name] = new StateSource<any>(state$, name) as any;
const sinks = main(sources as So);
reducerMimic$.imitate(sinks[name]);
const imitation$ = sinks[name] && xs.fromObservable(sinks[name]);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why this line is needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because without it tests failing, there is two tests with undefined sink and when we do xs.fromObservable(undefined) it fails

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. Can we use a traditional if ( ) { block here?

reducerMimic$.imitate(imitation$ as Stream<Reducer<any>>);
return sinks;
}
}