distolma / storeon-observable

Module for storeon which allows to create async actions
MIT License
5 stars 0 forks source link

Is this project still alive ? #8

Closed majo44 closed 4 years ago

majo44 commented 4 years ago

Hi, I see few things missing, but I'm not sure you want to continue this project. The first missing part is that we are not able to pick and return the event payload to/from epic. The second is typescript types which are not fallows the last updates within the storeon itself, and they are leaky in the meaning that event types and event payload types are not checked.

I do not want to start new project to do not create mess in storeon community, so if this project is still alive I can create some PR's to fix mentioned issues.

I even checked the possibility to solve first issue and it can look like:

import { merge, BehaviorSubject, Subject  }  from 'rxjs';
import { switchMap, filter, map }  from 'rxjs/operators';

export const combineEpics = (epics) => () => {
  var args = [].slice.call(arguments)
  return merge.apply(null, epics.map((epic) => epic.apply(null, args)));
};

export const createEpicMiddleware = (epics) => {
  var epic$ = new BehaviorSubject(epics)
  return (store) => {
    const action$ = new Subject()
    const state$ = new Subject()
    store.on('@dispatch', (_, event) => action$.next.call(action$, event));
    store.on('@changed', state => state$.next(state));
    epic$.pipe(
      switchMap(epic => epic(action$, state$))
    ).subscribe(ev => store.dispatch(ev[0], ev[1]));
  }
}

export const ofEvent = () => {
  var events = [].slice.call(arguments)
  return (source) =>
    source.pipe(
      filter((event) => events.indexOf(event[0]) !== -1),
      map((event) => event[1]));
}

Where:

Best regards

distolma commented 4 years ago

Hi, thanks for your interest. This project did not freeze and I didn’t abandon it. I will appreciate if you can make some improvements. In the nearest future, I am planning to make this project part of @storeon project.

majo44 commented 4 years ago

Ok so I have a design question?

I comes to conclusion that switching the type of stream within ofEvent operator is not the best idea as it can make confusions and at the end you have to back to the stream type and payload, so the events observable have to deliver both event type and event payload, so the EventsObservable will be a Observable<StoreonEvent>.

The question is about the shape of StoreonEvent.

