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

updating redux from 5.0.0 to 5.0.1 Typescript errors #4648

Closed
JustFly1984 opened this issue Dec 29, 2023 · 18 comments
Closed

updating redux from 5.0.0 to 5.0.1 Typescript errors #4648

JustFly1984 opened this issue Dec 29, 2023 · 18 comments

Comments

@JustFly1984
Copy link

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
Copy link
Contributor

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
Copy link
Author

@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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

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

@EskiMojo14
Copy link
Contributor

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

@JustFly1984
Copy link
Author

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
Copy link
Contributor

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
Copy link
Contributor

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

@JustFly1984
Copy link
Author

@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
Copy link
Contributor

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

@JustFly1984
Copy link
Author

@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
Copy link
Author

@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
Copy link
Contributor

@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
Copy link
Author

@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
Copy link
Contributor

EskiMojo14 commented Jan 3, 2024

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
Copy link
Author

Thank you @EskiMojo14, please consider an issue resolved.

@EskiMojo14
Copy link
Contributor

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

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

3 participants