reduxjs / redux-toolkit

The official, opinionated, batteries-included toolset for efficient Redux development
https://redux-toolkit.js.org
MIT License
10.66k stars 1.16k forks source link

[RED-11] Enhancement: add built-in support for "reducer injection" #1326

Closed markerikson closed 1 year ago

markerikson commented 3 years ago

The standard idiom for code-splitting Redux reducer logic is to call replaceReducer with a re-created root reducer. However, to do that, you have to keep around the original slice reducers, because you have to pass all of the slice reducers to combineReducers every time.

We have a couple recipes for doing this over at https://redux.js.org/usage/code-splitting . There's more complex things you can do as well, such as https://github.com/Microsoft/redux-dynamic-modules , but it's a common enough pattern that it's worth considering just adding a form of that basic implementation directly to our own store.

This also overlaps with the idea of higher-level self-contained "modules" in some ways, such as https://github.com/vmware/slices-for-redux . I'd prefer that we not go overboard here, but it's always worth evaluating a more holistic solution rather than a minimum viable approach.

RED-11

Abdallatif commented 3 years ago

I like the idea. We should also consider adding support for removing a reducer so we can clean the store after a component/module is unmounted.

markerikson commented 3 years ago

One issue here is that this really only works if the user is passing in the individual slice reducers to configureStore. If they are creating the root reducer themselves, such as calling combineReducers separately, then there's nothing we can do.

One potential very limited implementation would be:

Another question is TypeScript behavior. We currently tell users to infer the root state from the store, and the store gets its root state type from the reducers. How would that work here?

Matthew Gerstman has some suggestions at https://www.matthewgerstman.com/tech/redux-code-split-typecheck/ that are worth looking at.

Abdallatif commented 3 years ago

Maybe it should be combineReducers responsibility to add/remove reducers as different projects could implement combineReducers in different ways and maybe with a different data structure. So a high level implementation of it would look like:

function combineReducers(reducers) {
    // prepare reducers
    return {
        combination(state, action) { /** original combine reducers logic */ }, // maybe should just be renamed to just reducer
        addReducer(key, reducer) {},
        removeReducer(key) {},
    }
}
phryneas commented 3 years ago

combineReducers is an upstream dependency and it is unlikely that we are going to change that in redux or export something with the same name but similar functionality.

We could also not just add properties on the function returned by combineReducers as that return value is often wrapped by another function, e.g. by persistReducer which would remove those properties again.

Ephem commented 3 years ago

I think something like addReducer would be a great addition! I think optionally passing in an object at the root level and throwing if addReducer is called without that is a good approach.

While this is a good addition, I dont think it tackles the often hardest part (especially in server rendered apps), where to call addReducer, but as I noted in https://github.com/reduxjs/redux/discussions/4011, I'm not sure that is solvable in a good way inside Redux/React Redux/RTK itself. Even so, addReducer is still a good improvement!

That discussion also links to an older RFC and PR that might be of interest, even though they are based on breaking render purity like all libraries with SSR-support do today.

Ephem commented 3 years ago

Maybe consider adding removeReducer as well? I know some third party libs supports this to unmount big parts of the store when not needed anymore.

markerikson commented 3 years ago

Yeah, I assume if we did addReducer we'd do removeReducer as well.

My working assumption here is that we'd mostly just implement one of the basic snippets from the recipes page just so it's built-in and people don't have to copy-paste it, and they can continue to do something more complex separately if they need to... unless we decide to tackle something much more comprehensive.

Ephem commented 3 years ago

Yeah, there is definitely the question about scope here. Lazy loading/removing middlewares for example? What about nested reducers that are after all an encouraged pattern?

I also think the issue I describe in the linked discussion is a pretty fundamental one and I'd love to finally resolve it somehow with an easy to use API that works with all kinds of React apps. I think that's very worth pursuing, but not sure it's possible under the current React constraints. Might be worth looping in the core team on to see if they are interested/if it's something they want to support in React 18 SSR?

Still, addReducer seems like a good first step with no obvious drawbacks. 😀

MarcieMarc425 commented 2 years ago

Any update on this feature?

markerikson commented 2 years ago

Given that there are no recent comments, nope :)

epeterson320 commented 2 years ago

I'd love to take a shot at this, if you're open to a PR. Could it be implemented as a store enhancer, for an API like in the following example?

import { configureStore, enableDynamicReducers } from '@reduxjs/toolkit';
import cheeseReducer from './cheeseSlice';

const store = configureStore({
  reducer: {
    cheese: cheeseReducer,
  },
  enhancers: [enableDynamicReducers],
});