Option 1: is a tuple (array of two) so StoreonEvent = [type, payload] like in my previous comment. This option is not so bad as on operators we can destruct tuple eg map(([type, data]) => ..., what looks nice but in case of typescript usage type of type and data will be not inferred (this is probably bug in ts as in case of map( x => x[0] .... type of x[0] and x[1] will be inferred properly). The cons of this option is that you have to back to the stream array again so the code like map( x => [INCREMENT, 2]) do not look nice, but we can create helper like toEvent = (t, p) => [t, p] so the map code will look nicer map( x => toEvent(INCREMENT, 2)), such helper will also helps with type checking on typescript.

Option 2: is a structure similar like in redux so the StoreonEvent = {type, payload}, one of the pros of this approach will be super easy migration from redux as the structure will be exactly same as in redux-observables Actions. Destruction is easy and type safe in this case, and we can provide also toEvent helper to avoid action/event factories, and have full type safe code. Similarities to redux can be also counted as cons, as storeon should be different :).

I have a typescript implementation of option 1 (aligned with latest storeon typings, so is super type safe) and it can be easy modified to option 2.

The last think is that we probably should change the name from createEpicMiddleware to createEpicModule as Storeon is using modules not middlewares. What's more epic module can be intitialized in imperative, lazy way like: const store = createStore(...); createEpicModule(epic)(store); instead to be delivered upfront to createStore factory (this is very powerful if you are using code splitting, micro frontends, ....), that is why I think module name is better.

Best regards.

majo44 commented 4 years ago

Knock, knock ?

distolma commented 4 years ago

Good points. Option 2 will be more reliable, imho. So, if you implemented Option 1, could you please change it to looks like Option 2?

With the naming convention, I fully agree with you.

Please send me PR with this approach.

majo44 commented 4 years ago

Hi

I have implementation in typescript. I used typescript as is hard to maintain implementation separately from typescript declarations, as in the area of rxjs types declarations are so simple. Also types for stereon, in case when we want to have very strict types the declarations becomes not simple.

I was looking for implementation which is strict typed, what means that you can:

On other hand as I see this lib is targeting es6, what complicates way of usage (for legacy browsers you have to use typescript or babel).

So the question is, can we switch to typescript ? And then build index.js (package.json main) as es5/commonjs and on side module.js (package.json module) as es6/es6 module ?

Implementation:


/**
 * Composed Storeon event type.
 */
export type StoreonEvent<Events, Event extends keyof Events = keyof Events> = {
    type: Event;
    payload: Events[Event];
};

/**
 * Observable of Storeon events.
 */
export type EventsObservable<Events, Event extends keyof Events = keyof Events> = Observable<StoreonEvent<Events, Event>>;

/**
 * Creates observable of dispatched store events.
 */
export const toEventObservable = <State, Events = any, InEvent extends keyof Events = keyof Events>(
    store: Store<State, Events>) => {
    const event$ = new Subject<StoreonEvent<Events, InEvent>>();
    store.on('@dispatch', (_, event) => event$.next(event as any));
    return event$;
};

export class StateObservable<S> extends Observable<S> {
    value: S;
    private __notifier = new Subject<S>();

    constructor(input$: Observable<S>, initialState: S) {
        super(subscriber => {
            const subscription = this.__notifier.subscribe(subscriber);
            if (subscription && !subscription.closed) {
                subscriber.next(this.value);
            }
            return subscription;
        });

        this.value = initialState;
        input$.subscribe(value => {
            // We only want to update state$ if it has actually changed since
            // redux requires reducers use immutability patterns.
            // This is basically what distinctUntilChanged() does but it's so simple
            // we don't need to pull that code in
            if (value !== this.value) {
                this.value = value;
                this.__notifier.next(value);
            }
        });
    }
}

/**
 * Creates observable of store state.
 */
export const toStateObservable = <State, Events = any>(store: Store<State, Events>): StateObservable<State> => {
    const stateInput$ = new Subject<State>();
    const state$ = new StateObservable<State>(stateInput$, store.get());
    store.on('@changed', state => stateInput$.next(state));
    return state$;
};

/**
 * Filters the observable
 * @param event
 */
export const ofEvent = <Events, InEvent extends keyof Events = keyof Events, OutEvent extends InEvent = InEvent>(event: OutEvent):
    OperatorFunction<StoreonEvent<Events, InEvent>, StoreonEvent<Events, OutEvent>> => {
    return (source) =>
        source.pipe(
            filter((ev): ev is StoreonEvent<Events, OutEvent> => { return event === ev.type }));
};

/**
 * Returns the combined event object
 * @param event the event type
 * @param value data which should
 */
export const toEvent = <Events, Event extends keyof Events = keyof Events>(
    event: Event, ...value: Events[Event] extends (never | undefined) ? [never?] : [Events[Event]]): StoreonEvent<Events, Event> => ({
    type: event,
    payload: (value[0] as Events[Event])
});

/**
 * RxJS side-effect implementation for storeon
 */
export interface Epic<State, Events = any, InEvent extends keyof Events = keyof Events, OutEvent extends InEvent = InEvent> {
    (action$: EventsObservable<Events, InEvent>, state$: StateObservable<State>): EventsObservable<Events, OutEvent>;
}

/**
 * Creates Storeon module.
 */
export const createEpicModule = <State, Events = any, InEvent extends keyof Events = keyof Events, OutEvent extends InEvent = InEvent>(
    epic: Epic<State, Events, InEvent, OutEvent>): Module<State, Events> => {
    const epic$ = new BehaviorSubject(epic);
    return (store: Store<State, Events>) => {
        const state$ = toStateObservable<State, Events>(store);
        const event$ = toEventObservable<State, Events, InEvent>(store);
        epic$.pipe(
            switchMap((epic) => epic(event$, state$))
        ).subscribe((ev) => (store.dispatch as any).apply(store, [ev.type, ev.payload]));
    }
};

/**
 * As the name suggests, allows you to take multiple epics and combine them into a single one.
 * Please keep in mind that the order in which epics are combined affect the order in which they are
 * executed and receive events.
 */
export function combineEpics<S, Es = any, IE1 extends keyof Es = keyof Es, OE1 extends IE1 = IE1>(
    e1: Epic<S, Es, IE1, OE1>): Epic<S, Es, IE1, OE1>;
export function combineEpics<S, Es = any, IE1 extends keyof Es = keyof Es, OE1 extends IE1 = IE1,
    IE2 extends keyof Es = keyof Es, OE2 extends IE2 = IE2>(
    e1: Epic<S, Es, IE1, OE1>, 
    e2: Epic<S, Es, IE2, OE2>): Epic<S, Es, IE1 | IE2, OE1 | OE2>;
export function combineEpics<S, Es = any, IE1 extends keyof Es = keyof Es, OE1 extends IE1 = IE1,
    IE2 extends keyof Es = keyof Es, OE2 extends IE2 = IE2,
    IE3 extends keyof Es = keyof Es, OE3 extends IE3 = IE3>(
    e1: Epic<S, Es, IE1, OE1>, 
    e2: Epic<S, Es, IE2, OE2>, 
    e3: Epic<S, Es, IE3, OE3>): Epic<S, Es, IE1 | IE2 | IE3, OE1 | OE2 | OE3>;
export function combineEpics<S, Es = any, IE1 extends keyof Es = keyof Es, OE1 extends IE1 = IE1,
    IE2 extends keyof Es = keyof Es, OE2 extends IE2 = IE2,
    IE3 extends keyof Es = keyof Es, OE3 extends IE3 = IE3,
    IE4 extends keyof Es = keyof Es, OE4 extends IE4 = IE4>(
    e1: Epic<S, Es, IE1, OE1>, 
    e2: Epic<S, Es, IE2, OE2>, 
    e3: Epic<S, Es, IE3, OE3>,
    e4: Epic<S, Es, IE4, OE4>): Epic<S, Es, IE1 | IE2 | IE3 | IE4, OE1 | OE2 | OE3 | OE4>;
export function combineEpics<S, Es = any, IE1 extends keyof Es = keyof Es, OE1 extends IE1 = IE1,
    IE2 extends keyof Es = keyof Es, OE2 extends IE2 = IE2,
    IE3 extends keyof Es = keyof Es, OE3 extends IE3 = IE3,
    IE4 extends keyof Es = keyof Es, OE4 extends IE4 = IE4,
    IE5 extends keyof Es = keyof Es, OE5 extends IE5 = IE5>(
    e1: Epic<S, Es, IE1, OE1>,
    e2: Epic<S, Es, IE2, OE2>,
    e3: Epic<S, Es, IE3, OE3>,
    e4: Epic<S, Es, IE4, OE4>,
    e5: Epic<S, Es, IE5, OE5>): Epic<S, Es, IE1 | IE2 | IE3 | IE4 | IE5, OE1 | OE2 | OE3 | OE4 | IE5>;
export function combineEpics<S, Es = any, IE extends keyof Es = keyof Es, OE extends IE = IE>(
    ...e1: Array<Epic<S, Es, IE, OE>>): Epic<S, Es, IE, OE>;
export function combineEpics(...epics: Array<Epic<any>>): Epic<any> {
    return (action$: EventsObservable<any>, state$: StateObservable<any>): EventsObservable<any> => {
        return merge(...epics.map((epic) => epic(action$, state$)));
    }
}

This implementation:

Example:

export const PUSH = 'PUSH';
export const DISMISS = 'DISMISS';
export const REMOVE = 'REMOVE';

export interface AppEvents {
    [PUSH]: undefined;
    [DISMISS]: string;
    [REMOVE]: {id : string};
}

export interface AppState {
    id: string;
}

const e1: Epic<AppState, AppEvents, typeof DISMISS | typeof PUSH, typeof PUSH> =
    (actions$, state$) =>
        actions$.pipe(
            ofEvent(DISMISS),
            map(v => toEvent(PUSH))
        );

const e2: Epic<AppState, AppEvents> = (actions$, state$) =>
    merge(
        actions$.pipe(ofEvent(REMOVE)),
        actions$.pipe(ofEvent(PUSH)),
    ).pipe(
        map((x) => {
            if (x.type === REMOVE) {
                console.log(x.payload.id);
                return toEvent(PUSH)
            } else {
                return toEvent(REMOVE, {id: 'sa'})
            }
        }));

const store = createStore<AppState, AppEvents>([]);
const rootEpic: Epic<AppState, AppEvents> = combineEpics(e1, e2);
createEpicModule(rootEpic)(store);
distolma commented 4 years ago

Great implementation! I think we can switch the project to typescript, but we need to be careful with the size of the output file. I will create a new project setup using typescript in typescript branch.

One thing we should discuss is how to better split modules. For example, we can move all operators to different scopes, like storeon-observable/operators, and only createEpicModule functions leave on the root module. If this doesn't affect the bundle size - we can leave as it is.

distolma commented 4 years ago

@majo44 Here is https://github.com/distolma/storeon-observable/tree/typescript

majo44 commented 4 years ago

@distolma

  1. Can you add me to contributors, I do not want to play with forks
  2. I do not know we need the multiple modules, as we want to make this still small and easy, whats more, I think will be much easy to do not compile to dist, but directly in place, what will simplify the way of building and publishing.
  3. Why are you using rollup ?
distolma commented 4 years ago

Using rollup for building TS code into separate modules, or you want to use only ES modules? If you know better ideas, please provide it.

majo44 commented 4 years ago

I will show you idea in PR.

majo44 commented 4 years ago

https://github.com/distolma/storeon-observable/pull/10

distolma commented 4 years ago

https://github.com/distolma/storeon-observable/pull/10