Possibly incorrect type for createAction

[ianp]

I don't think that the recently added error field is getting included in the type returned by createAction, see

import { createAction, createSlice } from 'redux-starter-kit'

type MySliceState = { counter: number; error?: any }

const oops = createAction('oops', (error: any) => ({ payload: {}, error }))

const slice = createSlice({
  name: 'mySlice',
  initialState: { counter: 1 } as MySliceState,
  reducers: {},
  extraReducers: {
    [oops.type]: (state, action: ReturnType<typeof oops>) => {
      state.error = action.error // !!! DOES NOT COMPILE !!!
    }
  }
})

I get an error like property error does not exist on type WithPayload<{}, Action<"oops">>, but I think that the inferred type should include WithOptional?

Of course it's also possible that I'm specifying the action type incorrectly!

[markerikson]

I think this is due to declaring error as any.

Using your example, the return type of oops is:

const oops: WithTypeProperty<"oops", (error: any) => WithPayload<{}, Action<"oops">>>

However, if I just change error: any to error: boolean, it suddenly becomes:

const oops: WithTypeProperty<"oops", (error: boolean) => WithOptional<void, boolean, WithPayload<{}, Action<"oops">>>>

Ditto if I use string.

So, something about any vs a meaningful type kicks it into a different type definition path. Seems like the immediate solution is just to give error a non-any type.

@phryneas, @Jessidhia , @tvanier : just out of curiosity, any idea why we're hitting that path, and if it can be tightened up?

[ianp]

Yep, that's looks right! Interestingly, it works find with unknown which is probably a better option than any here anyway.

I think that the issue may be with the PrepareAction type: PayloadAction is defined to carry along the M and E types with it, via WithOptional, but PrepareAction is just a simple union type that ignores them.

[phryneas]

Yes, any is a little bit of a problem child, as any always takes both paths in ternaries:
type X = any extends {} ? "yes" : "no" will result in X being "yes" | "no".

Unknown works well there.

Will have to catch that as early as possible - maybe I find the time to look into this on the weekend.

[ianp]

Maybe this for PrepareAction?

export type PrepareAction<P, M = void, E = void> = (
  ...args: any[]
) => Omit<WithOptional<M, E, WithPayload<P, Action>>, 'type'>

(or something similar using Exclude to support older TS versions).

But then you'd probably need to update PayloadActionCreator as well, and TBH that type definition terrifies me!

[phryneas]

Mh, I think the error is in

type WithOptional<M, E, T> = T &
  ([M] extends [void] ? {} : { meta: M }) &
  ([E] extends [void] ? {} : { error: E })

Calling this with any for M or E will test [any] extends [void] which will be true.
I'll add a PR with a fix & some regression tests.

[tvanier]

Could/Should we simplify by restricting error to boolean, and error data would be in payload, as the Flux standard action suggests?
https://github.com/redux-utilities/flux-standard-action#error

This would also helps with this (not obvious imho)

If error has any other value besides true, including undefined and null, the action MUST NOT be interpreted as an error.

i.e. action is an error if and only if action.error === true

[markerikson]

Hmm. Could we? Yeah, and I agree that FSA expects that to just be a boolean.

That said, strictly speaking changing it now would technically be considered a breaking change and thus require a semver major. Now, realistically, no one's going to care, because it's been out for all of like 3 days and I'd be shocked if anyone's even using that field yet.

Still, I'm not sure there's any real benefit to us restricting the type of the error field atm.

[Jessidhia]

This sounds like a bug, unfortunately 😢

The base FSA type, at least one that is error-able, is something like { error: false, payload: T } | { error: true, payload: any }, but this is going to break any "raw" reducer that doesn't check for error before trying to use the payload (it'll just unify to any; unless unknown or Error is used?).

[markerikson]

Hmm. Okay, yeah, in that case I could see an argument for tightening it up.

[markerikson]

Hopefully fixed by #233 . I'll try to put out a 1.0.1 soon (no later than this weekend).

[markerikson]

Published https://github.com/reduxjs/redux-starter-kit/releases/tag/v1.0.1 .