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

Feature: expose the reducers mapping object supplied to combineReducers() #2613

Closed
jmar777 opened this issue Sep 18, 2017 · 9 comments
Closed

Comments

@jmar777
Copy link

jmar777 commented Sep 18, 2017

The reducer returned from combineReducers() is currently an opaque function with respect to the original reducers that comprise it. While this is typically desirable, there may be benefits to allowing enhancers to perform some introspection.

Example Use Case:

Several store enhancers currently exist [1] that allow "slices" of state to be dynamically added and removed from the store. Related behavior has also been provided by Dan Abramov[2].

If the initial reducers object was attached to combination [3] (we'll assign it to the _reducers property for this example), the functionality described above could trivially be achieved using a store enhancer:

let initialReducers;
let store;

exports.enhancer = createStore => (reducer, preloadedState, enhancer) => {
  store = createStore(reducer, preloadedState, enhancer);
  initialReducers = reducer._reducers;

  return store;
};

exports.addSlice = (key, reducer) => {
  store.replaceReducer(combineReducers({
    ...initialReducers,
    [key]: reducer
  });

  store.dispatch(/* something to "wake up" the new reducer */);
};

Why not just create another module that does this?

I wholly respect that combineReducers() is neither intended nor presented as the one-true approach to slice management, but I also support paving cowpaths. This is a two-line addition with no backwards compatibility concerns or API changes relevant to regular usage. In exchange, it opens the door to some useful reducer manipulation, including patterns like the one in the example that assist with both code organization and code splitting. The benefit of including this in redux core vs. a new module is the increased interoperability of these enhancers, as they have a consistent and ubiquitously used property to target (i.e., _reducers).

Thoughts?

[1] redux-inject-reducer, paradux, redux-dynamix, redux-react-dynamic-store, ReducerRegistry
[2] https://stackoverflow.com/questions/32968016/how-to-dynamically-load-reducers-for-code-splitting-in-a-redux-application/33044701#33044701
[3] https://github.com/reactjs/redux/blob/master/src/combineReducers.js#L132

@jmar777
Copy link
Author

jmar777 commented Sep 18, 2017

For the sake of illustrating the actual modification I'm suggesting, it would look something like this:

// change this:
return function combination(state = {}, action) { ... }

// to this:
const combination = function(state = {}, action) { ... }
combination._reducers = reducers

return combination

https://github.com/reactjs/redux/blob/master/src/combineReducers.js#L132

@markerikson
Copy link
Contributor

Busy atm, but at first glance I think I can get behind this. We've got prior precedent with connect exposing WrappedComponent. (Also, as a link-centric person, I definitely appreciate you pulling together relevant resources for this topic :) )

@jmar777
Copy link
Author

jmar777 commented Sep 19, 2017

A few topics for discussion if anyone has opinions:

  1. Any better property names than _reducers? Looks like the _ prefixing isn't a common convention in the redux source, so perhaps one of reducers, originalReducers, initialReducers, or reducerMap?
  2. Would it be better to retain a reference to finalReducers , rather than the unpruned reducers argument itself?
  3. Would it be better to store a copy of the reducers, rather than the initial reference? I can't think of why someone would try to mutate the proposed property, but if they did, this would at least prevent it from borking the behavior of the current reducer instance.

I'm currently leaning towards 1) originalReducers, 2) yes, and 3) yes. So:

combination._reducers = reducers

... from my example would turn into:

combination.originalReducers = { ...finalReducers }.

@timdorr
Copy link
Member

timdorr commented Sep 19, 2017

It would seem you can do this with a higher-order/enhancer function:

function combineAndSaveReducers(reducers) {
  const combinedReducers = combineReducers(reducers)
  combinedReducers.reducers = reducers
  return combinedReducers
}

@jmar777
Copy link
Author

jmar777 commented Sep 19, 2017

@timdorr This is what I'm currently doing. I'm just advocating that the resulting utility benefits make this a compelling core feature.

Without a standard target included in core, multiple enhancers trying to introspect against the reducers object will face interoperability issues. IMHO, it's a pretty powerful enabler for being such a simple expando addition.

@timdorr
Copy link
Member

timdorr commented Sep 19, 2017

It doesn't seem like a necessary feature to me. You can either use a HoF like that, or you can simply collect the input to combineReducers before running the function and store it in a singleton or some other form:

let currentReducers
export function buildCombineReducers(reducers) {
  currentReducers = reducers
  return combineReducers(reducers)
}

export const getCurrentReducers = () => currentReducers

I don't think we need to add this to the core library.

@timdorr timdorr closed this as completed Sep 19, 2017
@jmar777
Copy link
Author

jmar777 commented Sep 19, 2017

@timdorr I agree that the workaround is trivial for a single enhancer. The problem is when multiple enhancers (especially from independent module developers) need to introspect, they have nothing to coordinate against.

combineReducers isn't composable in the sense needed here, so you can't use independent HoF's from both module-a and module-b (at least not with a reasonable level of API ergonomics).

Interoperability between independent enhancers requires a common target, and it's unlikely to see that emerge outside of core (as evidenced by the present lack of a common target, despite there already being substantial community activity in this area).

@jmar777
Copy link
Author

jmar777 commented Sep 25, 2017

@timdorr Any chance you could weigh in on the merits (or lack thereof) for this being an interoperable solution? I think that's the biggest limitation of simple HoF-based solutions.

A secondary advantage would be that libraries like this could be enhancer-only, rather than enhancer + custom reducer.

@wtgtybhertgeghgtwtg
Copy link

While it is trivial, I think it would be nice to have an official way to access the current reducers. It'd help when writing redux libraries regarding dynamic reducers, at least.

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

No branches or pull requests

4 participants