import('./supremeSlice').then(supremeReducer => {
  store.addReducer(supremeReducer);
  store.removeReducer(cheeseReducer);
});
markerikson commented 2 years ago

That might be one approach, yeah - store enhancers can certainly add more fields to the store object.

I do still have the questions / concerns I raised earlier in the thread, about things like TS type inference and how this would work in cases where you created the root reducer separately.

phryneas commented 2 years ago

On a TS side, something could be implemented like this:

// @filename: enhancer.ts
import { StoreEnhancer, Reducer, AnyAction } from '@reduxjs/toolkit'
export declare function createEnhancer<InjectableState>(): StoreEnhancer<
  {
    addReducer<K extends keyof InjectableState>(key: K, reducer: Reducer<InjectableState[K], AnyAction>): void
    addSlice<K extends keyof InjectableState>(slice: { name: K, reducer: Reducer<InjectableState[K], AnyAction> }): void
  }, 
  Partial<InjectableState>
>

// @filename: store.ts
import { createEnhancer } from './enhancer'
import { configureStore } from '@reduxjs/toolkit'

export interface InjectedState {}

const enhancer = createEnhancer<InjectedState>()
export const store = configureStore({
  reducer: () => ({
    defaultSlice: {}
  }),
  enhancers: defaultEnhancers => defaultEnhancers.concat(enhancer)
})

// @filename: slice1.js
declare module './store' {
  export interface InjectedState {
    slice1: { name: string }
  }
}

export const slice = createSlice(...)

// @filename: slice2.ts
declare module './store' {
  export interface InjectedState {
    slice2: { count: number }
  }
}

export const slice = createSlice(...)

// @filename: usage.ts

import { store } from './store'
import { slice as slice1 } from './slice1'

store.addReducer('slice1', slice1.reducer)
// or
store.addSlice(slice1)

The point here would be to have that central InjectedState interface in one file and using module augmentation to add more and more reducer types to that. Of course, injected reducer slices would always stay | undefined. It's pretty much impossible to make that type available after injection.

We could think about some api like

const useSelectorForSlice1 = createFunctionWithInjection(useSelector, 'slice1', slice1.reducer)
// or
const useSelectorForSlice1 = createFunctionWithInjection(useSelector, slice1)

that would internally make sure that the reducer is already injected every time useSelectorForSlice1 is called and then return useSelector with adjusted types to allow access to the slice without type-checking.

markerikson commented 2 years ago

FWIW, there's actually a discussion over in the main repo about relying on Suspense to help make sure that a reducer is injected before we ever try to read from it:

https://github.com/reduxjs/redux/discussions/4114

haven't tried it in practice, but should work.

epeterson320 commented 2 years ago

For the instance where combineReducers() is called outside of configure store, I see two options:

  1. Throw an error when a reducer is added, like you said.
  2. Enhance combineReducers() to add a symbol-keyed property on the combined reducer to keep track of its reducer functions. This would mean redux core now has to export a reducers symbol. Then RTK can test for that property and turn combined reducers back into their child reducers, if they were created with combineReducers().

So redux/combineReducers.ts would be:

const reducers = Symbol('reducers')

export default function combineReducers(reducers: ReducersMapObject) {
  const finalReducers: ReducersMapObject = {}

  // ...

 function combination(
    state: StateFromReducersMapObject<typeof reducers> = {},
    action: AnyAction
  ) {
    // ...
    return hasChanged ? nextState : state
  }
  combination[reducers] = finalReducers
  return combination
}

As far as Typescript patterns go, I like the strategy from Matthew Gerstman about importing all the reducers' types into a type-only root file, but I think it's also viable to have each slice file export its own typed dispatch and selector hooks, even though they're all going to be the same thing at runtime. One could also define useAppSelector(slices, selector) and useAppDispatch(slices) hooks that inject any missing slices in effects and return the correctly typed values. So I think there are a few options for getting good types.

markerikson commented 1 year ago

Related note: https://github.com/Microsoft/redux-dynamic-modules doesn't appear to be maintained, and https://github.com/ioof-holdings/redux-dynostore has been dead for a while. That repo also links to a few other related libs:

of those only redux-injectors has a release within the last 2 years.

https://github.com/fostyfost/redux-eggs seems to be the most recent "dynamically add Redux logic" lib I've seen.

markerikson commented 1 year ago

And a tweet poll on RTK / code-splitting usage:

markerikson commented 1 year ago

FYI, the new combineSlices API in 2.0-beta is meant to support this: