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

listenerMiddleware type error in matcher using custom actions. #4641

Open
antondalgren opened this issue Sep 24, 2024 · 10 comments
Open

listenerMiddleware type error in matcher using custom actions. #4641

antondalgren opened this issue Sep 24, 2024 · 10 comments

Comments

@antondalgren
Copy link
Contributor

antondalgren commented Sep 24, 2024

I have a codebase using legacy redux patterns and trying to slowly but steadily transition to using modern redux. I have a set of old Actions that are created similiar to ExtraAction mentioned below. When using the listenerMiddleware to add a set of type guards as matchers using the isAnyOf utility as demonstrated below, I see a type error on the matcher.

Also, it seems as the type inferred from the predicate (probably the matcher to) doesn't carry along into effect callback.

    interface ExtraAction extends Action {
      payload: number
    }

    function isPayloadAction(action: any): action is ExtraAction {
      return (
        isFluxStandardAction(action) && typeof action.payload === 'number'
      )
    }

    startListening({
      matcher: isAnyOf(isPayloadAction),
      effect: (action, listenerApi) => {
        expectTypeOf(action).toMatchTypeOf<ExtraAction>()
      },
    })
@EskiMojo14
Copy link
Collaborator

predicate isn't used as a type guard, only matcher is.

Could you post what the type error you're getting is?

#3862 is also likely related.

@antondalgren
Copy link
Contributor Author

predicate isn't used as a type guard, only matcher is.

Could you post what the type error you're getting is?

#3862 is also likely related.

Thank you for the swift response.

Here is the ts-error:

No overload matches this call.
  The last overload gave the following error.
    Type '(action: any) => action is ActionMatchingAnyOf<[(action: any) => action is ExtraAction]>' is not assignable to type 'undefined'.ts(2769)
types.ts(489, 7): The expected type comes from property 'matcher' which is declared here on type '{ actionCreator?: undefined; type?: undefined; matcher?: undefined; predicate: AnyListenerPredicate<unknown>; effect: ListenerEffect<UnknownAction, unknown, ThunkDispatch<unknown, unknown, UnknownAction>, unknown>; }'
types.ts(485, 3): The last overload is declared here.

This does provide action with the correct type, thus using a type guard as predicate should carry along the correct type, shouldn't it? 🤔

    startListening({
      predicate: incrementByAmount.match,
      effect: (action, listenerApi) => {
        expectTypeOf(action).toMatchTypeOf<ExtraAction>()
      },
    })

@markerikson
Copy link
Collaborator

No, predicate explicitly does not narrow the action type. It's a very generic (action, prevState, nextState) => boolean callback that could have any logic inside, and so it can't guarantee the type of action.

@antondalgren antondalgren changed the title listenerMiddleware type error in matcher and predicate using custom actions. listenerMiddleware type error in matcher using custom actions. Sep 24, 2024
@antondalgren
Copy link
Contributor Author

No, predicate explicitly does not narrow the action type. It's a very generic (action, prevState, nextState) => boolean callback that could have any logic inside, and so it can't guarantee the type of action.

OK. The explanation of the issue still holds for matchers. Any suggestions on how I could fix this in either a PR or a workaround in my codebase?

@EskiMojo14
Copy link
Collaborator

EskiMojo14 commented Oct 30, 2024

have you checked out the issue i linked, and tried moving the isAnyOf call to a separate variable like the replies to that issue recommend?

@antondalgren
Copy link
Contributor Author

have you checked out the issue i linked, and tried moving the isAnyOf call to a separate variable like the replies to that issue recommend?

Yes, just tried and that didn't help. It works as expected when using RTK defined actions such as below. But not when using custom built matcher functions.

const myThunk = createAsyncThunk.....;

const myMatcher = isAnyOf(myThunk.fulfilled),
startListening({
   matcher: myMatcher,
   effect: (action, listenerApi) => {
     expectTypeOf(action).toMatchTypeOf<typeof myThunk....>()
   },
})

@EskiMojo14
Copy link
Collaborator

Could you put together a reproduction of this, either in Typescript Playground or a repo i can checkout?

@antondalgren
Copy link
Contributor Author

Could you put together a reproduction of this, either in Typescript Playground or a repo i can checkout?

Sure, here is a commit adding a testcase in a fork of this repo. Hope this is clear enough, otherwise let me know and I'll try to adjust!
antondalgren@de72aac

@EskiMojo14
Copy link
Collaborator

EskiMojo14 commented Nov 15, 2024

ok, that helped a little - it still won't work inline because of the TS bug mentioned previously, but i found a change that fixes it when the predicate is defined outside.

Switching from UnknownAction to Action works:

index b5980e10..eb42c86b 100644
--- a/packages/toolkit/src/listenerMiddleware/types.ts
+++ b/packages/toolkit/src/listenerMiddleware/types.ts
@@ -466,7 +466,7 @@ export type AddListenerOverloads<
   ): Return
 
   /** Accepts an RTK matcher function, such as `incrementByAmount.match` */
-  <MatchFunctionType extends MatchFunction<UnknownAction>>(
+  <MatchFunctionType extends MatchFunction<Action>>(

Presumably this is because any interface is missing the index signature UnknownAction has. you can verify this by switching ExtraAction to a type instead and the error disappears:
Image

This is briefly covered in this article by Matt Pocock.

@antondalgren
Copy link
Contributor Author

Oh, so the preferred way of declaring actions in a legacy type of application would be to use type instead of interface? Well, today I learned something new. Thank you for taking the time to look through this issue @EskiMojo14 !

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