Giter Site home page Giter Site logo

Comments (15)

butchmarshall avatar butchmarshall commented on May 3, 2024 1

I just put together a minimal example - the kicker... it works as expected! I'll keep investigating as to why it doesn't work in my full react app.

In my full app if i don't "fork" it works. I think i'll just not fork.

import fetch from 'node-fetch';
import {createStore, applyMiddleware, compose, combineReducers} from "redux";
import createSagaMiddleware, { delay, END } from 'redux-saga';
import { apiMiddleware,CALL_API } from 'redux-api-middleware';
import { all, race, take, put, call, fork, select, takeEvery, takeLatest, takeFirst, actionChannel, cancelled, throttle } from 'redux-saga/effects';

// Action

function createAction() {
	return {
		[CALL_API]: {
			endpoint: '/create',
			fetch: () => {
				return fetch("https://www.google.com");
			},
			method: 'GET',
			body: {},
			types: [
				{
					type: "CREATING",
					payload: {some_data_for_ui_update:1}
				},
				"CREATED",
				"CREATE_FAILED",
			],
		}
	};
}

// Saga

function * createActionResource(data) {
	yield put(createAction());
	yield call(delay,3000);
}


function * createActionSaga(action) {
	yield fork(createActionResource, {});
	//yield put(createAction());
console.log("OK WAITING FOR RACE");
	const response = yield race({
		success: take("CREATED"),
		fail: take("CREATE_FAILED"),
	});

	console.log("ANTYHING AFTER FORK???", response);
}

export function* watchCreateAction() {
	yield takeEvery("CREATE_ACTION", createActionSaga);
}

function * rootSaga() {
	yield all([
		watchCreateAction(),
	]);
}

// Reducer

const reducer = (state = {}, {type, payload}) => {
	console.log("in reducer:", type, payload);

	return state;
}

const rootReducer = combineReducers({
    reducer,
});

// Store

const sagaMiddleware = createSagaMiddleware();

const middlewares = [
	apiMiddleware,
	sagaMiddleware,
];
const middlewareEnhancer = applyMiddleware(...middlewares);

const storeEnhancers = [middlewareEnhancer];

const composedEnhancer = compose(...storeEnhancers);

const store = createStore(
	rootReducer,
	undefined,
	composedEnhancer
);

store.runSaga = sagaMiddleware.run;
store.runSaga(rootSaga);


store.dispatch({
	type: "CREATE_ACTION"
});

from redux-saga.

dmitriykharchenko avatar dmitriykharchenko commented on May 3, 2024

Looks like problem caused by the fact that there are different emitters and same dispatcher inside middleware and runSaga:

middleware code:

export default (...sagas) => ({getState, dispatch}) => {

  const sagaEmitter = emitter()

  sagas.forEach( saga => {
    proc(
      saga(getState),
      sagaEmitter.subscribe,
      dispatch,
      action => asap(() => dispatch(action)),
      0,
      saga.name
    )
  })

  return next => action => {
    const result = next(action) // hit reducers
    sagaEmitter.emit(action)
    return result;
  }
}

storeIO code, that creates subscribe/dispatch io for store:

export function storeIO(store) {

  if(store[IO])
    return store[IO]

  const storeEmitter = emitter()
  const _dispatch = store.dispatch
  store.dispatch = action => {
    const result = _dispatch(action)
    storeEmitter.emit(action)
    return result
  }
   store[IO] = {
    subscribe: storeEmitter.subscribe,
    dispatch : store.dispatch
  }

  return store[IO]

Both functions are taking dispatchers from store and creating new emitter for subscribe. So, saga called with runSaga can dispatch actions to store and all sagas from middleware will see it, but actions dispatched from middleware sagas can't be taken by saga called with runSaga

from redux-saga.

yelouafi avatar yelouafi commented on May 3, 2024

That's annoying. I didnt like the monkey patching solution used in runSaga from the start. Things would be much easier if the redux store provided the dispatched action directly to its subscribers.

One possible solution is redux-saga providing explicit channels for inter-saga communication.

from redux-saga.

gaearon avatar gaearon commented on May 3, 2024

Would an alternative API like

const sagas = sagaMiddleware(midSaga1, midSaga2)
const store = applyMiddleware(sagas)(createStore)(reducer)
sagas.run(externalSaga);

help fix this? I'm not sure I understand the problem yet, just thinking aloud. In other words I'm proposing to make run instance method of the middleware function itself so it has access to the dispatched actions.

from redux-saga.

yelouafi avatar yelouafi commented on May 3, 2024

Yes I think it would work; Although the developer has to export the middleware which feel a bit unnatural (just a feeling :) ).

