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

[Typings BUG] preloadedState and redux-persist compatibility issues. #4473

Closed
Qwin opened this issue Jun 20, 2024 · 14 comments
Closed

[Typings BUG] preloadedState and redux-persist compatibility issues. #4473

Qwin opened this issue Jun 20, 2024 · 14 comments

Comments

@Qwin
Copy link

Qwin commented Jun 20, 2024

I been at this the whole day yesterday and one of the things I am trying to achieve is having both redux-persist working with the mentioned way to configure store to use preloadedState for unit tests.

Doing the following will work fine (no errors):

 export const setupStore = (preloadedState?: Partial<RootState>) => {
  return configureStore({
    middleware: getDefaultMiddleware => {

      const middleware = getDefaultMiddleware({
        immutableCheck: { warnAfter: 128 },
        serializableCheck: {
          warnAfter: 128,
          ignoredActions: [FLUSH, REHYDRATE, PAUSE, PERSIST, PURGE, REGISTER],
        },
        // prepend listenerMiddleware as its has functions and serializable middleware would conflict as per docs
      }).prepend(listenerMiddleware.middleware);

      if (!process.env.JEST_WORKER_ID && __DEV__) {
        middleware.concat(customLogger as Middleware);
      }

      return middleware;
    },
    devTools: process.env.NODE_ENV !== "production",
    reducer: rootReducer,
    preloadedState: preloadedState,
  });
}

export const store = setupStore();

export type RootState = ReturnType<typeof rootReducer>;

export type AppStore = ReturnType<typeof setupStore>;

However as soon as I add redux-persist in the middle like so (and I have tried various variations):

const persistedReducer = persistReducer<ReturnType<typeof rootReducer>>(rootPersistConfig, rootReducer);

export const setupStore = (preloadedState?: Partial<RootState>) => {
  return configureStore({
    middleware: getDefaultMiddleware => {

      const middleware = getDefaultMiddleware({
        immutableCheck: { warnAfter: 128 },
        serializableCheck: {
          warnAfter: 128,
          ignoredActions: [FLUSH, REHYDRATE, PAUSE, PERSIST, PURGE, REGISTER],
        },
        // prepend listenerMiddleware as its has functions and serializable middleware would conflict as per docs
      }).prepend(listenerMiddleware.middleware);

      if (!process.env.JEST_WORKER_ID && __DEV__) {
        middleware.concat(customLogger as Middleware);
      }

      return middleware;
    },
    devTools: process.env.NODE_ENV !== "production",
    reducer: persistedReducer,
    preloadedState: preloadedState,
  });
}

export const store = setupStore();

export const persistor = persistStore(store);

export type RootState = ReturnType<typeof rootReducer>;

export type AppStore = ReturnType<typeof setupStore>;

both preloadedState and middleware give an error out. This is due to reducer setting type as to RootState and causing a preloadedState to be RootState which then is trying to add a Partial (preloadedState parameter) to the preloadedState property that is of type RootState.

I get the following error from preloadedState

Types of property 'account' are incompatible.
Type 'AccountStateInterface | undefined' is not assignable to type 'AccountStateInterface'.
Type 'undefined' is not assignable to type 'AccountStateInterface'

Is there no way to get redux-persist working properly with RTK 2.x with preloadedState, it works fine without preloadedState? I know there is a remember enhancer which I could utilize and is compatible rtk 2.x however I wouldn't want to make a drastic change like that if I can help it.

things I have tried:

  • patching the redux-persist lib to add _persist to typings
  • changing RootState to be of typeof persistReducer
  • removing some my middleware where its only getDefaultMiddleware (this is not the issue although it errors out, if I add preloadedState, it just does that if I am setting a wrong type on preloadedState)
  • changing the order of middleware property to the top (yes I heard it matters)

I am using latest:
"@reduxjs/toolkit": "^2.2.5"
"redux-persist": "^6.0.0"

Why I created this issue instead of stackoverflow is because I think this is a bug, the preloadedState should not be affected by what type I set on reducer, I know it maps to S template and S template is set on preloadedState property but I should have more control over what type is set on preloadedState manually. Also it doesn't make sense to me that when an error arises from preloadedState the middleware property also errors out.

@EskiMojo14
Copy link
Collaborator

@bylly1
Copy link

bylly1 commented Jun 25, 2024

see rt2zz/redux-persist#1459 (comment)

I added this to my store.ts file:

