TS Bug found while testing the 2.0.0-beta.4

[wirdehall]

When creating a listener middleware that matches an action using matcher: isAnyOf(...) it's no longer getting the correct action type. In this instance it's missing that it's a payload action.

Here's a code example:

const notificationSlice = createSlice({
  name: 'notification',
  initialState,
  reducers: {
    addNotification: {
      reducer: (state, { payload: { notificationType, notification, autoDismissInMs, id } }: PayloadAction<FullNotificationActionPayload>) => {
        state[notificationType].push({ notification, autoDismissInMs, id });
      },
      prepare: (payload: NotificationActionPayload) => ({ payload: { ...payload, id: nanoid() } })
    },
  }
})

export const { addNotification } = notificationSlice.actions;

And where I create my listenerMiddleware:

notificationEffects.startListening({
  matcher: isAnyOf(addNotification),
  effect: async (action, listenerApi) => {
    const { autoDismissInMs, notificationType, id } = action.payload;
    .
    .
    .

Here it will complain that action.payload does not exist since action is typed as Action instead of PayloadAction.

This is not a big issue in this example since I shoud probably just use actionCreateoras a matcher instead, then action get's the correct type.

But the same listener middleware has another listner where I match multiple actions like this:

notificationEffects.startListening({
  matcher: isAnyOf(addSuccess, addNotice, addError),
  effect: async (action, listenerApi) => {
    listenerApi.dispatch(addNotification(action.payload));
  },
});

The same problem here.
This worked prior to upgrading to the beta.

[markerikson]

Huh, that's surprising. We really didn't change anything about the listener middleware, other than going from AnyAction to UnknownAction in some places.

Could you put together a project that reproduces this? Also, what version of TS are you on? And is there only one version of the redux core installed, or multiple versions?

[EskiMojo14]

i'm not sure what changed the inference behaviour, but it's worth noting that moving the matcher definition outside of the startListening call will work:

const isNotificationAction = isAnyOf(addSuccess, addNotice, addError)
notificationEffects.startListening({
  matcher: isNotificationAction,
  effect: async (action, listenerApi) => {
    listenerApi.dispatch(addNotification(action.payload));
  },
});

TS Playground link

though if i'm honest, this particular usage of listener middleware doesn't seem very necessary? if you're dispatching an action in response to an action you should instead just use extraReducers to respond to the original action within your reducer.

const notificationSlice = createSlice({
  name: 'notification',
  initialState,
  reducers: {
    addNotification: {
      reducer: (state, { payload: { notificationType, notification, autoDismissInMs, id } }: PayloadAction<FullNotificationActionPayload>) => {
        state[notificationType].push({ notification, autoDismissInMs, id });
      },
      prepare: (payload: NotificationActionPayload) => ({ payload: { ...payload, id: nanoid() } })
    },
  }
  extraReducers: (builder) => {
    builder.addMatcher(
      isAnyOf(addSuccess, addNotice, addError),
      (state, action) => notificationSlice.caseReducers.addNotification(state, addNotification(action.payload))
    )
  }
})

export const { addNotification } = notificationSlice.actions;

[wirdehall]

First of all, thanks for the quick replies!

@markerikson : Here's a minimal project that shows this error: https://github.com/wirdehall/bug-redux-toolkit-beta
It doesn't have enough files to run, but we don't need to run it, we just need to see the TS error.
Typescript version: 5.0.2.
It's the only version of redux installed.

@EskiMojo14: Interesting that it changes behaviour when we move it out. I tried it and in my case it still does not work since notification types are not set to string but to "error" | "notice" | "success".

Hum, yeah that seams like a good way of going about it. Might change that up in my code.

Still, I didn't report this specificlly for my use-case as I can easily solve this by just type-casting.
I thought it might be a good idea to report as this did work in 1.9 but not in the 2.0 beta and it might affect alot of people.
And most importanly, it was nice when it just worked 😁

[EskiMojo14]

After some more investigation, it appears this was already an issue for TS in 1.9?

The difference being that AnyAction (the type in 1.9) will just return any when you access unknown fields, whereas the new type (Action) will not let you access unknown fields. This is why you got new type errors - but if you checked in 1.9, I suspect that they would have just been any and all type safety had gone out the window.

If anything, out of the two I'd argue the new behaviour is safer - though obviously neither is ideal, and you can define the matcher outside of the startListening call to get the proper narrowing behaviour as discussed.

[EskiMojo14]

for future reference, this seems to be another issue fixed by @Andarist's PR here: microsoft/TypeScript#54183

playground using the build from the PR, proving it fixes the issue: https://tsplay.dev/wE6RVN