9

I have a React app that dynamically loads a module, including the module's reducer function, and then calls Redux's replaceReducer to, well, replace the reducer. Unfortunately I'm getting an error of

Unexpected key "bookEntry" found in initialState argument passed to createStore. Expected to find one of the known reducer keys instead: "bookList", "root". Unexpected keys will be ignored.

where bookEntry was a key on the older reducer that's getting replaced. And starting with the bookEntry module and switching to bookList causes this inverse error

Unexpected key "bookList" found in initialState argument passed to createStore. Expected to find one of the known reducer keys instead: "bookEntry", "root". Unexpected keys will be ignored.

The code is below - un-commenting the commented code does in fact fix this, but I'm guessing it shouldn't be needed.

Am I doing something else wrong with Redux that's making this code necessary?

function getNewReducer(reducerObj){
    if (!reducerObj) return Redux.combineReducers({ root: rootReducer });

    //store.replaceReducer(function(){
    //    return {
    //        root: rootReducer()
    //    }
    //});

    store.replaceReducer(Redux.combineReducers({
        [reducerObj.name]: reducerObj.reducer,
        root: rootReducer
    }));
}
Adam Rackis
  • 82,527
  • 56
  • 270
  • 393
  • Is there any specific reason you want to remove the previous reducer when dynamically loading code? I don't quite understand. Generally you want the old reducers to stay, not to be removed. – Dan Abramov Dec 06 '15 at 19:56
  • @DanAbramov - hm, no specific reason. I just assumed the outgoing module would "clean up" after itself. Is that not how I should be doing that in practice? Should I just fire off a dispatch to clear its data, but leave the reducer? – Adam Rackis Dec 06 '15 at 20:19
  • What is the purpose of "cleaning up"? Usually you'd just keep data around in case user comes back to this page. – Dan Abramov Dec 07 '15 at 00:33
  • @DanAbramov When trying out new things like React I usually try to simulate something *big*, like at my 9-5 job, so I can see how it'd be used work in "real life" (as opposed to ToDo). Things would get out of hand quickly if we kept everything around, from scheduling, to billing, to contacts manager, to tasks, files, etc etc etc. Clearing out certainly a sine qua non for the type of app I'm used to working with -- https://centralreach.com/ if you're curious (baffling why the public-facing site doesn't have screenshots of app - it's quite sharp) – Adam Rackis Dec 07 '15 at 00:48

1 Answers1

10

In general we don’t suggest you to “clean up” the data when changing routes or loading new modules. This makes the application a little less predictable. If we’re talking about hundreds of thousands of records, then sure. Is this the volume of the data you plan to load?

If there’s just a couple of thousands of items on every page, there is no benefit to unloading them, and there are downsides associated with the complexity you add to the application. So make sure you’re solving a real problem, and not optimizing prematurely.

Now, to the warning message. The check is defined inside combineReducers(). It means that unexpected state keys will be discarded. After you removed the bookEntry reducer that managed state.bookEntry, that part of the state was no longer recognized by the new root reducer, and combineReducers() logged a warning that it’s going to be discarded. Note that this is a warning, and not an error. Your code runs just fine. We use console.error() to make warning prominent, but it didn’t actually throw, so you can safely ignore it.

We don’t really want to make the warning configurable because you’re essentially implicitly deleting part of the application state. Usually people do this by mistake, and not intentionally. So we want to warn about that. If you want to get around the warning, your best bet is to write the root reducer (currently generated by combineReducers()) by hand. It would look like this:

// I renamed what you called "root" reducer
// to "main" reducer because the root reducer
// is the combined one.
let mainReducer = (state, action) => ...

// This is like your own combineReducers() with custom behavior
function getRootReducer(dynamicReducer) {
  // Creates a reducer from the main and a dynamic reducer
  return function (state, action) {
    // Calculate main state
    let nextState = {
      main: mainReducer(state.main, action)
    };

    // If specified, calculate dynamic reducer state
    if (dynamicReducer) {
      nextState[dynamicReducer.name] = dynamicReducer.reducer(
        nextState[dynamicReducer.name],
        action
      );
    }

    return nextState;
  };
}

// Create the store without a dynamic reducer
export function createStoreWithoutDynamicReducer() {
  return Redux.createStore(getRootReducer());
}

// Later call this to replace the dynamic reducer on a store instance
export function setDynamicReducer(store, dynamicReducer) {
  store.replaceReducer(getRootReducer(dynamicReducer));
}

However the pattern we recommend is to keep the old reducers around.

Community
  • 1
  • 1
Dan Abramov
  • 264,556
  • 84
  • 409
  • 511
  • Thanks Dan - by law I have to wait 17 more hours to award the bounty :) Thank you SO much for the info. The days of my fighting utilities are long over; I'll gladly leave the old reducers, and simply clear out excessive data if actually needed. Any chance you can see your way to getting some of this info into the docs? This is pretty much the *only* part of redux where the docs are a bit limited. – Adam Rackis Dec 07 '15 at 02:30
  • @Adam I have literally no time to contribute to the docs right now. There's a need for a comprehensive “dynamic code loading” recipe but somebody needs to own that, and I can't. – Dan Abramov Dec 07 '15 at 02:32
  • no worries at all. You've done quite a lot for the dev community. Would you accept a PR with some updated docs on replaceReducer, after I spend some more time with it to wrap my head around things a bit better? – Adam Rackis Dec 07 '15 at 02:40
  • @Adam Yes, but we try to keep API docs concise. Recipe would be a more correct form. – Dan Abramov Dec 07 '15 at 13:07