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.

Sign up for GitHub

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

feat(store): strict projectors #3581

Merged
merged 5 commits into from
Nov 2, 2022
Merged

feat(store): strict projectors #3581

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
db52fe2
feat(store): strict projectors
david-shortman Sep 17, 2022
229dcc2
use loose projector in test
david-shortman Sep 17, 2022
d187e59
remove "strictness" generic
david-shortman Oct 22, 2022
7d39d0d
Update modules/store/src/selector.ts
timdeschryver Nov 2, 2022
57276e6
Update modules/store/src/selector.ts
timdeschryver Nov 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 8 additions & 14 deletions modules/router-store/spec/types/selectors.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,7 @@ describe('router selectors', () => {
selectCurrentRoute,
route => route
);
`).toInfer(
'selector',
'MemoizedSelector<State, any, DefaultProjectorFn<any>>'
);
`).toInfer('selector', 'MemoizedSelector<State, any, (s1: any) => any>');
});

it('selectQueryParams should return Params', () => {
Expand All @@ -52,7 +49,7 @@ describe('router selectors', () => {
);
`).toInfer(
'selector',
'MemoizedSelector<State, Params, DefaultProjectorFn<Params>>'
'MemoizedSelector<State, Params, (s1: Params) => Params>'
);
});

Expand All @@ -65,7 +62,7 @@ describe('router selectors', () => {
);
`).toInfer(
'selector',
'MemoizedSelector<State, string, DefaultProjectorFn<string>>'
'MemoizedSelector<State, string, (s1: string) => string>'
);
});

Expand All @@ -77,7 +74,7 @@ describe('router selectors', () => {
);
`).toInfer(
'selector',
'MemoizedSelector<State, Params, DefaultProjectorFn<Params>>'
'MemoizedSelector<State, Params, (s1: Params) => Params>'
);
});

Expand All @@ -90,7 +87,7 @@ describe('router selectors', () => {
);
`).toInfer(
'selector',
'MemoizedSelector<State, string, DefaultProjectorFn<string>>'
'MemoizedSelector<State, string, (s1: string) => string>'
);
});

Expand All @@ -100,10 +97,7 @@ describe('router selectors', () => {
selectRouteData,
data => data
);
`).toInfer(
'selector',
'MemoizedSelector<State, Data, DefaultProjectorFn<Data>>'
);
`).toInfer('selector', 'MemoizedSelector<State, Data, (s1: Data) => Data>');
});

