reduxjs / redux

A JS library for predictable global state management
https://redux.js.org
MIT License
60.51k stars 15.26k forks source link

updating redux from 5.0.0 to 5.0.1 Typescript errors #4648

Closed JustFly1984 closed 4 months ago

JustFly1984 commented 4 months ago

I have a bunch of thunks, updating redux from 5.0.0 to 5.0.1 triggers a bunch of errprs on dispatch:

Argument of type '(dispatch: ((action: Action<"listenerMiddleware/add">) => UnsubscribeListener) & Dispatch<UnknownAction>, getState: () => { ...; }) => Promise<...>' is not assignable to parameter of type 'UnknownAction'.

reverting it back to 5.0.0 helps.

I already use @redux/toolkit, useAppState and useAppDispatch. What else could be an issue?

markerikson commented 4 months ago

That seems surprising - the changes in 5.0.1 shouldn't have any effect on the type of dispatch. Could you share a project that reproduces this issue?

JustFly1984 commented 4 months ago

@markerikson Sorry I can't share the project, It is under NDA. The only I can say it uses middleware:

import { localStorageMiddleware } from 'redux/middlewares/local-storage-middleware'

and latest packages:

    "react-redux": "9.0.4",
    "redux": "5.0.0",
    "redux-thunk": "3.1.0",

