New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redux-observable? #40

Open
DarrylD opened this Issue Sep 12, 2017 · 7 comments

Comments

Projects
None yet
4 participants
@DarrylD

DarrylD commented Sep 12, 2017

Any thought on adding redux-observable support?

Could look something like:

@kea({
    key: (props) => props.id,

    path: (key) => ['scenes', 'homepage', 'slider', key],

    actions: () => ({
        updateSlide: index => ({ index })
    }),

    reducers: ({ actions, key, props }) => ({
        currentSlide: [props.initialSlide || 0, PropTypes.number, {
            [actions.updateSlide]: (state, payload) => payload.key === key ? payload.index % images.length : state
        }]
    }),

    selectors: ({ selectors }) => ({
        currentImage: [
            () => [selectors.currentSlide],
            (currentSlide) => images[currentSlide],
            PropTypes.object
        ]
    }),

    epic: (action$, store, selectors) => action$
        .ofType(SOMETHING_INIT)
        .map(() =>
            selectors.getSlideImages(store.getState()) 
        )
        .map(images => ({
            ...images,
            ...getSlideMetaData(store.getState(), images.cardSelection)
        }))
        //..etc...
})

@DarrylD DarrylD changed the title from Redux-obseravbles? to Redux-observable? Sep 12, 2017

@mariusandra

This comment has been minimized.

Member

mariusandra commented Sep 13, 2017

Hey, this looks pretty cool. I've never used RxJS myself and didn't know about redux-observable until now. I watched the 37min talk the docs linked to and that's about as much as I know about it now. So I need to play with it a bit before I can give you an informed opinion about this. :)

@DarrylD

This comment has been minimized.

DarrylD commented Sep 13, 2017

It's worth the look into. I like sagas but, moved from them because it's super redux specific while observables aren't and I can move them around everywhere (middleware, components, backend, etc...). It's basically "lodash for async".

If I have some time soon, I could pitch in.

@mariusandra

This comment has been minimized.

Member

mariusandra commented Sep 14, 2017

So I read through the docs of RxJS and redux-observable.

I can definitely see how it's an attractive alternative to redux-saga. Of course implementing it has some issues. The biggest ones:

1. Bundle size and redux-saga.

I'm quite sure that making rxjs a required dependency is a no-go. At the same time, if you use redux-observable, you might want to omit redux-saga from your bundle.

While we're at it, could we make Kea flexible enough to add new side-effect libraries in the future? I'm sure that with redux-observable we haven't seen the end of it.

Thus I'm favouring some sort of plugin system. But then, how to implement it? How to opt in to the features you want? How to know what you even want? :).

I need to revisit the current code and think of ways to achieve this. It is in dire need of some refactoring anyway. In fact, I already started with refactoring the low hanging fruits.

I believe in the end it should be as simple as

// in your root index.js
import 'kea-saga'
import 'kea-epic' // or kea-observable?

// elsewhere in the code
import { kea } from 'kea' // and use normally

2. When and how to start and stop epics?

Assuming epics are injected into Kea properly, we get into the nitty gritty of how to use them. Here's where I'll need help the most.

The main issue is related to starting and stopping epics as components are mounted/unmounted. I think I have an idea how to do it

Starting is probably easy. Here's how it's done for the Sagas - I create a channel, onto which kea sends all sagas that need to be started. The sagas themselves pay attention to if they're already running or not.

Then when the component unmounts, I send a cancel event on the same channel.

How to do it with observables? I guess something similar would work? How to assure that only one instance of a observable is running?

Are there any common ways to stop observables, other than .takeUntil(action$.ofType("@@kea/unmounted/scenes.homepage.index"))?

3. Syntactic sugar

With sagas, in Kea you can do yield this.get('nameOfSelector') and that's syntactic sugar for yield select(this.selectors.nameOfSelector). Should we have anything similar for redux-observable?

As by your example, we'd be anyway passing both the store and selectors to the epic, we could as well combine them into a "select" function, e.g.:

    // select = (selectorName) => selectors[selectorName](store.getState())

    epic: (action$, store, select) => action$
        .ofType(SOMETHING_INIT)
        .map(() =>
            select('slideImages')
        )
        .map(images => ({
            ...images,
            ...select('slideMetadata')
        }))
        //..etc...

... and/or how to give to the epic the created actions? Perhaps something like:

    // select = (selectorName) => selectors[selectorName](store.getState())

    epic: ({ actions, select }) => (action$, store) => action$
        .ofType(actions.initAction)
        .map(() =>
            select('slideImages')
        )
        .map(images => ({
            ...images,
            ...select('slideMetadata')
        }))
        .delay(1000)
        .map(image => actions.doSomethingToImage(image))
        //..etc...

... or to simplify the first line like:

    epic: ({ actions, select, action$, store }) => action$

... ? :)

As I haven't used redux-observable, I don't know what is the best way forward here. I need to play around with it. So all sort of feedback is welcome! :)

@mariusandra

This comment has been minimized.

Member

mariusandra commented Sep 29, 2017

Hey, starting with v0.25, kea-saga is now a separate package and all redux-saga code has been ripped out of the main package.

I also added a kea-thunk package.

Next step: create a kea-observable package and add support for epics!

@DarrylD

This comment has been minimized.

DarrylD commented Oct 2, 2017

  1. I like the plugin concept. redux-observable is fairly small since you can also add in the operators as you need them (import 'rxjs/add/operator/map';)

  2. I haven't needed to unsubscribe any of my epics but, this is a good read on the topic https://medium.com/@benlesh/rxjs-dont-unsubscribe-6753ed4fda87. Looks like takeUntil (or takeWhile ) would be ideal.

  3. I'm a big fan of epic: ({ actions, select }) => (action$, store) => action$. Less to rewrite when migrating or copy and pasting :)

@nhducit

This comment has been minimized.

nhducit commented Oct 19, 2017

these errors make me think of replace Redux Observable with Redux saga
redux-observable/redux-observable#94
redux-observable/redux-observable#263

@jayphelps

This comment has been minimized.

jayphelps commented Dec 22, 2017

FWIW the error swallowing bug was fixed in rxjs 5.5.6 ✌️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment