Q: createAction with payload typings

[donifer]

I'm trying to type an action with something like:

const incrementBy = createAction<number>('INCREMENT_BY');

// TS complains as expected if I pass the wrong type
incrementBy('This is not a valid payload');

// However, TS likes both of these:
incrementBy(3);
incrementBy();

Why is the last one not an error? How can I type it in a way that payload is required?

[markerikson]

Hmm. Yeah, I would generally expect that last one to be an error. I know there's some trickiness around how the action creators get defined, but I'm not clear on the details myself.

@phryneas , @Jessidhia, @denisw : any idea what's happening here?

[phryneas]

I just tested this and at least on master, this seems to work. Are you maybe not on the most current version of RSK?
Also, what's your TS version? We've had differing behaviour with older versions before.

[donifer]

Thanks for looking at this. @phryneas I created a repro here: https://codesandbox.io/s/nervous-frost-rczon?fontsize=14

Ignore the react setup, just used it as a base, check out the redux.ts file.

[phryneas]

Weirdly, this doesn't trigger a test i added right now, but in your repro i can see it. Seems like a problem with the IfMaybeUndefined type. Looking into it.

[phryneas]

okay, the following type only works when the compilerOption strictNullChecks is set to true in tsconfig.json:

type IfMaybeUndefined<P, True, False> = [undefined] extends [P] ? True : False;

I'm not sure if that CAN be tested without the strictNullChecks flag though.

Firstly, could you please validate if setting the flag helps you with your problem?

[donifer]

That works @phryneas! Sorry for the false positive. 🙇

[phryneas]

As this is out of my knowledge to resolve, I've posted a SO question - maybe someone else knows if this is testable, even with strictNullChecks: false.

We'll see. If it is possible, I'll do a PR later.

[wuifdesign]

is there any update on this? is it possible without using strictNullChecks?

[markerikson]

@wuifdesign : what version of RSK are you using? We merged in some changes in #174 that might fix this. Make sure you're using v1.0.1 - does that fix the issue?

[phryneas]

It's not possible without strictNullChecks.

The answerer of my above SO question quoted the TS documentation

In strict null checking mode, the null and undefined values are not in the domain of every type and are only assignable to themselves and any (the one exception being that undefined is also assignable to void). So, whereas T and T | undefined are considered synonymous in regular type checking mode (because undefined is considered a subtype of any T), they are different types in strict type checking mode, and only T | undefined permits undefined values. The same is true for the relationship of T to T | null.

So, even if you're writing

createAction<number>('INCREMENT_BY');

without strictNullChecks, that's not different from writing

createAction<number|null|undefined>('INCREMENT_BY');

so from our side, there is no way to distinguish if you WANTED to write number|undefined or if TS just added the undefined.

So, we're always handling it as if you had written number|undefined, allowing the actionCreator to be called without an argument.

There's really no way around this - but if you're not using strictNullChecks, honestly this should be the least of your problems. I really suggest you evaluate turning it on.

[phryneas]

Or, technically, we could go from undefined to never here, but that would be unexpected behaviour for most of our users (undefined is what TS uses for optional properties, not never) and a breaking change, too.
So I'd rather not do it, even though it would be technically possible.

[wuifdesign]

v1.0.1 dosn't fix this issue.

it is just a possible cause of erros if this check isn't working, for example you can do this without TS error:

const slice = createSlice({
  name: 'test',
  initialState: 0,
  reducers: {
    increment: (state, action: PayloadAction<number>) => state + action.payload,
  },
});

store.dispatch(slice.actions.increment()); //This dosn't show a TS error

this will end up in a "NaN" in the store because action.payload will be undefined. but with TS we should be able to catch this error before having to debugg it in runtime.

if i do:

function test(num: number) {}
test(); //This will show a TS error

maybe the types have to be rewiritten somehow to do a different check for the "undefined", but i don't know how.

[phryneas]

Yet, if you do

function myManualActionCreator(): { payload: number } {
  return { payload: undefined };
}

TS with strictNullChecks: false will not complain at all.
Likewise, with your test function above, TS with strictNullChecks: false would be totally okay if you call it like test(undefined) or test(null). Calls like this would also result in action.payload being undefined (or null), resulting in the same bug.

This is just your decision to disable a fundamental type security feature of TypeScript. From that point on you'll have to double-check everything you do as a dev.

The typings are stating here: if the payload of the resulting action is optional, the method may be called without an argument.
And as long as you disable strictNullChecks, the payload is always optional. That's what that means.