State should be passed with the action about redux-cycles HOT 19 CLOSED

from redux-cycles.

A downside to keeping these separate is you either do not have the latest state or action as soon as one updates, so you end up needing to implementing logic to take next. I tried this initially as well and it just ended up introducing unnecessary complexity.

from redux-cycles.

Do you mean passing 'store' so that you can getState() when needed? (a-la redux-observable)

from redux-cycles.

Sometimes you only need one or the other, though, and I think having both separate kind of makes sense. Both thunks and sagas are explicit when it comes to accessing the state.

The readme already shows how to combine the two: https://github.com/cyclejs-community/redux-cycles#drivers

It might look a bit too implicit. Maybe it would be a good idea to provide a utility function, like actionsWithState(sources), that would do the combining for you. Maybe not. @nickbalestra @lmatteis wdyt?

@nickbalestra I would say against passing store in and doing getState inside. It is imperative and it might be possible for the getState() call to occur several actions later after the one in question, so the state obtained this way would be too new for the action.

from redux-cycles.

I think passing getState() could work. I personally am passing { action, state } in each value of my REDUX source. I avoided passing getState() in my implementation under the premise that grabbing state in an imperative manner was not the best "cycle" way to do things (this is just my opinion). By passing getState you can guarantee you have the absolute latest state, which does seem advantageous.

The key difference between why redux-saga separates this out and how cycle works is that a generator is sequentially executing. With a source you have to wait for the next value instead of pulling it on the next line of code.

from redux-cycles.

@goshakkk totally agree on staying away from having an imperative getState(), was asking to get a better understanding of the context.

Personally I'm totally fine with how it's at the moment, as you said you don't need both all the times, and when you need combining them is pretty trivial, but I can see when this can get a bit into the way. Not sure I'll like to provide specific actionsWithState either I'll go with one API (a combined one) or a separate one, but not both, my 5c.

from redux-cycles.

@kloy what sort of API are you thinking about? Sources.REDUX would emit an array such as: [ action, state ] or an object? Also, what if you want to react only to state changes? The state driver is still needed.

EDIT: also we should think about our cycle functions more abstractly - not specifically tied to redux. Concepts of actions and state are pretty abstract. However, combing them together would imply a system (like redux) where each action triggers a state change - which might not necessarily always be the case in other state management systems. Also how would you call such stream? ACTIONSTATE? REDUX?

from redux-cycles.

After thinking about it I guess it does make sense. But I'm still in doubt about the name. Also the API would make it more clunky (.filter(({ action }) => rather than .filter(action =>. Having the state is indeed convenient and, after using redux-observable, it's something we do quite often.

sources.ACTION.filter(({ state }) => state.counter % 2 == 0) seems nice though. Even though semantically speaking you're listening for ACTION changes, not state changes. I'm not sure.

from redux-cycles.

I named the driver REDUX in my current implementation and pass the store to it versus running the cycle app from redux. This allows for anything matching the store contract to be used for state management.

The REDUX source passes an object with action and state props. New values are emitted for every action regardless of state change. This is to work around the actions being leveraged as an event bus at times in our app. Listening for state changes is done using distinct since the references are unique.

from redux-cycles.

How do you create the middleware though?

const reduxCycles = reduxCycles();
const middleware = reduxCycles.createCycleMiddleware();
const store = createStore(
  rootReducer,
  applyMiddleware(middleware)
);
Cycle.run(main, { REDUX: reduxCycles.makeDriver(store) });

So .createDriver sets a local listener which is accessed by the middleware defined earlier?

from redux-cycles.

Apart from merging the ACTION and STATE drivers, I like the idea of keeping the run in userland rather than inside the middleware. This would also help when we switch to Unified and allow users to use different stream libs.

from redux-cycles.

I don't use middleware. If I understand your middleware correctly you are just using it to build the driver for cycle, and subsequently also calling cycle run. I instead break the driver and the wrapping of redux's state in an observable into separate approaches. This is what it looks like...

// cycle driver for redux
export function makeReduxDriver(store$, dispatch) {
    return function reduxDriver(input$) {
        input$.subscribe(action => {
            dispatch(action);
        });

        return store$;
    };
}

// redux store enhancer
import { Subject } from 'disco/rx';

function magic(
    createStore,
    reducer,
    initialState
) {
    const store = createStore(reducer, initialState);
    const store$ = new Subject();

    function dispatch(action) {
        const dispatched = store.dispatch(action);
        const state = store.getState();
        store$.next({action, state});
        return dispatched;
    }

    return {
        ...store,
        dispatch,
        store$: store$.share()
    };
}

const enhancer = createStore => (reducer, initialState) => (magic(
    createStore,
    reducer,
    initialState
));

export default enhancer;

from redux-cycles.

@kloy that's cool implementing this as a store enhancer, although you'd probably achieve the same using a more API-safe solution in the form of a middleware.

In terms of usage it doesn't change much from the solution I proposed above - still puts Cycle.run in user-land.

Thoughts @goshakkk @nickbalestra? We can:

• Create a single REDUX driver as specified above and pass {action, state} along the stream. So it's objects in and actions out. Not sure of any shortcoming of this apprach
• Otherwise we keep ACTION and STATE but we don't run Cycle.run from within the middleware, but in user-land (as my example above).
• Or we just don't do anything, keep it as is, and perhaps pass run as a third argument when Unified is shipped.

Cast your preference!

from redux-cycles.

• 👍 on the single REDUX driver (not sure about naming though)
• 👍 on passing run as a third argument
• 👎 on having run in userland (current API are cleaner imho)

from redux-cycles.

If run is not in userland how would you support users writing their own cycle adapters (I do this now for optimizing bundle size).

Also, I like the names store or redux for the driver name personally.

from redux-cycles.

👎 on a single driver
👍 on either run as an argument or exposing ACTION and STATE drivers more explicitly

from redux-cycles.

@kloy by cycle adapters what do you mean? Stream adapters?

from redux-cycles.

You can see referenced 2 PRs:

#26 is about using a single STORE driver - frankly I'm not keen about this solution.
#27 I think is the best solution. I exported run in user-land and am exporting drivers explicitly. Also upgraded to Cycle Unified so now we can use RxJs and added a test for it as well.

@goshakkk @nickbalestra @kloy

EDIT: the only thing I'm worried about with #27 is that the drivers are no longer using the store passed by the middleware, but the one from user-land (from createStore). I doubt there are differences though, right?

from redux-cycles.

Closing this as we decided to go with #28. It was inspiring for us to expose the drivers in userland, but we still like to have separate drivers for ACTION and STATE.

from redux-cycles.