it('selectUrl should return string', () => {
Expand All @@ -114,7 +108,7 @@ describe('router selectors', () => {
);
`).toInfer(
'selector',
'MemoizedSelector<State, string, DefaultProjectorFn<string>>'
'MemoizedSelector<State, string, (s1: string) => string>'
);
});

Expand All @@ -126,7 +120,7 @@ describe('router selectors', () => {
);
`).toInfer(
'selector',
'MemoizedSelector<State, string, DefaultProjectorFn<string>>'
'MemoizedSelector<State, string, (s1: string) => string>'
);
});
});
2 changes: 1 addition & 1 deletion modules/store/spec/selector.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ describe('Selectors', () => {

const selector = createSelector(incrementOne, incrementTwo, projectFn);

expect(selector.projector()).toBe(2);
expect(selector.projector<any>()).toBe(2);

selector.setResult(5);

Expand Down
51 changes: 51 additions & 0 deletions modules/store/spec/types/selector.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import { expecter } from 'ts-snippet';
import { compilerOptions } from './utils';

describe('createSelector()', () => {
const expectSnippet = expecter(
(code) => `
import {createSelector} from '@ngrx/store';
import { MemoizedSelector, DefaultProjectorFn } from '@ngrx/store';

${code}
`,
compilerOptions()
);

describe('projector', () => {
it('should require correct arguments by default', () => {
expectSnippet(`
const selectTest = createSelector(
() => 'one',
() => 2,
(one, two) => 3
);
selectTest.projector();
`).toFail(/Expected 2 arguments, but got 0./);
});
it('should not require correct parameters when strictness bypassed with `any` generic argument', () => {
expectSnippet(`
const selectTest = createSelector(
() => 'one',
() => 2,
(one, two) => 3
);
selectTest.projector<any>();
`).toSucceed();
});
it('should not require parameters for existing explicitly loosely typed selectors', () => {
expectSnippet(`
const selectTest: MemoizedSelector<
unknown,
number,
DefaultProjectorFn<number>
> = createSelector(
() => 'one',
() => 2,
(one, two) => 3
);
selectTest.projector();
`).toSucceed();
});
});
});
57 changes: 46 additions & 11 deletions modules/store/src/selector.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,32 @@ export type ComparatorFn = (a: any, b: any) => boolean;

export type DefaultProjectorFn<T> = (...args: any[]) => T;

type ProjectorStrictness = 'strict';

type SelectorProjectorArgs<
StrictnessConfig extends ProjectorStrictness,
ProjectorArgs
> = StrictnessConfig extends 'strict'
? ProjectorArgs extends unknown[]
? ProjectorArgs
: any[]
: any[];

type SelectorProjectorFn<ProjectorFn> = ProjectorFn extends (
...args: infer ProjectorArgs
) => infer ProjectorResult
? <Strictness extends ProjectorStrictness = 'strict'>(
...args: SelectorProjectorArgs<Strictness, ProjectorArgs>
) => ProjectorResult
: ProjectorFn;
timdeschryver marked this conversation as resolved.
Show resolved Hide resolved

export interface MemoizedSelector<
State,
Result,
ProjectorFn = DefaultProjectorFn<Result>
> extends Selector<State, Result> {
release(): void;
projector: ProjectorFn;
projector: SelectorProjectorFn<ProjectorFn>;
timdeschryver marked this conversation as resolved.
Show resolved Hide resolved
setResult: (result?: Result) => void;
clearResult: () => void;
}
Expand Down Expand Up @@ -125,33 +144,37 @@ export function defaultMemoize(
export function createSelector<State, S1, Result>(
s1: Selector<State, S1>,
projector: (s1: S1) => Result
): MemoizedSelector<State, Result>;
): MemoizedSelector<State, Result, (s1: S1) => Result>;
export function createSelector<State, S1, S2, Result>(
s1: Selector<State, S1>,
s2: Selector<State, S2>,
projector: (s1: S1, s2: S2) => Result
): MemoizedSelector<State, Result>;
): MemoizedSelector<State, Result, (s1: S1, s2: S2) => Result>;
export function createSelector<State, S1, S2, S3, Result>(
s1: Selector<State, S1>,
s2: Selector<State, S2>,
s3: Selector<State, S3>,
projector: (s1: S1, s2: S2, s3: S3) => Result
): MemoizedSelector<State, Result>;
): MemoizedSelector<State, Result, (s1: S1, s2: S2, s3: S3) => Result>;
export function createSelector<State, S1, S2, S3, S4, Result>(
s1: Selector<State, S1>,
s2: Selector<State, S2>,
s3: Selector<State, S3>,
s4: Selector<State, S4>,
projector: (s1: S1, s2: S2, s3: S3, s4: S4) => Result
): MemoizedSelector<State, Result>;
): MemoizedSelector<State, Result, (s1: S1, s2: S2, s3: S3, s4: S4) => Result>;
export function createSelector<State, S1, S2, S3, S4, S5, Result>(
s1: Selector<State, S1>,
s2: Selector<State, S2>,
s3: Selector<State, S3>,
s4: Selector<State, S4>,
s5: Selector<State, S5>,
projector: (s1: S1, s2: S2, s3: S3, s4: S4, s5: S5) => Result
): MemoizedSelector<State, Result>;
): MemoizedSelector<
State,
Result,
(s1: S1, s2: S2, s3: S3, s4: S4, s5: S5) => Result
>;
export function createSelector<State, S1, S2, S3, S4, S5, S6, Result>(
s1: Selector<State, S1>,
s2: Selector<State, S2>,
Expand All @@ -160,7 +183,11 @@ export function createSelector<State, S1, S2, S3, S4, S5, S6, Result>(
s5: Selector<State, S5>,
s6: Selector<State, S6>,
projector: (s1: S1, s2: S2, s3: S3, s4: S4, s5: S5, s6: S6) => Result
): MemoizedSelector<State, Result>;
): MemoizedSelector<
State,
Result,
(s1: S1, s2: S2, s3: S3, s4: S4, s5: S5, s6: S6) => Result
>;
export function createSelector<State, S1, S2, S3, S4, S5, S6, S7, Result>(
s1: Selector<State, S1>,
s2: Selector<State, S2>,
Expand All @@ -170,7 +197,11 @@ export function createSelector<State, S1, S2, S3, S4, S5, S6, S7, Result>(
s6: Selector<State, S6>,
s7: Selector<State, S7>,
projector: (s1: S1, s2: S2, s3: S3, s4: S4, s5: S5, s6: S6, s7: S7) => Result
): MemoizedSelector<State, Result>;
): MemoizedSelector<
State,
Result,
(s1: S1, s2: S2, s3: S3, s4: S4, s5: S5, s6: S6, s7: S7) => Result
>;
export function createSelector<State, S1, S2, S3, S4, S5, S6, S7, S8, Result>(
s1: Selector<State, S1>,
s2: Selector<State, S2>,
Expand All @@ -190,15 +221,19 @@ export function createSelector<State, S1, S2, S3, S4, S5, S6, S7, S8, Result>(
s7: S7,
s8: S8
) => Result
): MemoizedSelector<State, Result>;
): MemoizedSelector<
State,
Result,
(s1: S1, s2: S2, s3: S3, s4: S4, s5: S5, s6: S6, s7: S7, s8: S8) => Result
>;

export function createSelector<State, Slices extends unknown[], Result>(
...args: [...slices: Selector<State, unknown>[], projector: unknown] &
[
...slices: { [i in keyof Slices]: Selector<State, Slices[i]> },
projector: (...s: Slices) => Result
]
): MemoizedSelector<State, Result>;
): MemoizedSelector<State, Result, (...s: Slices) => Result>;

/**
* @deprecated Selectors with props are deprecated, for more info see {@link https://github.com/ngrx/platform/issues/2980 Github Issue}
Expand Down Expand Up @@ -346,7 +381,7 @@ export function createSelector<State, Slices extends unknown[], Result>(
selectors: Selector<State, unknown>[] &
[...{ [i in keyof Slices]: Selector<State, Slices[i]> }],
projector: (...s: Slices) => Result
): MemoizedSelector<State, Result>;
): MemoizedSelector<State, Result, (...s: Slices) => Result>;

/**
* @deprecated Selectors with props are deprecated, for more info see {@link https://github.com/ngrx/platform/issues/2980 Github Issue}
Expand Down