Skip to content
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

Code splitting / dynamically loading sagas #76

Closed
patrickheeney opened this issue Feb 3, 2016 · 25 comments
Closed

Code splitting / dynamically loading sagas #76

patrickheeney opened this issue Feb 3, 2016 · 25 comments

Comments

@patrickheeney
Copy link

I just discovered redux-saga and I am looking to start integrating this into my app. I am currently code splitting my reducers and using replaceReducer when they hit the correct route. From the saga examples it looks like they all use a rootSaga. The closest I found to an answer is #23 (comment) but I am not certain if this is the official API and usage.

My current setup looks something like this: http://stackoverflow.com/a/33045558 which I pasted a snippet below:

function createRoutes(reducerRegistry) {
  return <Route path="/" component={App}>
    <Route path="async" getComponent={(location, cb) => {
      require.ensure([], require => {
        reducerRegistry.register({async: require('./reducers/async')})
        cb(null, require('./screens/Async'))
      })
    }}/>
  </Route>
}

function createStore(reducerRegistry) {
  var rootReducer = createReducer(reducerRegistry.getReducers())
  var store = createStore(rootReducer)

  reducerRegistry.setChangeListener((reducers) => {
    store.replaceReducer(createReducer(reducers))
  })

  return store
}

From the other comment, it sounds like this should be possible, but it looks like sagas need access to the store?

    <Route path="async" getComponent={(location, cb) => {
      require.ensure([], require => {
        reducerRegistry.register({async: require('./reducers/async').default})
        const sagas = require('./sagas').default
        // store?
        runSaga(sagas) 
        // right api usage? where to get the store? does any array work?
        sagas.forEach(saga => runSaga( saga(store.getState), store )) 
        // only supports one rootSaga, not an array?
        runSaga( sagas(store.getState), store )
        cb(null, require('./screens/Async'))
      })
    }}/>

It looks like a middleware approach was discussed in #13 (comment) that would have solved this, but it was closed. This could have potentially worked:

  var store = createStore(rootReducer)

  reducerRegistry.setChangeListener(reducers => {
    store.replaceReducer(createReducer(reducers))
  })

  sagaRegistry.setChangeListener(iterator => {
    store.runSaga(iterator)
  })

Would this be the official usage? What about "stopping/removing/disconnecting" the sagas? Will this work with server side rendering?

@yelouafi
Copy link
Member

yelouafi commented Feb 4, 2016

Would this be the official usage?