declare module "redux-persist" {
	export function persistReducer<S, A extends Action = Action, P = S>(
		config: PersistConfig<S>,
		baseReducer: Reducer<S, A, P>,
	): Reducer<S & {_persist: PersistState}, A, P & {_persist?: PersistState}>;
}

but still not working... where should I add this

@EskiMojo14 EskiMojo14 reopened this Jun 26, 2024
@EskiMojo14
Copy link
Collaborator

could you put together a reproduction? @bylly1
that should be all you need

@bylly1
Copy link

bylly1 commented Jun 26, 2024

could you put together a reproduction? @bylly1 that should be all you need

yes, of course. I will add all informations needed:

package.json

{
  "dependencies": {
    ...
    "@reduxjs/toolkit": "^2.2.5",
    "react": "^18.2.0",
    "react-dom": "^18.2.0",
    "react-redux": "^9.1.2",
    "redux-persist": "^6.0.0",
    ...
  },
}

redux/store.ts

import { configureStore, combineReducers } from '@reduxjs/toolkit';
import { setupListeners } from '@reduxjs/toolkit/query';
import { persistStore, persistReducer, FLUSH, REHYDRATE, PAUSE, PERSIST, PURGE, REGISTER } from 'redux-persist';
import storage from 'redux-persist/lib/storage';
// core
import api from './api';
import { rtkQueryErrorLogger } from './middleware';

const rootReducer = combineReducers({
    [api.reducerPath]: api.reducer,
});

const persistConfig = {
    key: 'test',
    storage,
};

const persistedReducer = persistReducer(persistConfig, rootReducer);

const store = configureStore({
    reducer: persistedReducer,
    devTools: import.meta.env.NODE_ENV !== 'production',
    middleware: (getDefaultMiddleware) =>
        getDefaultMiddleware({
            serializableCheck: {
                ignoredActions: [FLUSH, REHYDRATE, PAUSE, PERSIST, PURGE, REGISTER],
            },
            immutableCheck: true,
        }).concat(api.middleware, rtkQueryErrorLogger),
});

setupListeners(store.dispatch);

export type AppDispatch = typeof store.dispatch;
export type RootState = ReturnType<typeof store.getState>;

export const persistor = persistStore(store);
export default store;

I tried first to add this inside store.ts but didn t work

declare module 'redux-persist' {
    export function persistReducer<S, A extends Action = Action, P = S>(
        config: PersistConfig<S>,
        baseReducer: Reducer<S, A, P>
    ): Reducer<S & { _persist: PersistState }, A, P & { _persist?: PersistState }>;
}

then I tried to create a separate file named redux-persist.d.ts and add to tsconfig.json but still not working. I don t know what method to try

@EskiMojo14
Copy link
Collaborator

@bylly1 did you import the types you need? PersistConfig and PersistState need to be imported from redux persist, and Reducer and Action need to be imported from RTK.

@bylly1
Copy link

bylly1 commented Jun 26, 2024

@bylly1 did you import the types you need? PersistConfig and PersistState need to be imported from redux persist, and Reducer and Action need to be imported from RTK.

I followed the instructions from redux-persist post

import type { Action, Reducer } from "redux"; 
import type { PersistConfig, PersistState } from "redux-persist";

@aryaemami59
Copy link
Contributor

@EskiMojo14 What if we add this instead?

  • In redux-persist.d.ts:

    import type { Action, Reducer } from "redux"
    import type { PersistConfig, PersistState } from "redux-persist"
    
    declare module "redux-persist" {
      export function persistReducer<S, A extends Action = Action, P = S>(
        config: PersistConfig<S>,
        baseReducer: Reducer<S, A, P>
      ): Reducer<
        S & { _persist: PersistState },
        A,
        Partial<P> & { _persist?: PersistState }
      >
    }
  • Diff

    import type { Action, Reducer } from "redux"
    import type { PersistConfig, PersistState } from "redux-persist"
    
    declare module "redux-persist" {
      export function persistReducer<S, A extends Action = Action, P = S>(
      config: PersistConfig<S>,
      baseReducer: Reducer<S, A, P>
      ): Reducer<
      S & { _persist: PersistState },
      A,
    - P & { _persist?: PersistState }
    + Partial<P> & { _persist?: PersistState }
      >
    }
    

@EskiMojo14
Copy link
Collaborator

@aryaemami59 that wouldn't be accurate, not all reducers accept a partial preloaded state, and the ones that do should infer it correctly from the original reducer.

@Qwin
Copy link
Author

Qwin commented Jun 26, 2024

