From db52fe236c8cc854dec1497fc94f76045668a6ef Mon Sep 17 00:00:00 2001 From: David <25187726+david-shortman@users.noreply.github.com> Date: Sat, 17 Sep 2022 17:57:08 -0400 Subject: [PATCH 1/5] feat(store): strict projectors --- .../router-store/spec/types/selectors.spec.ts | 22 +++---- modules/store/spec/types/selector.spec.ts | 51 +++++++++++++++++ modules/store/src/selector.ts | 57 +++++++++++++++---- 3 files changed, 105 insertions(+), 25 deletions(-) create mode 100644 modules/store/spec/types/selector.spec.ts diff --git a/modules/router-store/spec/types/selectors.spec.ts b/modules/router-store/spec/types/selectors.spec.ts index 3fd7f972f0..43c88e6a71 100644 --- a/modules/router-store/spec/types/selectors.spec.ts +++ b/modules/router-store/spec/types/selectors.spec.ts @@ -38,10 +38,7 @@ describe('router selectors', () => { selectCurrentRoute, route => route ); - `).toInfer( - 'selector', - 'MemoizedSelector>' - ); + `).toInfer('selector', 'MemoizedSelector any>'); }); it('selectQueryParams should return Params', () => { @@ -52,7 +49,7 @@ describe('router selectors', () => { ); `).toInfer( 'selector', - 'MemoizedSelector>' + 'MemoizedSelector Params>' ); }); @@ -65,7 +62,7 @@ describe('router selectors', () => { ); `).toInfer( 'selector', - 'MemoizedSelector>' + 'MemoizedSelector string>' ); }); @@ -77,7 +74,7 @@ describe('router selectors', () => { ); `).toInfer( 'selector', - 'MemoizedSelector>' + 'MemoizedSelector Params>' ); }); @@ -90,7 +87,7 @@ describe('router selectors', () => { ); `).toInfer( 'selector', - 'MemoizedSelector>' + 'MemoizedSelector string>' ); }); @@ -100,10 +97,7 @@ describe('router selectors', () => { selectRouteData, data => data ); - `).toInfer( - 'selector', - 'MemoizedSelector>' - ); + `).toInfer('selector', 'MemoizedSelector Data>'); }); it('selectUrl should return string', () => { @@ -114,7 +108,7 @@ describe('router selectors', () => { ); `).toInfer( 'selector', - 'MemoizedSelector>' + 'MemoizedSelector string>' ); }); @@ -126,7 +120,7 @@ describe('router selectors', () => { ); `).toInfer( 'selector', - 'MemoizedSelector>' + 'MemoizedSelector string>' ); }); }); diff --git a/modules/store/spec/types/selector.spec.ts b/modules/store/spec/types/selector.spec.ts new file mode 100644 index 0000000000..77fcf08cd6 --- /dev/null +++ b/modules/store/spec/types/selector.spec.ts @@ -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(); + `).toSucceed(); + }); + it('should not require parameters for existing explicitly loosely typed selectors', () => { + expectSnippet(` + const selectTest: MemoizedSelector< + unknown, + number, + DefaultProjectorFn + > = createSelector( + () => 'one', + () => 2, + (one, two) => 3 + ); + selectTest.projector(); + `).toSucceed(); + }); + }); +}); diff --git a/modules/store/src/selector.ts b/modules/store/src/selector.ts index ec2ce5ac8c..460a61be63 100644 --- a/modules/store/src/selector.ts +++ b/modules/store/src/selector.ts @@ -17,13 +17,32 @@ export type ComparatorFn = (a: any, b: any) => boolean; export type DefaultProjectorFn = (...args: any[]) => T; +type ProjectorStrictness = 'strict'; + +type SelectorProjectorArgs< + StrictnessConfig extends ProjectorStrictness, + ProjectorArgs +> = StrictnessConfig extends 'strict' + ? ProjectorArgs extends unknown[] + ? ProjectorArgs + : any[] + : any[]; + +type SelectorProjectorFn = ProjectorFn extends ( + ...args: infer ProjectorArgs +) => infer ProjectorResult + ? ( + ...args: SelectorProjectorArgs + ) => ProjectorResult + : ProjectorFn; + export interface MemoizedSelector< State, Result, ProjectorFn = DefaultProjectorFn > extends Selector { release(): void; - projector: ProjectorFn; + projector: SelectorProjectorFn; setResult: (result?: Result) => void; clearResult: () => void; } @@ -125,25 +144,25 @@ export function defaultMemoize( export function createSelector( s1: Selector, projector: (s1: S1) => Result -): MemoizedSelector; +): MemoizedSelector Result>; export function createSelector( s1: Selector, s2: Selector, projector: (s1: S1, s2: S2) => Result -): MemoizedSelector; +): MemoizedSelector Result>; export function createSelector( s1: Selector, s2: Selector, s3: Selector, projector: (s1: S1, s2: S2, s3: S3) => Result -): MemoizedSelector; +): MemoizedSelector Result>; export function createSelector( s1: Selector, s2: Selector, s3: Selector, s4: Selector, projector: (s1: S1, s2: S2, s3: S3, s4: S4) => Result -): MemoizedSelector; +): MemoizedSelector Result>; export function createSelector( s1: Selector, s2: Selector, @@ -151,7 +170,11 @@ export function createSelector( s4: Selector, s5: Selector, projector: (s1: S1, s2: S2, s3: S3, s4: S4, s5: S5) => Result -): MemoizedSelector; +): MemoizedSelector< + State, + Result, + (s1: S1, s2: S2, s3: S3, s4: S4, s5: S5) => Result +>; export function createSelector( s1: Selector, s2: Selector, @@ -160,7 +183,11 @@ export function createSelector( s5: Selector, s6: Selector, projector: (s1: S1, s2: S2, s3: S3, s4: S4, s5: S5, s6: S6) => Result -): MemoizedSelector; +): MemoizedSelector< + State, + Result, + (s1: S1, s2: S2, s3: S3, s4: S4, s5: S5, s6: S6) => Result +>; export function createSelector( s1: Selector, s2: Selector, @@ -170,7 +197,11 @@ export function createSelector( s6: Selector, s7: Selector, projector: (s1: S1, s2: S2, s3: S3, s4: S4, s5: S5, s6: S6, s7: S7) => Result -): MemoizedSelector; +): MemoizedSelector< + State, + Result, + (s1: S1, s2: S2, s3: S3, s4: S4, s5: S5, s6: S6, s7: S7) => Result +>; export function createSelector( s1: Selector, s2: Selector, @@ -190,7 +221,11 @@ export function createSelector( s7: S7, s8: S8 ) => Result -): MemoizedSelector; +): MemoizedSelector< + State, + Result, + (s1: S1, s2: S2, s3: S3, s4: S4, s5: S5, s6: S6, s7: S7, s8: S8) => Result +>; export function createSelector( ...args: [...slices: Selector[], projector: unknown] & @@ -198,7 +233,7 @@ export function createSelector( ...slices: { [i in keyof Slices]: Selector }, projector: (...s: Slices) => Result ] -): MemoizedSelector; +): MemoizedSelector Result>; /** * @deprecated Selectors with props are deprecated, for more info see {@link https://github.com/ngrx/platform/issues/2980 Github Issue} @@ -346,7 +381,7 @@ export function createSelector( selectors: Selector[] & [...{ [i in keyof Slices]: Selector }], projector: (...s: Slices) => Result -): MemoizedSelector; +): MemoizedSelector Result>; /** * @deprecated Selectors with props are deprecated, for more info see {@link https://github.com/ngrx/platform/issues/2980 Github Issue} From 229dcc24d7ec93c2ae7fe9d80cb8e2d98ab4f1a2 Mon Sep 17 00:00:00 2001 From: David <25187726+david-shortman@users.noreply.github.com> Date: Sat, 17 Sep 2022 18:15:03 -0400 Subject: [PATCH 2/5] use loose projector in test --- modules/store/spec/selector.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/store/spec/selector.spec.ts b/modules/store/spec/selector.spec.ts index 747a8ef7e6..81daff1517 100644 --- a/modules/store/spec/selector.spec.ts +++ b/modules/store/spec/selector.spec.ts @@ -56,7 +56,7 @@ describe('Selectors', () => { const selector = createSelector(incrementOne, incrementTwo, projectFn); - expect(selector.projector()).toBe(2); + expect(selector.projector()).toBe(2); selector.setResult(5); From d187e59d4967de682521f8ec834ca161bb3f6630 Mon Sep 17 00:00:00 2001 From: David <25187726+david-shortman@users.noreply.github.com> Date: Sat, 22 Oct 2022 21:14:15 +0000 Subject: [PATCH 3/5] remove "strictness" generic --- modules/store/spec/selector.spec.ts | 2 +- modules/store/spec/types/selector.spec.ts | 10 ---------- modules/store/src/selector.ts | 15 +-------------- 3 files changed, 2 insertions(+), 25 deletions(-) diff --git a/modules/store/spec/selector.spec.ts b/modules/store/spec/selector.spec.ts index 81daff1517..62cfed3974 100644 --- a/modules/store/spec/selector.spec.ts +++ b/modules/store/spec/selector.spec.ts @@ -56,7 +56,7 @@ describe('Selectors', () => { const selector = createSelector(incrementOne, incrementTwo, projectFn); - expect(selector.projector()).toBe(2); + expect((selector.projector as any)()).toBe(2); selector.setResult(5); diff --git a/modules/store/spec/types/selector.spec.ts b/modules/store/spec/types/selector.spec.ts index 77fcf08cd6..9e6448f764 100644 --- a/modules/store/spec/types/selector.spec.ts +++ b/modules/store/spec/types/selector.spec.ts @@ -23,16 +23,6 @@ describe('createSelector()', () => { 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(); - `).toSucceed(); - }); it('should not require parameters for existing explicitly loosely typed selectors', () => { expectSnippet(` const selectTest: MemoizedSelector< diff --git a/modules/store/src/selector.ts b/modules/store/src/selector.ts index 460a61be63..7a897d6115 100644 --- a/modules/store/src/selector.ts +++ b/modules/store/src/selector.ts @@ -17,23 +17,10 @@ export type ComparatorFn = (a: any, b: any) => boolean; export type DefaultProjectorFn = (...args: any[]) => T; -type ProjectorStrictness = 'strict'; - -type SelectorProjectorArgs< - StrictnessConfig extends ProjectorStrictness, - ProjectorArgs -> = StrictnessConfig extends 'strict' - ? ProjectorArgs extends unknown[] - ? ProjectorArgs - : any[] - : any[]; - type SelectorProjectorFn = ProjectorFn extends ( ...args: infer ProjectorArgs ) => infer ProjectorResult - ? ( - ...args: SelectorProjectorArgs - ) => ProjectorResult + ? (...args: ProjectorArgs) => ProjectorResult : ProjectorFn; export interface MemoizedSelector< From 7d39d0dbdd9977a6944ada2d6be0cfd3dd3fec20 Mon Sep 17 00:00:00 2001 From: Tim Deschryver <28659384+timdeschryver@users.noreply.github.com> Date: Wed, 2 Nov 2022 13:15:26 +0100 Subject: [PATCH 4/5] Update modules/store/src/selector.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Marko Stanimirović --- modules/store/src/selector.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/modules/store/src/selector.ts b/modules/store/src/selector.ts index 7a897d6115..27d678b8a9 100644 --- a/modules/store/src/selector.ts +++ b/modules/store/src/selector.ts @@ -17,12 +17,6 @@ export type ComparatorFn = (a: any, b: any) => boolean; export type DefaultProjectorFn = (...args: any[]) => T; -type SelectorProjectorFn = ProjectorFn extends ( - ...args: infer ProjectorArgs -) => infer ProjectorResult - ? (...args: ProjectorArgs) => ProjectorResult - : ProjectorFn; - export interface MemoizedSelector< State, Result, From 57276e6c1c4b093a68cdc7653df91df5121ea91f Mon Sep 17 00:00:00 2001 From: Tim Deschryver <28659384+timdeschryver@users.noreply.github.com> Date: Wed, 2 Nov 2022 13:15:32 +0100 Subject: [PATCH 5/5] Update modules/store/src/selector.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Marko Stanimirović --- modules/store/src/selector.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/store/src/selector.ts b/modules/store/src/selector.ts index 27d678b8a9..81acb35740 100644 --- a/modules/store/src/selector.ts +++ b/modules/store/src/selector.ts @@ -23,7 +23,7 @@ export interface MemoizedSelector< ProjectorFn = DefaultProjectorFn > extends Selector { release(): void; - projector: SelectorProjectorFn; + projector: ProjectorFn; setResult: (result?: Result) => void; clearResult: () => void; }