Actually yes. But there are some issues with the actual method runSaga (see #48). Sagas started by runSaga can't see actions from sagas started by the middleware

It looks like a middleware approach was discussed in #13 (comment) that would have solved this

Not a middleware but a store enhancer. There are some potential issues with this approach (see #13 (comment))

It seems the solution proposed by @gaearon (#48 (comment)) is the simplest one; a run method will be attached to the middleware itself.

Would this be the official usage? What about "stopping/removing/disconnecting" the sagas? Will this work with server side rendering?

What about "stopping/removing/disconnecting" the sagas?

Didn't think about this. But If we provide a way to dynamically attach sagas to the middleware, it makes sens to be able to detach them. I think we can do this by providing exposing a cancel method

Will this work with server side rendering?

I don't see any issue with that

@patrickheeney
Copy link
Author

So basically as it stands now its not possible, but an "official" api (run method within the middleware) is being considered? I just wanted to make sure I didn't miss anything from the current API as I am new to sagas.

@yelouafi
Copy link
Member

yelouafi commented Feb 4, 2016

So basically as it stands now its not possible

for now you can use runSaga but only if your dynamic Sagas don't take actions from middleware-Sagas. Otherwise yes, it's not possible

but an "official" api (run method within the middleware) is being considered?

Yes exactly. I think I'll go with that. should be probably this week-end if things go well

@gaearon
Copy link
Contributor

gaearon commented Feb 4, 2016

I think we can do this by providing exposing a cancel method

Usually in Redux we try to return a function for this, e.g. const unsubscribe = store.subscribe(...). Not saying it always makes sense but worth considering as an API choice.

@yelouafi
Copy link
Member

yelouafi commented Feb 4, 2016

Usually in Redux we try to return a function for this, e.g. const unsubscribe = store.subscribe(...). Not saying it always makes sense but worth considering as an API choice.

Actually runSaga returns a task descriptor which contains some useful fields, like a done promise, an isRunning() method. My idea was adding some cancel(reason) method to the descriptor

@gaearon
Copy link
Contributor

gaearon commented Feb 4, 2016

Ah, okay then!

@yelouafi
Copy link
Member

yelouafi commented Feb 8, 2016

FYI new 0.7.0 version is out with the new method of running dynamic sagas

@patrickheeney
Copy link
Author

I haven't tried this out yet but this is the pseudocode I will be experimenting with.

// app
import createSagaMiddleware from 'redux-saga'

const rootReducer = createReducers(reducerRegistry.getReducers());
const sagaMiddleware = createSagaMiddleware(sagaRegistry.getSagas());

const store = createStore(
  rootReducer,
  initialState,
  applyMiddleware(sagaMiddleware)
);

reducerRegistry.setChangeListener(reducers => {
  store.replaceReducer(createReducer(reducers));
});

sagaRegistry.setChangeListener(sagas => {
  sagaMiddleware.run(sagas);
});

// routes
function createRoutes(reducerRegistry, sagaRegistry) {
  return <Route path="/" component={App}>
    <Route path="async" getComponent={(location, cb) => {
      require.ensure([], require => {
        reducerRegistry.register(require('./reducers').default);
        sagaRegistry.register(require('./sagas').default);
        cb(null, require('./views/Index').default);
      });
    }}/>
  </Route>
}

@mjrussell
Copy link
Contributor

@patrickheeney I was implementing something similar recently for code splitting with both reducers and sagas. Its working quite well, although the sagaRegistry has to be implemented a bit differently from the reducerRegistry.

With reducers, you can easily trigger store.replaceReducer(createReducer(reducers)); on each route change because the reducer is pure function without side effects and the store data is held elsewhere.

With the sagas, however, it would be incorrect to both (1) run the all the sagas store in the registry and (2) re-run sagas that have been started for the route.

(1) is pretty easy to solve by just only sending to the change listener the new saga
(2) seems a bit more tricky, you could (a) use the name of the function as only fire off new names, or (b) send a key in the sagaRegistry.register and only emit a new change for a new key.

Im currently doing (1) and (2b) but I'd be interested to hear how others tackle the problem

@yelouafi
Copy link
Member

Closing this, feel free to add your comments. I can reopen it if needed.

@pke
Copy link
Contributor

pke commented Mar 10, 2016

I don't quite understand the code split with the reducers. Care to elaborate on that?

@mjrussell
Copy link
Contributor

@pke check out reduxjs/redux#37 for more details on code splitting with reducers. Its got a great explanation by Dan Ambramov about the process and workflow.

@johanneslumpe
Copy link

johanneslumpe commented Dec 2, 2016

@mjrussell I'm at the stage now where I need to implement the saga library. I'm still new to sagas, which is probably why this is a bit harder for me right now. I am trying to figure out how to do hot-reloading and code-splitting for sagas. Would you mind sharing your approach? I realize that i can call sagaMiddleware.run multiple times for different sagas. That will not get rid of the original saga though. So if I run a saga, then change some code inside said saga and hot-reload it, how can I make sure to replace the old saga without losing state?

Maybe @yelouafi can chime in too :)

One way I figured it might work is by storing the returned tasks for each .run call and then cancel them when updates arrive before registering the new saga - not sure if this really works though.

@mjrussell
Copy link
Contributor

@johanneslumpe AFAIK you cannot hot-reload a saga. Changes to sagas during hot reloading should force an entire app refresh. What I do is pass in a "key" of the saga to a registry which calls sagaMiddleware.run. And I only run a saga when I register a saga with a key that is different from any key I've registered before. This is because the registration exists inside the getComponent call in a React Router <Route> which is invoked on every route enter/change

@johanneslumpe
Copy link

johanneslumpe commented Dec 3, 2016

@mjrussell that's the way I went for now - only adding new sagas to the store and passing them to .run - seems to work fine. Hot reloading might still be possible - apart from the fact that it is going to reset the reloaded saga (which might or might not be an acceptable trade-off).

@GuillaumeCisco
Copy link

GuillaumeCisco commented Jan 17, 2017

Hello there,

I had the same issue and found redux-injector for injecting reducers asynchronously via routing.
I decided to create the same philosophy for redux-sagas : https://github.com/GuillaumeCisco/redux-sagas-injector

You can find an exemple in the README.
Using injectReducer and injectSaga, you can easily load your js code asynchronously via routing and deciding what route need only what it needs.

Hope you'll enjoy it 🎉

PS: It also works with SSR Stream Caching and with react-redux >= 6.0.0

@navneet-g
Copy link

We recently built a library called redux-dynamic-modules. The library helps in code splitting Redux applications and allows loading reducers, middlewares, sagas dynamically. It also comes with a HOC to load/unload a module when the component requiring them mounts or unmounts.

@luskin
Copy link

luskin commented Feb 15, 2019

@navneet-g cool library, any roadmap to support immutable?

@navneet-g
Copy link

@luskin we use immer in our web application and do not see any issues using it along with redux-dynamic-modules. Please share any issues you are observing with immutable.

@luskin
Copy link

luskin commented Feb 15, 2019

@navneet-g Is this issue irrelevant then?

microsoft/redux-dynamic-modules#52

@luskin
Copy link

luskin commented Feb 15, 2019

When using immutable with redux-dynamic-modules you receive the following error:

The previous state received by the reducer has unexpected type of "Object". Expected argument to be an object with the following keys: "accountCredit"

@navneet-g
Copy link

Thanks @luskin , let us continue the discussion on the issue there. Seems like Alex plans to take a stab at it.

@can-acar
Copy link

can-acar commented Jun 1, 2019

import React, {Component} from "react";
import {ReactReduxContext} from "react-redux";
import hoistNonReactStatic from 'hoist-non-react-statics';


const withSaga = (saga = []) => WrappedComponent => {

    class ReduxSagaInjector extends Component {
        constructor(props) {
            super(props);

            const {reduxContext} = props;

            reduxContext.store.injectSaga(saga)

        }

        render() {
            return <WrappedComponent {...this.props}/>
        }
    }

    const Wrapped = (props) => {
        return (
            <ReactReduxContext.Consumer>
                {(reduxContext) => <ReduxSagaInjector reduxContext={reduxContext} {...props}/>}
            </ReactReduxContext.Consumer>
        );
    };
    return hoistNonReactStatic(Wrapped, WrappedComponent);
};
export {withSaga};

how to use?

/// configuritionStore.jsx

  const store = createStore(createRootReducer(initialState), initialState, bindMiddleware([loggerMiddleware, sagaMiddleware, routeMiddleware]));


  store.injectSaga = (rootSaga) => {
        store.sagaTask = sagaMiddleware.run( rootSaga, store.getState);
    }

Some router component

  
 const saga = withSaga(demoRootSaga);
 const withConnect= connect(state=>state,{});

export default compose(saga,withConnect)(Component);

@dthtien
Copy link

dthtien commented Oct 11, 2019

Hi guys, I have made the injector like this. But I'm stuck with the ejection saga in this HOC. Could anyone please give me the suggestion? Thank you so much.

@GuillaumeCisco
Copy link

Hi @dthtien I've recently updated my public website using this technology.
You can read about it there: https://github.com/GuillaumeCisco/guillaumecisco
Feel free to implement things your own way.
You will find multiple examples of how to inject sagas.
I've created a project entitled https://github.com/GuillaumeCisco/redux-sagas-injector helping with dynamic sagas injection under route splitting.

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

No branches or pull requests