@EskiMojo14 I have the same issues that @bylly1 is having, no matter what I do the mismatch of typings is not working even with defining the typings file to overwrite the redux-persist module.

@aryaemami59
Copy link
Contributor

@EskiMojo14 In that case I think the issue is since we are explicitly passing in a generic type parameter here:

const persistedReducer = persistReducer<ReturnType<typeof rootReducer>>(rootPersistConfig, rootReducer);

the other generic type parameters don't get inferred because (correct me if I'm wrong) TypeScript doesn't do partial inference for generic type parameters, so if we remove the explicit type parameter:

- const persistedReducer = persistReducer<ReturnType<typeof rootReducer>>(rootPersistConfig, rootReducer);
+ const persistedReducer = persistReducer(rootPersistConfig, rootReducer);

the problem goes away.

@EskiMojo14
Copy link
Collaborator

i missed that @Qwin was doing that - yes, none of those generics should be annotated manually.

@aryaemami59
Copy link
Contributor

@Qwin If you make the changes mentioned in this comment the problem should go away. Can you check and see if that fixes the issue you're having?

@Qwin
Copy link
Author

Qwin commented Jun 27, 2024

I FINALLY GOT IT TO WORK!!!

Here is what I did:

  1. As mentioned by @EskiMojo14 and @aryaemami59 I had to remove the manual setting of generics on persistReducer (however that didnt solve it, it did get me closer)

  2. next thing I had to do due to an error with persistReducer not knowing anymore what the state should be, I had to specify it specifically on the rootConfig like so:

const rootPersistConfig: PersistConfig<ReturnType<typeof rootReducer>, any, any, any> = {
  key: "root",
  storage: AsyncStorage,
};
  1. Lastly I had to modify the type for persistReducer to:
declare module 'redux-persist' {
    export function persistReducer<S, A extends Action = Action, P = S>(
        config: PersistConfig<S>,
        baseReducer: Reducer<S, A, P>
    ): Reducer<S & { _persist: PersistState }, A, P & { _persist?: PersistState }>;
}

The reason why this was failling is due to mismatching between rootReducer and rootConfig, when rootConfig is defined without explicit type like the screenshot above, the first "S" state template parameter will be set to unknown, however rootReducer is set to the actuall RootState. when persistReducer tries to figure it out by using the 2 parameters, it sees unknown and a RootState and doesnt really know which one to choose.

I am still not sure why it cant prioritize the RootState = S instead of unknown from RootConfig, but I guess thats typescript dark magic. Maybe it has to do with the typescript version I am using ? Let me know if the full code works for you guys and if it also works if you remove the typings on the rootPersistConfig.

here is the full code:

declare module "redux-persist" {
	export function persistReducer<S, A extends Action = Action, P = S>(
		config: PersistConfig<S>,
		baseReducer: Reducer<S, A, P>,
	): Reducer<S & {_persist: PersistState}, A, P & {_persist?: PersistState}>;
}

const rootPersistConfig: PersistConfig<ReturnType<typeof rootReducer>, any, any, any> = {
  key: "root",
  storage: AsyncStorage,
};

const persistedReducer = persistReducer(rootPersistConfig, rootReducer);

export const setupStore = (preloadedState?: Partial<RootState>) => {
  return configureStore({
    middleware: getDefaultMiddleware => {

      const middleware = getDefaultMiddleware({
        immutableCheck: { warnAfter: 128 },
        serializableCheck: {
          warnAfter: 128,
          ignoredActions: [FLUSH, REHYDRATE, PAUSE, PERSIST, PURGE, REGISTER],
        },
        // prepend listenerMiddleware as its has functions and serializable middleware would conflict as per docs
      }).prepend(listenerMiddleware.middleware);

      if (!process.env.JEST_WORKER_ID && __DEV__) {
        middleware.concat(customLogger as Middleware);
      }

      return middleware;
    },
    devTools: process.env.NODE_ENV !== "production",
    reducer: persistedReducer,
    preloadedState: preloadedState,
  });
}

export const store = setupStore();
export const persistor = persistStore(store);

export type RootState = ReturnType<typeof rootReducer>;
export type AppStore = ReturnType<typeof setupStore>;
export type AppDispatch = AppStore['dispatch'];

I think this issue will help a lot of people getting redux-persist and redux-toolkit 2.x with preloadedState support working correctly :)

BIG THANK YOU!

@EskiMojo14
Copy link
Collaborator

ah yeah, annotating the config will definitely help - annoyingly with TS inference, order often matters, which is why I think it's odd redux-persist takes the config first

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