Skip to content

Commit

Permalink
feat(store): strict projector for selectors with props (#3640)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
timdeschryver authored Nov 2, 2022
1 parent f825357 commit 351459f
Show file tree
Hide file tree
Showing 3 changed files with 179 additions and 20 deletions.
8 changes: 4 additions & 4 deletions modules/store/spec/selector.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down
40 changes: 40 additions & 0 deletions modules/store/spec/types/selector.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<number>
> = createSelector(
() => 'one',
() => 2,
(one, two, props) => 3
);
selectTest.projector();
`).toSucceed();
});
});
});
151 changes: 135 additions & 16 deletions modules/store/src/selector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -222,7 +223,12 @@ export function createSelector<State, Slices extends unknown[], Result>(
export function createSelector<State, Props, S1, Result>(
s1: SelectorWithProps<State, Props, S1>,
projector: (s1: S1, props: Props) => Result
): MemoizedSelectorWithProps<State, Props, Result>;
): 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}
Expand All @@ -231,7 +237,12 @@ export function createSelector<State, Props, S1, S2, Result>(
s1: SelectorWithProps<State, Props, S1>,
s2: SelectorWithProps<State, Props, S2>,
projector: (s1: S1, s2: S2, props: Props) => Result
): MemoizedSelectorWithProps<State, Props, Result>;
): 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}
Expand All @@ -241,7 +252,12 @@ export function createSelector<State, Props, S1, S2, S3, Result>(
s2: SelectorWithProps<State, Props, S2>,
s3: SelectorWithProps<State, Props, S3>,
projector: (s1: S1, s2: S2, s3: S3, props: Props) => Result
): MemoizedSelectorWithProps<State, Props, Result>;
): 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}
Expand All @@ -252,7 +268,12 @@ export function createSelector<State, Props, S1, S2, S3, S4, Result>(
s3: SelectorWithProps<State, Props, S3>,
s4: SelectorWithProps<State, Props, S4>,
projector: (s1: S1, s2: S2, s3: S3, s4: S4, props: Props) => Result
): MemoizedSelectorWithProps<State, Props, Result>;
): 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}
Expand All @@ -264,7 +285,12 @@ export function createSelector<State, Props, S1, S2, S3, S4, S5, Result>(
s4: SelectorWithProps<State, Props, S4>,
s5: SelectorWithProps<State, Props, S5>,
projector: (s1: S1, s2: S2, s3: S3, s4: S4, s5: S5, props: Props) => Result
): MemoizedSelectorWithProps<State, Props, Result>;
): 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}
Expand All @@ -285,7 +311,12 @@ export function createSelector<State, Props, S1, S2, S3, S4, S5, S6, Result>(
s6: S6,
props: Props
) => Result
): MemoizedSelectorWithProps<State, Props, Result>;
): 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}
Expand Down Expand Up @@ -319,7 +350,21 @@ export function createSelector<
s7: S7,
props: Props
) => Result
): MemoizedSelectorWithProps<State, Props, Result>;
): 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}
Expand Down Expand Up @@ -356,7 +401,22 @@ export function createSelector<
s8: S8,
props: Props
) => Result
): MemoizedSelectorWithProps<State, Props, Result>;
): 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<State, Slices extends unknown[], Result>(
selectors: Selector<State, unknown>[] &
Expand All @@ -370,7 +430,12 @@ export function createSelector<State, Slices extends unknown[], Result>(
export function createSelector<State, Props, S1, Result>(
selectors: [SelectorWithProps<State, Props, S1>],
projector: (s1: S1, props: Props) => Result
): MemoizedSelectorWithProps<State, Props, Result>;
): 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}
Expand All @@ -381,7 +446,12 @@ export function createSelector<State, Props, S1, S2, Result>(
SelectorWithProps<State, Props, S2>
],
projector: (s1: S1, s2: S2, props: Props) => Result
): MemoizedSelectorWithProps<State, Props, Result>;
): 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}
Expand All @@ -393,7 +463,12 @@ export function createSelector<State, Props, S1, S2, S3, Result>(
SelectorWithProps<State, Props, S3>
],
projector: (s1: S1, s2: S2, s3: S3, props: Props) => Result
): MemoizedSelectorWithProps<State, Props, Result>;
): 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}
Expand All @@ -406,7 +481,12 @@ export function createSelector<State, Props, S1, S2, S3, S4, Result>(
SelectorWithProps<State, Props, S4>
],
projector: (s1: S1, s2: S2, s3: S3, s4: S4, props: Props) => Result
): MemoizedSelectorWithProps<State, Props, Result>;
): 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}
Expand All @@ -420,7 +500,12 @@ export function createSelector<State, Props, S1, S2, S3, S4, S5, Result>(
SelectorWithProps<State, Props, S5>
],
projector: (s1: S1, s2: S2, s3: S3, s4: S4, s5: S5, props: Props) => Result
): MemoizedSelectorWithProps<State, Props, Result>;
): 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}
Expand All @@ -443,7 +528,12 @@ export function createSelector<State, Props, S1, S2, S3, S4, S5, S6, Result>(
s6: S6,
props: Props
) => Result
): MemoizedSelectorWithProps<State, Props, Result>;
): 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}
Expand Down Expand Up @@ -479,7 +569,21 @@ export function createSelector<
s7: S7,
props: Props
) => Result
): MemoizedSelectorWithProps<State, Props, Result>;
): 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}
Expand Down Expand Up @@ -518,7 +622,22 @@ export function createSelector<
s8: S8,
props: Props
) => Result
): MemoizedSelectorWithProps<State, Props, Result>;
): 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[]
Expand Down

0 comments on commit 351459f

Please sign in to comment.