all thunks type signatures:

 export function getCart() {
  return async function thunk(
    dispatch: AppDispatch,
    getState: GetState
  ): Promise<void> { ... }

Updating redux@5.0.1 breaks typescript. The case where husky+lint-staged saved the day for me.

markerikson commented 4 months ago

What happens if you remove the manual typings of dispatch and getState from this usage, and instead use the ThunkAction or AppThunk types shown in https://redux.js.org/usage/usage-with-typescript#type-checking-redux-thunks ?

JustFly1984 commented 4 months ago

I see an issue at store initialization:

export const store = configureStore({
  reducer: rootReducer,
  middleware: (getDefaultMiddleware) =>
    getDefaultMiddleware().concat(localStorageMiddleware),
  devTools: GATSBY_ENV !== 'production',
})

export type AppDispatch = typeof store.dispatch

AppDispatch here is inferred from the middleware as

type AppDispatch = ((action: Action<"listenerMiddleware/add">) => UnsubscribeListener) & Dispatch<UnknownAction>

and down the line where I'm using dispatch with thunks I'm getting following error:

o overload matches this call.
  Overload 1 of 2, '(action: Action<"listenerMiddleware/add">): UnsubscribeListener', gave the following error.
    Argument of type '(dispatch: ((action: Action<"listenerMiddleware/add">) => UnsubscribeListener) & Dispatch<UnknownAction>, getState: () => { ...; }) => Promise<...>' is not assignable to parameter of type 'Action<"listenerMiddleware/add">'.
  Overload 2 of 2, '(action: UnknownAction, ...extraArgs: any[]): UnknownAction', gave the following error.
    Argument of type '(dispatch: ((action: Action<"listenerMiddleware/add">) => UnsubscribeListener) & Dispatch<UnknownAction>, getState: () => { ...; }) => Promise<...>' is not assignable to parameter of type 'UnknownAction'.ts(2769)
markerikson commented 4 months ago

Can you show me both the revised definition of that getCart thunk with the AppThunk usage, as well as the code for localStorageMiddleware?

EskiMojo14 commented 4 months ago

i will say - it's odd that your dispatch has the listener middleware overload when it isn't in your store setup 😕

JustFly1984 commented 4 months ago

I've simplified it a bit, but I have a recursion issue here:

export const store = configureStore({
  reducer: rootReducer,
  middleware: function middleware(getDefaultMiddleware) {
    const listenerMiddleware = createListenerMiddleware()

    const allowedActions = isAnyOf(
      fetchSuccess,
      toggleSentAsGift,
      deleteRemovedItem,
      addRemovedItem,
      addCartItem,
      updateCartItemQuantity,
      resetCart
    )

    listenerMiddleware.startListening({
      matcher: allowedActions,
      effect: (_, listenerApi) => {
        const cartState = listenerApi.getState().cart

        CartService.setCartToLocalStorage(cartState.data)
      },
    })

    return getDefaultMiddleware().concat(listenerMiddleware.middleware)
  },
  devTools: GATSBY_ENV !== 'production',
})

export type AppDispatch = typeof store.dispatch

export type GetState = typeof store.getState

export type ReduxState = ReturnType<typeof store.getState>
// Use throughout your app instead of plain `useDispatch` and `useSelector`
export const useAppDispatch: () => AppDispatch = useDispatch
export const useAppSelector: TypedUseSelectorHook<ReduxState> = useSelector

createListenerMiddleware expects ReduxState and AppDispatch as arguments, but passing it causes store to be inferred as any.

Screenshot 2566-12-30 at 19 10 51
EskiMojo14 commented 4 months ago

don't set up your listener middleware inside your middleware callback, do it beforehand.

const listenerMiddleware = createListenerMiddleware();

const allowedActions = isAnyOf(
  fetchSuccess,
  toggleSentAsGift,
  deleteRemovedItem,
  addRemovedItem,
  addCartItem,
  updateCartItemQuantity,
  resetCart,
);

listenerMiddleware.startListening({
  matcher: allowedActions,
  effect: (_, listenerApi) => {
    const cartState = listenerApi.getState().cart;

    CartService.setCartToLocalStorage(cartState.data);
  },
});

export const store = configureStore({
  reducer: rootReducer,
  middleware: function middleware(getDefaultMiddleware) {
    return getDefaultMiddleware().concat(listenerMiddleware.middleware);
  },
  devTools: GATSBY_ENV !== "production",
});
EskiMojo14 commented 4 months ago

we also don't tend to recommend passing those generics when creating the middleware - instead, see the recommendations here.

JustFly1984 commented 4 months ago

@EskiMojo14 I've reduced the code and it seems that the difference in 5.0.0 vs 5.0.1 is in typeof store.dispatch

Before update it infers:

interface dispatch ((action: Action<"listenerMiddleware/add">) => UnsubscribeListener) & ThunkDispatch<ReduxState, undefined, UnknownAction> & Dispatch<UnknownAction>

but after update it infers incorrectly as

interface dispatch ((action: Action<"listenerMiddleware/add">) => UnsubscribeListener) & Dispatch<UnknownAction> so useAppDispatch in components produces errors.

EskiMojo14 commented 4 months ago

any chance you could put together a reproduction of this, in Typescript Playground or Codesandbox, etc?

JustFly1984 commented 4 months ago

@EskiMojo14

I've reproduced it in my testing repository, I will give you an access.

In general, this is the types I get inferred for AppDispatch with 5.0.1:

Screenshot 2567-01-04 at 01 43 56

and this is the types I get inferred AppDispatch with 5.0.0:

Screenshot 2567-01-04 at 01 40 48
JustFly1984 commented 4 months ago

@EskiMojo14 I've Invited you to https://github.com/JustFly1984/ecommerce-frontend/invitations - please checkout the branch refactoring, toggle last two commits to switch redux versions. run nvm use && yarn and navigate to src/redux/store.ts Please ping me then it is ok to remove your access. Please delete the fork after debugging.

EskiMojo14 commented 4 months ago

@JustFly1984 can't fully explain it, but the issue is solved by adding a forced resolution to ^v5. It seems that some of your dependencies still have a dependency on redux v4 (which you can see by running yarn why redux) which seems to be messing with the types somehow.

I will also note that you have an unnecessary dependency on @types/react-redux, as react-redux has shipped its own types since v8.

if that's a satisfactory answer/workaround, please feel free to remove my access and I'll delete my local copy :)

JustFly1984 commented 4 months ago

@EskiMojo14 enforcing redux@5.0.1 in package.json resolutions, and removing outdated @types/react-redux helped.

I have one more question: I've personally never used it, but I found https://github.com/reduxjs/redux-thunk?tab=readme-ov-file#composition. Looks like it is possible to return values from the dispatch(thunkFunc()), and if it is a promise, possible to await it, but I do not see any return value inferred from dispatch. Is it something outdated? How to type return value of dispatch correctly?

EskiMojo14 commented 4 months ago

yes, you can return values from a thunk - it's just whatever the function returns. see Type Checking Redux Thunks.

that should be inferred by the ThunkDispatch type, if everything's working correctly

image

JustFly1984 commented 4 months ago

Thank you @EskiMojo14, please consider an issue resolved.

EskiMojo14 commented 4 months ago

great! i did it 2 days ago but just confirming in writing i deleted my local copy of your repo :)