store icon indicating copy to clipboard operation
store copied to clipboard

🚀[FEATURE]: Add more types for createSelector function

Open Aracturat opened this issue 5 years ago • 4 comments

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[ ] Performance issue
[x] Feature request
[ ] Documentation issue or request
[ ] Support request => https://github.com/ngxs/store/blob/master/CONTRIBUTING.md
[ ] Other... Please describe:

Current behavior

When use multiple nesting createSelector functions, types are missing.

Expected behavior

Function createSelector should infer parameters from nesting createSelector functions.

Minimal reproduction of the problem with instructions

Example:

import { createSelector, State } from '@ngxs/store';

export interface TestStateModel {
  prop: string;
}

@State<TestStateModel>({
  name: 'state',
  defaults: {
    prop: null
  }
})
export class TestState {
}

export const selectTestStateModel = createSelector(
  [TestState],
  (state: TestStateModel) => state
); // Type is (state: TestStateModel) => TestStateModel

export const selectStateModelProp = createSelector(
  [selectTestStateModel],
  model => model.prop // here model is any.
); // Type is (model: any) => any, should be (state: TestStateModel) => string

Possible typings (it works, but maybe it is not good typings).

export declare function createSelector<T extends (...args: any[]) => any, T1>(
    selectors: [(...args: any[]) => T1],
    originalFn: (arg: T1) => T,
    creationMetadata?: {
    containerClass: any;
        selectorName: string;
    }
): T;
export declare function createSelector<T extends (...args: any[]) => any, T1, T2>(
    selectors: [(...args: any[]) => T1, (...args: any[]) => T2],
    originalFn: (arg1: T1, arg2: T2) => T,
    creationMetadata?: {
        containerClass: any;
        selectorName: string;
    }
): T;
export declare function createSelector<T extends (...args: any[]) => any, T1, T2, T3>(
    selectors: [(...args: any[]) => T1, (...args: any[]) => T2, (...args: any[]) => T3],
    originalFn: (arg1: T1, arg2: T2, arg3: T3) => T,
    creationMetadata?: {
        containerClass: any;
        selectorName: string;
    }
): T;
export declare function createSelector<T extends (...args: any[]) => any>(selectors: any[] | undefined, originalFn: T, creationMetadata?: {
    containerClass: any;
    selectorName: string;
}): T;

What is the motivation / use case for changing the behavior?

Add more type safety to code.

Environment

Libs:
- @angular/core version: 7.1.0
- @ngxs/store version: 3.4.2

Aracturat avatar Mar 12 '19 06:03 Aracturat

I agree that this is needed. One challenge is that either functions or state classes could be provided as selectors. The type definitions should allow combinations of these. For Example:

export const selectTestStateModel = createSelector(
  [TestState, someOtherSelector],
  (state: TestStateModel, someOtherData: SomeOtherModel) => ({ state, someOtherData })
);

markwhitfeld avatar Mar 12 '19 07:03 markwhitfeld

It would be easier to implement it if the creation of store looked like this:

@State<TestStateModel>({
  name: 'state',
  defaults: {
    prop: null
  }
})
export class TestState implements SomeStateClass<TestStateModel> {

}

Aracturat avatar Mar 12 '19 07:03 Aracturat

Yes, agreed. I have actually been planning to add an option for that as part of an opt-in improved type safety for NGXS v4. I should probably document the idea somewhere ;-) My thought was for something like:

@State<TestStateModel>({
  name: 'state',
  defaults: {
    prop: null
  }
})
export class TestState extends StateModelOf<TestStateModel> {

}

(extending StateModelOf<> would be optional) With the testing I did mid last year you need to use extends as opposed to implements to get the type inference and safety but this may have improved in TS 3.x This would also potentially allow for better type safety for the @Action and @Selector decorators.

markwhitfeld avatar Mar 12 '19 09:03 markwhitfeld

Also

image

splincode avatar Mar 12 '19 11:03 splincode

Great news! v3.8.0 has been released and it includes a fix for this issue. We are closing this issue, but please re-open it if the issue is not resolved. Please leave a comment in the v3.8.0 release discussion if you come across any regressions with respect to your issue.

markwhitfeld avatar Mar 29 '23 14:03 markwhitfeld