I think also using a store enhancer with a lifted reducer like in devtools would also work, since the reducer can intercept all actions. Basically it's a problem of getting a store action from the outside, it seems I was doing it the wrong way.

from redux-saga.

gaearon avatar gaearon commented on May 3, 2024

Yes I think it would work; Although the developer has to export the middleware which feel a bit unnatural (just a feeling :) ).

Agreed, OTOH this pattern is used by other popular middleware like https://github.com/rackt/react-router-redux. So maybe not too evil.

I think also using a store enhancer with a lifted reducer like in devtools would also work, since the reducer can intercept all actions.

Lifting reducers is useful if you want to accumulate custom state without letting the app know. In case of sagas it doesn't seem necessary.

from redux-saga.

 avatar commented on May 3, 2024

I'm also trying to understand the problem here so I might be able to patch my local version while waiting for a PR from @gaearon (which I'm sure will be a better solution than my own).

When you invoke the middleware with sagaMiddleware(...sagas) it creates one new emitter. For each saga the emitter's subscribe is handed over to proc along with the store's dispatch function. That subscribe fn is immediately invoked and handed a callback that will use a closure to capture deferredInputs (which is an array that receives gets a new item each time a runTakeEffect is invoked).

At some point in the future (during the middleware next chain) that same emitter instance is invoked via sagaEmitter.emit(action), which trigger's a flushing of the matching deferredInputs... then a whole bunch of other stuff happens that I don't quite follow yet-- generating descriptions of events... somewhere in there is the kick-start to the whole process... dispatch is finally triggered with a runPutEffect (not sure who's running the put)... chaining/reducing of actions + events and resolving deferred values via next and runEffect, etc.

When you call runSaga it creates a new emitter instance, and hooks up the subscribe of proc to this different instance's subscribe, and attempts to hand over the same dispatching function. It can't hand over the exact same function, though, because it needs a way to invoke emit(action) for this new emitter instance... and since the middleware chain is set and locked during startup, we can't become another step in the middleware. Instead, we slip ourselves into the middleware at the tail end of the original call to dispatch, which is redirected to a monkey-patched dispatch to flush this separate emitter's subscribers.

And here we arrive at @dmitriykharchenko much more concise explanation:

saga called with runSaga can dispatch actions to store and all sagas from middleware will see it, but actions dispatched from middleware sagas can't be taken by saga called with runSaga

The proposed solution is to get rid of runSaga and instead offer sagaMiddleware.run so that on a run we can just invoke another proc but this time utilize the original emitter instance and have a handle to the original dispatch function?

I'm a little confused by this statement:

so it has access to the dispatched actions


By the way @yelouafi really neat stuff, I'm learning a lot from reading your code.

from redux-saga.

 avatar commented on May 3, 2024

I also don't like the pattern of exporting middleware, it just seems smelly.

export default function createSagaMiddleware(...sagas) {
    const sagaEmitter = emitter()
    let _dispatch

    function middleware({ getState, dispatch }) {
        _dispatch = dispatch // better way to do this? really hacky
        sagas.forEach(saga => {
            proc(
                saga(getState),
                sagaEmitter.subscribe,
                dispatch,
                action => asap(() => dispatch(action)),
                0,
                saga.name
            )
        })

        return (next) => (action) => {
            const result = next(action) // hit reducers
            sagaEmitter.emit(action)
            return result;
        }
    }

    middleware.run = (iterator, { subscribe, dispatch }, monitor = noop) => {
        if (!subscribe) {
            subscribe = sagaEmitter.subscribe
        }

        if (!dispatch) {
            dispatch = _dispatch
        }

        check(iterator, is.iterator, NOT_ITERATOR_ERROR)

        return proc(
            iterator
            , subscribe
            , dispatch
            , monitor
        )
    }

    return middleware
}

If someone is using an IoC container, they now have to jump through some hoops to have access to run ( which was previously exposed directly as runSaga ). I'm not sure how, though, you could split this up into two modules - they seem rather dependent on one another's scope.

It would be nice if you could still have a runSaga exposed as it is now, perhaps (optionally) curried with the results of createSagaMiddleware (with the ability to override for different IO schemes).

from redux-saga.

gaearon avatar gaearon commented on May 3, 2024

If someone is using an IoC container, they now have to jump through some hoops to have access to run ( which was previously exposed directly as runSaga ). I'm not sure how, though, you could split this up into two modules - they seem rather dependent on one another's scope.

What is the big difference between

import { runSaga } from 'redux-saga'

and

import sagaMiddleware from '../sagas/middleware'
sagaMiddleware.runSaga(...)

?

In any case, beginners likely wonโ€™t need this, and when you need I think itโ€™s OK to export saga middleware just like you currently export sagas themselves.

from redux-saga.

 avatar commented on May 3, 2024

I'm more referring to how to manage instance-sharing. If invoking the middleware creates a new EventEmitter and we need that emitter to be shared between runSaga and all the ...sagas, it becomes sort of a 'conditional export', if that makes sense. You have to have invoked the sagaMiddleware before you can use runSaga with the above strategy, unless you can think of a more clever way to share the same emitter? Maybe some sort of ...

import { runSaga } from 'redux-saga'

const runner = runSaga(emitterInstance)

runner(saga(), storeIO(store))

The issue with running an IoC container becomes something like this...

ioc.register('sagaMiddleware', () => {
    const saga = require('redux-saga')
    // or import { saga } from 'redux-saga'

    let runSaga
    function createMiddleware(sagas) {
        // generate the middleware
        let middleware = saga.default(sagas)

        // now that we have the instance, hook up to 'run'
        runSaga = middleware.runSaga

        return middleware
    }

    createMiddleware.runSaga = (args) => {
        if (!runSaga) {
            throw new Error('cannot runSaga before initializing sagaMiddleware')
        }

        return runSaga(args)
    }

    return createMiddleware
})

It's a lot of hoops to jump through, and in the end we cannot register a runSaga component to be injected where it's needed across the app. Instead we have to utilize the opaque createMiddleware.runSaga. Not a huge deal, and can probably be solved by runtime/dynamic IoC registration, but it's still jumping through hoops. If someone smarter than me could devise a way to share the emitter instance while exposing two independent modules for export that would make life a lot easier.

ioc.register('sagaMiddleware', require('redux-saga').default)
ioc.register('runSaga', require('redux-saga').runSaga)

from redux-saga.

yelouafi avatar yelouafi commented on May 3, 2024

FYI version 0.7.0 provides a new method to start dynamic Sagas

from redux-saga.

butchmarshall avatar butchmarshall commented on May 3, 2024

I still seem to be getting this in 0.15.4, v0.16.0 and v1.0.0-beta.0

redux-api-middleware is dispatching actions that my reducer gets but my saga does not (both via takeEvery and race).

from redux-saga.

dmitriykharchenko avatar dmitriykharchenko commented on May 3, 2024

I believe this is the completely different case.
@butchmarshall could you share the code?

from redux-saga.

Andarist avatar Andarist commented on May 3, 2024

It might be some scheduling problem (we have a simple internal scheduler), cant say more without a slimmed down repro case though. Please also test this only against v1-beta as it has improved scheduling over 0.x

from redux-saga.

ProphetDaniel avatar ProphetDaniel commented on May 3, 2024

Sagas started with runSaga can't take actions from Saga started with the middleware

In the other hand, according to @butchmarshall 's example, reducers are able to take actions from the sagas started with the middleware.

So my workaround to this problem is to dispatch a new event triggered by the reducer capture so that the sagas started with runSaga can properly take them indirectly in a two step process. Just make sure this anti-pattern is avoided.

from redux-saga.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    ๐Ÿ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. ๐Ÿ“Š๐Ÿ“ˆ๐ŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google โค๏ธ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.