From 351459f27ed772270f775974e68f45a7a0452019 Mon Sep 17 00:00:00 2001 From: Tim Deschryver <28659384+timdeschryver@users.noreply.github.com> Date: Wed, 2 Nov 2022 20:03:59 +0100 Subject: [PATCH] feat(store): strict projector for selectors with props (#3640) BREAKING CHANGE: The projector method has become strict BEFORE: You could pass any arguments to the projector method const selector = createSelector( selectString, // returning a string selectNumber, // returning a number (s, n, prefix: string) => { return prefix + s.repeat(n); } ) // you could pass any argument selector.projector(1, 'a', true); AFTER: const selector = createSelector( selectString, // returning a string selectNumber, // returning a number (s, n, prefix: string) => { return prefix + s.repeat(n); } ) // this throws selector.projector(1, 'a', true); // this does not throw because the arguments have the correct type selector.projector(1, 'a', 'prefix'); Closes #3571 --- modules/store/spec/selector.spec.ts | 8 +- modules/store/spec/types/selector.spec.ts | 40 ++++++ modules/store/src/selector.ts | 151 +++++++++++++++++++--- 3 files changed, 179 insertions(+), 20 deletions(-) diff --git a/modules/store/spec/selector.spec.ts b/modules/store/spec/selector.spec.ts index 62cfed3974..b7d4cabc0c 100644 --- a/modules/store/spec/selector.spec.ts +++ b/modules/store/spec/selector.spec.ts @@ -198,11 +198,11 @@ describe('Selectors', () => { }, projectFn ); - selector.projector('', '', 47); + selector.projector('', '', 47, 'prop'); expect(incrementOne).not.toHaveBeenCalled(); expect(incrementTwo).not.toHaveBeenCalled(); - expect(projectFn).toHaveBeenCalledWith('', '', 47); + expect(projectFn).toHaveBeenCalledWith('', '', 47, 'prop'); }); it('should call the projector function when the state changes', () => { @@ -387,11 +387,11 @@ describe('Selectors', () => { projectFn ); - selector.projector('', '', 47); + selector.projector('', '', 47, 'prop'); expect(incrementOne).not.toHaveBeenCalled(); expect(incrementTwo).not.toHaveBeenCalled(); - expect(projectFn).toHaveBeenCalledWith('', '', 47); + expect(projectFn).toHaveBeenCalledWith('', '', 47, 'prop'); }); it('should call the projector function when the state changes', () => { diff --git a/modules/store/spec/types/selector.spec.ts b/modules/store/spec/types/selector.spec.ts index 9e6448f764..caf4457967 100644 --- a/modules/store/spec/types/selector.spec.ts +++ b/modules/store/spec/types/selector.spec.ts @@ -39,3 +39,43 @@ describe('createSelector()', () => { }); }); }); + +describe('createSelector() with props', () => { + const expectSnippet = expecter( + (code) => ` + import {createSelector} from '@ngrx/store'; + import { MemoizedSelectorWithProps, DefaultProjectorFn } from '@ngrx/store'; + + ${code} + `, + compilerOptions() + ); + + describe('projector', () => { + it('should require correct arguments by default', () => { + expectSnippet(` + const selectTest = createSelector( + () => 'one', + () => 2, + (one, two, props) => 3 + ); + selectTest.projector(); + `).toFail(/Expected 3 arguments, but got 0./); + }); + it('should not require parameters for existing explicitly loosely typed selectors', () => { + expectSnippet(` + const selectTest: MemoizedSelectorWithProps< + unknown, + number, + any, + DefaultProjectorFn + > = createSelector( + () => 'one', + () => 2, + (one, two, props) => 3 + ); + selectTest.projector(); + `).toSucceed(); + }); + }); +}); diff --git a/modules/store/src/selector.ts b/modules/store/src/selector.ts index 81acb35740..a1ad099d2d 100644 --- a/modules/store/src/selector.ts +++ b/modules/store/src/selector.ts @@ -91,6 +91,7 @@ export function defaultMemoize( } /* eslint-disable prefer-rest-params, prefer-spread */ + // disabled because of the use of `arguments` function memoized(): any { if (overrideResult !== undefined) { @@ -222,7 +223,12 @@ export function createSelector( export function createSelector( s1: SelectorWithProps, projector: (s1: S1, props: Props) => Result -): MemoizedSelectorWithProps; +): MemoizedSelectorWithProps< + State, + Props, + Result, + (s1: S1, props: Props) => Result +>; /** * @deprecated Selectors with props are deprecated, for more info see {@link https://github.com/ngrx/platform/issues/2980 Github Issue} @@ -231,7 +237,12 @@ export function createSelector( s1: SelectorWithProps, s2: SelectorWithProps, projector: (s1: S1, s2: S2, props: Props) => Result -): MemoizedSelectorWithProps; +): MemoizedSelectorWithProps< + State, + Props, + Result, + (s1: S1, s2: S2, props: Props) => Result +>; /** * @deprecated Selectors with props are deprecated, for more info see {@link https://github.com/ngrx/platform/issues/2980 Github Issue} @@ -241,7 +252,12 @@ export function createSelector( s2: SelectorWithProps, s3: SelectorWithProps, projector: (s1: S1, s2: S2, s3: S3, props: Props) => Result -): MemoizedSelectorWithProps; +): MemoizedSelectorWithProps< + State, + Props, + Result, + (s1: S1, s2: S2, s3: S3, props: Props) => Result +>; /** * @deprecated Selectors with props are deprecated, for more info see {@link https://github.com/ngrx/platform/issues/2980 Github Issue} @@ -252,7 +268,12 @@ export function createSelector( s3: SelectorWithProps, s4: SelectorWithProps, projector: (s1: S1, s2: S2, s3: S3, s4: S4, props: Props) => Result -): MemoizedSelectorWithProps; +): MemoizedSelectorWithProps< + State, + Props, + Result, + (s1: S1, s2: S2, s3: S3, s4: S4, props: Props) => Result +>; /** * @deprecated Selectors with props are deprecated, for more info see {@link https://github.com/ngrx/platform/issues/2980 Github Issue} @@ -264,7 +285,12 @@ export function createSelector( s4: SelectorWithProps, s5: SelectorWithProps, projector: (s1: S1, s2: S2, s3: S3, s4: S4, s5: S5, props: Props) => Result -): MemoizedSelectorWithProps; +): MemoizedSelectorWithProps< + State, + Props, + Result, + (s1: S1, s2: S2, s3: S3, s4: S4, s5: S5, props: Props) => Result +>; /** * @deprecated Selectors with props are deprecated, for more info see {@link https://github.com/ngrx/platform/issues/2980 Github Issue} @@ -285,7 +311,12 @@ export function createSelector( s6: S6, props: Props ) => Result -): MemoizedSelectorWithProps; +): MemoizedSelectorWithProps< + State, + Props, + Result, + (s1: S1, s2: S2, s3: S3, s4: S4, s5: S5, s6: S6, props: Props) => Result +>; /** * @deprecated Selectors with props are deprecated, for more info see {@link https://github.com/ngrx/platform/issues/2980 Github Issue} @@ -319,7 +350,21 @@ export function createSelector< s7: S7, props: Props ) => Result -): MemoizedSelectorWithProps; +): MemoizedSelectorWithProps< + State, + Props, + Result, + ( + s1: S1, + s2: S2, + s3: S3, + s4: S4, + s5: S5, + s6: S6, + s7: S7, + props: Props + ) => Result +>; /** * @deprecated Selectors with props are deprecated, for more info see {@link https://github.com/ngrx/platform/issues/2980 Github Issue} @@ -356,7 +401,22 @@ export function createSelector< s8: S8, props: Props ) => Result -): MemoizedSelectorWithProps; +): MemoizedSelectorWithProps< + State, + Props, + Result, + ( + s1: S1, + s2: S2, + s3: S3, + s4: S4, + s5: S5, + s6: S6, + s7: S7, + s8: S8, + props: Props + ) => Result +>; export function createSelector( selectors: Selector[] & @@ -370,7 +430,12 @@ export function createSelector( export function createSelector( selectors: [SelectorWithProps], projector: (s1: S1, props: Props) => Result -): MemoizedSelectorWithProps; +): MemoizedSelectorWithProps< + State, + Props, + Result, + (s1: S1, props: Props) => Result +>; /** * @deprecated Selectors with props are deprecated, for more info see {@link https://github.com/ngrx/platform/issues/2980 Github Issue} @@ -381,7 +446,12 @@ export function createSelector( SelectorWithProps ], projector: (s1: S1, s2: S2, props: Props) => Result -): MemoizedSelectorWithProps; +): MemoizedSelectorWithProps< + State, + Props, + Result, + (s1: S1, s2: S2, props: Props) => Result +>; /** * @deprecated Selectors with props are deprecated, for more info see {@link https://github.com/ngrx/platform/issues/2980 Github Issue} @@ -393,7 +463,12 @@ export function createSelector( SelectorWithProps ], projector: (s1: S1, s2: S2, s3: S3, props: Props) => Result -): MemoizedSelectorWithProps; +): MemoizedSelectorWithProps< + State, + Props, + Result, + (s1: S1, s2: S2, s3: S3, props: Props) => Result +>; /** * @deprecated Selectors with props are deprecated, for more info see {@link https://github.com/ngrx/platform/issues/2980 Github Issue} @@ -406,7 +481,12 @@ export function createSelector( SelectorWithProps ], projector: (s1: S1, s2: S2, s3: S3, s4: S4, props: Props) => Result -): MemoizedSelectorWithProps; +): MemoizedSelectorWithProps< + State, + Props, + Result, + (s1: S1, s2: S2, s3: S3, s4: S4, props: Props) => Result +>; /** * @deprecated Selectors with props are deprecated, for more info see {@link https://github.com/ngrx/platform/issues/2980 Github Issue} @@ -420,7 +500,12 @@ export function createSelector( SelectorWithProps ], projector: (s1: S1, s2: S2, s3: S3, s4: S4, s5: S5, props: Props) => Result -): MemoizedSelectorWithProps; +): MemoizedSelectorWithProps< + State, + Props, + Result, + (s1: S1, s2: S2, s3: S3, s4: S4, s5: S5, props: Props) => Result +>; /** * @deprecated Selectors with props are deprecated, for more info see {@link https://github.com/ngrx/platform/issues/2980 Github Issue} @@ -443,7 +528,12 @@ export function createSelector( s6: S6, props: Props ) => Result -): MemoizedSelectorWithProps; +): MemoizedSelectorWithProps< + State, + Props, + Result, + (s1: S1, s2: S2, s3: S3, s4: S4, s5: S5, s6: S6, props: Props) => Result +>; /** * @deprecated Selectors with props are deprecated, for more info see {@link https://github.com/ngrx/platform/issues/2980 Github Issue} @@ -479,7 +569,21 @@ export function createSelector< s7: S7, props: Props ) => Result -): MemoizedSelectorWithProps; +): MemoizedSelectorWithProps< + State, + Props, + Result, + ( + s1: S1, + s2: S2, + s3: S3, + s4: S4, + s5: S5, + s6: S6, + s7: S7, + props: Props + ) => Result +>; /** * @deprecated Selectors with props are deprecated, for more info see {@link https://github.com/ngrx/platform/issues/2980 Github Issue} @@ -518,7 +622,22 @@ export function createSelector< s8: S8, props: Props ) => Result -): MemoizedSelectorWithProps; +): MemoizedSelectorWithProps< + State, + Props, + Result, + ( + s1: S1, + s2: S2, + s3: S3, + s4: S4, + s5: S5, + s6: S6, + s7: S7, + s8: S8, + props: Props + ) => Result +>; export function createSelector( ...input: any[]