platform icon indicating copy to clipboard operation
platform copied to clipboard

Using a custom generic function for defining props in createActionGroup results in weird TypeScript errors

Open Armenvardanyan95 opened this issue 1 year ago • 4 comments

Which @ngrx/* package(s) are the source of the bug?

store

Minimal reproduction of the bug/regression with instructions

The events field in in the createActionGroup function's first argument allows us to define props either by using the props function

const actions = createActionGroup({
  source: 'source',
  events: {
    someAction: props<{username: string}>(),
  },
})

Or by using a custom function:

export const actions = createActionGroup({
  source: 'source',
  events: {
    someAction: (username: string) => ({username}),
  },
});

This works fine, however, a weird error arises when we try to use a custom factory function for creating specific props. Here is an example of such a function:

export function httpSuccessProps<T = void>(message: string) {
  return function(data: T) {
    return {
      success: true,
      message,
      data,
    };
  }
}

As we can see, the T type is defaulting to void. If we use it with some type then, we will be fine:

export const actions= createActionGroup({
  source: 'source',
  events: {
    someAction: (username: string) => ({username}),
    someHttpAction: httpSuccessProps<void>('Message'),
  },
});

However, if we omit the void type when calling the httpSuccessProps function, which we theoretically should be able to do (void is the default type anyway!), we get a TypeScript error:

 Type '(data: void) => { success: boolean; message: string; data: void; }' is not assignable to type 'never'

What is even weirder, if we just extract the same code to a constant and use it the same way, the error goes away:

const httpAction = httpSuccessProps('Message'); // extract to a constant

export const newActions = createActionGroup({
  source: 'source',
  events: {
    someAction: (username: string) => ({username}),
    someHttpAction: httpAction, // use the same thing but through a constant
  },
});

This is confusing and nothing really can be done with this, other than just spelling <void> everywhere. I am unsure if this is an NgRx problem or a TypeScript problem, but could be some deeper issue with some complex mapped types that NgRx uses for the createActionGroup function. Working reproduction on stackblitz can be found here

Expected behavior

Prop factory function types should work as intended form the TypeScript perspective

Versions of NgRx, Angular, Node, affected browser(s) and operating system(s)

NgRx: 17.1 Angular: 17.1 Node: 18 Windows 10

Other information

No response

I would be willing to submit a PR to fix this issue

  • [ ] Yes
  • [ ] No

Armenvardanyan95 avatar Mar 06 '24 13:03 Armenvardanyan95

Can't say I'm 100% sure I have solution to this, but here is the way to get rid of that error (StackBlitz included):

type Cr = () => {success:true,message:string}
type CrWithData<T> = (data:T) => {success:true,message:string,data:T}

export function httpSuccessProps<T = void>(message: string): T extends void ? Cr : CrWithData<T> {
  return function(data: T) {
    return {
      success: true,
      message,
      data,
    };
  } as any
}

Basically, instead of letting the compiler to infer type, I just clearly define it. I think that's a good practice to explicitly specify return types for any top-level (and especially exported) functions -- sort of an inline "unit test" for types, a fixed point or a fixture that improves compiler performance.

I don't like that any cast, but actual JS 'type' in case of T = void would be { success: true, message: string, data: undefined }, so, I guess just convince compiler we're know what we're doing here. But that's pretty typical for overloaded functions, I don't see a big problem here.

Of course, this does not explain why the error is there in the first place -- I'm also curious to lear the actual reason.

alexkunin avatar Mar 14 '24 14:03 alexkunin

I think this is related to https://github.com/microsoft/TypeScript/issues/241, we've experienced a similar behavior with the on functions within createReducer.

timdeschryver avatar Mar 15 '24 20:03 timdeschryver

@timdeschryver Oh I see, seems reasonable that it could cause an issue here!

Armenvardanyan95 avatar Apr 05 '24 11:04 Armenvardanyan95

@alexkunin thanks a lot, it seems what you suggested solved the issue!

Armenvardanyan95 avatar Apr 05 '24 11:04 Armenvardanyan95