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

Expose currentReducer from store through getReducer() #4107

Closed
smeng9 opened this issue Jun 12, 2021 · 11 comments
Closed

Expose currentReducer from store through getReducer() #4107

smeng9 opened this issue Jun 12, 2021 · 11 comments

Comments

@smeng9
Copy link

smeng9 commented Jun 12, 2021

This is not a duplicate of #1246 because we have a different type of use case

New Features

I would like to get the currentReducer using the store.getReducer()

What is the new or updated feature that you are suggesting?

The createStore in https://github.com/reduxjs/redux/blob/master/src/createStore.ts will have a new method

function getReducer(): Reducer<S, A> {

  return currentReducer as Reducer<S, A>;

}

and get exposed in store object

Why should this feature be included?

Our project uses a closed source proprietary library which initializes the redux context for us.
However we would like to override some part of the reducer logic, but we cannot modify the source because we don't have it or use other libraries to do it.

Since we cannot initialize the reducer by ourselves, we can only resort to extracting the reducer from the store. This is a different situation than #1246 as they have the full access of the reducer when passing into createStore.
So we would like to add this method to get the currentReducer out, and wrap it to change some behavior, and pass it back to replaceReducer.

What docs changes are needed to explain this?

We will need to modify the Store Methods part of the document to explain the new method https://redux.js.org/api/store#store-methods

Also I can add some of the example code in the documentation to illustrate how to use together with replaceReducer to override some of the reducer logic.

@markerikson
Copy link
Contributor

I'm not quite sure I understand the specific use case you're describing.

When you say "initializes the Redux context for us", do you actually mean a Redux store? As in, the other lib creates the store internally and gives it to you, and thus you never have a chance to see what the reducer is?

@smeng9
Copy link
Author

smeng9 commented Jun 12, 2021

Yes we are not able to see the source code of the reducer. But we would like to override some behavior of the reducer.

@timdorr
Copy link
Member

timdorr commented Jun 12, 2021

The problem is the reducer is only half the issue there. There may be middleware or enhancers also applied to the store, which can significantly alter its behavior before ever getting to the reducer function.

@smeng9
Copy link
Author

smeng9 commented Jun 12, 2021

That's true, but the store is already created, so we cannot add the enhancer before hand

@markerikson
Copy link
Contributor

I can see at least two potential use cases for having this:

  • Wanting to do reducer replacement for code-splitting
  • Wanting to wrap the original reducer to add additional behavior

Agreed that this doesn't take middleware into account at all, but then again that's not something you can customize later even if you did make the store originally.

Obviously this is trivial at an implementation level - just return reducer.

I'm always a bit hesitant when it comes to adding things to the public API at this point, but I can't immediately think of any reasons why this would be a bad idea exactly.

@markerikson
Copy link
Contributor

It's worth noting that we actually did have getReducer() once upon a time. By which I mean for the first couple months of Redux's existence :) It was there in 1.0, but removed in 2.0 because React-Redux no longer relied on it:

@wesleygrimes
Copy link

Something like this would be great for code-splitting. I have been hacking around this afternoon to find a clean way to load a feature slice when a lazy loaded component is routed to. Here's something I have come up with so far. This specific part of the code could benefit from having getCurrentReducer when called replaceReducer. I will try and get an example repo pushed up soon, but here's a screen shot:

carbon (1)

@markerikson
Copy link
Contributor

Hmm. I'm actually a bit confused by this piece of the snippet:

useEffect(() => {
  const existingReducers = store.getCurrentReducer();
  const updatedReducers = {...existingReducers, ...{ [slice.name]: slice.reducer}};
  store.replaceReducer(combineReducers(updatedReducers);
}, [slice, store])

I realize this is effectively pseudocode because store.getReducer() doesn't even exist yet :) But there's a problem with this bit conceptually.

The underlying Redux store takes a reducer function. Once that function has been created, there's no way to figure out how it might have been created in the first place, and what the store itself tracks is a reference to that function.

This snippet is written assuming that the store is tracking an object full of slice reducer functions, which isn't what it does.

Per the Code Splitting page in the docs, this is why it's necessary / recommended to keep around an object with the original slice reducers for later reuse, so you can re-create the root reducer using those same slice reducers as the input.

But, there's no way for store.getReducer() to return those slice reducers by itself - the fact that those ever existed is gone by the time the store gets its hands on the root reducer.

So, in that sense, I don't see how having a store.getReducer() function helps here for this use case.

@wesleygrimes
Copy link

Thanks for the feedback Mark. I’ll dig back into the code and see if I can find another solution. I’m fairly new to react, been knee deep in Angular/NgRx for years.

@markerikson
Copy link
Contributor

Sure, no problem! :) And feel free to ping me or the other maintainers over in Reactiflux if you'd like to chat in more detail there.

@smeng9
Copy link
Author

smeng9 commented Jun 13, 2021

Thanks for the feedback Mark. I’ll dig back into the code and see if I can find another solution. I’m fairly new to react, been knee deep in Angular/NgRx for years.

@wesleygrimes Yeah you will get the whole reducer function out from the store, and you will need to wrap it as another function. We cannot unpack the current reducer.

@smeng9 smeng9 changed the title Expose currentReducer from store through getCurrentReducer() Expose currentReducer from store through getReducer() Jul 25, 2021
@timdorr timdorr closed this as completed Jul 26, 2021
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

Successfully merging a pull request may close this issue.

4 participants