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

createAction() does not handle generic types correctly #4110

Open
2 tasks
tomer953 opened this issue Nov 6, 2023 · 3 comments
Open
2 tasks

createAction() does not handle generic types correctly #4110

tomer953 opened this issue Nov 6, 2023 · 3 comments

Comments

@tomer953
Copy link

tomer953 commented Nov 6, 2023

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

store

Minimal reproduction of the bug/regression with instructions

import { createAction, props } from '@ngrx/store';
import { PcDataKey, PcDataType, RequestStatus } from './state.interface';

export const setUsername = createAction(
  '[Main] setUsername',
  props<{ username: string }>()
);
export const setPcStatus = createAction(
  '[Main] setPcStatus',
  props<{ key: PcDataKey; status: RequestStatus }>()
);

// props does not support for generic types, so I used the arrow function method:
const pcDataProps = <K extends PcDataKey>(obj: {
  key: K;
  data: PcDataType<K>;
}) => obj;
export const setPcData = createAction('[Main] setPcData', pcDataProps);

// now, while this throws as it should (type safety for the data type based on the key):
pcDataProps({ key: 'Drives', data: { response: 100 } });
// this doesn't, so the createAction is somewhere ignore the generic types:
setPcData({ key: 'Drives', data: { response: 100 } }); // Should throw Type '{ response: number; }' is not assignable to type 'DrivesResult'

Full code, interfaces and such are here:

https://stackblitz.com/github/tomer953/ngrx-store-generic-types?file=src%2Fapp%2Fstore%2Factions.ts

Expected behavior

assume we have a function that use generic types to infer the arguments type, ie:

<K extends PcDataKey>(obj: {
  key: K;
  data: PcDataType<K>;
})

The createAction function should enforce strict typing based on the provided function, such that providing incorrect data types for data should result in a compilation error.

so the signature should look like:

image

but instead is:

image

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

Angular CLI: 16.2.9
Node: 20.9.0 (Unsupported)
Package Manager: npm 10.1.0
OS: win32 x64

Angular: 16.2.12
... animations, common, compiler, compiler-cli, core, forms
... platform-browser, platform-browser-dynamic, router

Package Version

@angular-devkit/architect 0.1602.9
@angular-devkit/build-angular 16.2.9
@angular-devkit/core 16.2.9
@angular-devkit/schematics 16.2.9
@angular/cli 16.2.9
@schematics/angular 16.2.9
rxjs 7.8.1
typescript 5.1.6
zone.js 0.13.3
@ngrx/store 16.3.0

Other information

No response

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

  • Yes
  • No
@tomer953
Copy link
Author

tomer953 commented Nov 6, 2023

B.t.w, I wrapped with object obj since when I used a "flat" version, no type safety at all, so it can be another bug, not sure:

type PcDataItem<K extends PcDataKey> = { key: K; data: PcDataType<K> };
const pcDataPropsFlatten = <K extends PcDataKey>(key: K, data: PcDataType<K>) =>
  ({ key, data } as PcDataItem<K>);
pcDataPropsFlatten('Ping', []); // works as expected with type checking for the data
const setPcDataTest = createAction('[Main] setPcData', pcDataPropsFlatten);
setPcDataTest('foo123', 'bar456'); // no type checking at all, both key,data are 'any'

image

@alexkunin
Copy link

@tomer953 , try the following approach (playground):

const pcDataProps2 = <K extends PcDataKey>(
  obj: K extends PcDataKey ? { key: K; data: PcDataType<K> } : never
) => obj;
export const setPcData2 = createAction('[Main] setPcData', pcDataProps2);

pcDataProps2({ key: 'Drives', data: { response: 100 } }); // Invalid
pcDataProps2({ key: 'Ping', data: { response: 100 } }); // Valid

setPcData2({ key: 'Drives', data: { response: 100 } }); // Invalid
setPcData2({ key: 'Ping', data: { response: 100 } }); // Valid

To be fair, I'm not exactly sure why it works (or rather, why your original approach does not). Probably, has something to do with better type narrowing due to never branch in the conditional type? Would love to hear some thoughts on this.

@david-shortman
Copy link
Contributor

@tomer953 , try the following approach (playground):

const pcDataProps2 = <K extends PcDataKey>(
  obj: K extends PcDataKey ? { key: K; data: PcDataType<K> } : never
) => obj;
export const setPcData2 = createAction('[Main] setPcData', pcDataProps2);

pcDataProps2({ key: 'Drives', data: { response: 100 } }); // Invalid
pcDataProps2({ key: 'Ping', data: { response: 100 } }); // Valid

setPcData2({ key: 'Drives', data: { response: 100 } }); // Invalid
setPcData2({ key: 'Ping', data: { response: 100 } }); // Valid

To be fair, I'm not exactly sure why it works (or rather, why your original approach does not). Probably, has something to do with better type narrowing due to never branch in the conditional type? Would love to hear some thoughts on this.

Hm, my hunch is that your solution is introduce deference with a conditional type, making the generic type evaluated later. I think NoInfer introduced in TS 5.4 may solve this issue...

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

No branches or pull requests

3 participants