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

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

Open
2 tasks
Armenvardanyan95 opened this issue Mar 6, 2024 · 4 comments

Comments

@Armenvardanyan95
Copy link
Contributor

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
@alexkunin
Copy link

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.

@timdeschryver
Copy link
Member

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

@Armenvardanyan95
Copy link
Contributor Author

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

@Armenvardanyan95
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants