redux-toolkit icon indicating copy to clipboard operation
redux-toolkit copied to clipboard

Enhancement: add built-in support for "reducer injection"

Open markerikson opened this issue 4 years ago • 16 comments

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.

markerikson avatar Jul 19 '21 14:07 markerikson

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.

Abdallatif avatar Jul 19 '21 14:07 Abdallatif

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:

  • If the user calls configureStore and passing in an object of slice reducers, keep those around
  • If they call addReducer later, add it and re-create the root reducer
  • If they passed in an initial reducer function instead, throw an error when they call addReducer because we don't have the data needed to do that

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.

markerikson avatar Jul 19 '21 14:07 markerikson

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) {},
    }
}

Abdallatif avatar Jul 19 '21 15:07 Abdallatif

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.

phryneas avatar Jul 19 '21 15:07 phryneas

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 avatar Jul 19 '21 17:07 Ephem

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.

Ephem avatar Jul 19 '21 18:07 Ephem

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.

markerikson avatar Jul 19 '21 18:07 markerikson

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. 😀

Ephem avatar Jul 19 '21 18:07 Ephem

Any update on this feature?

MarcieMarc425 avatar Apr 27 '22 10:04 MarcieMarc425

Given that there are no recent comments, nope :)

markerikson avatar Apr 27 '22 15:04 markerikson

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);
});

epeterson320 avatar Aug 26 '22 22:08 epeterson320

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.

markerikson avatar Aug 27 '22 01:08 markerikson

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.

phryneas avatar Aug 27 '22 03:08 phryneas

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.

markerikson avatar Aug 27 '22 16:08 markerikson

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.

epeterson320 avatar Sep 19 '22 16:09 epeterson320

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:

  • https://www.npmjs.com/package/redux-injectors
  • https://www.npmjs.com/package/redux-injector
  • https://www.npmjs.com/package/redux-reducers-injector
  • https://www.npmjs.com/package/redux-sagas-injector
  • https://www.npmjs.com/package/paradux

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 avatar Oct 14 '22 20:10 markerikson

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

  • https://twitter.com/acemarke/status/1580991161358962688

markerikson avatar Jan 09 '23 00:01 markerikson

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

  • https://github.com/reduxjs/redux-toolkit/releases/tag/v2.0.0-beta.0

markerikson avatar Aug 16 '23 16:08 markerikson