diff --git a/modules/component-store/spec/types/component-store.types.spec.ts b/modules/component-store/spec/types/component-store.types.spec.ts index 6677ed077f..baeeae11e6 100644 --- a/modules/component-store/spec/types/component-store.types.spec.ts +++ b/modules/component-store/spec/types/component-store.types.spec.ts @@ -345,5 +345,201 @@ describe('ComponentStore types', () => { componentStore.updater((state, v: string) => ({ ...state }))(number$); }); }); + + describe('catches excess properties', () => { + it('when extra property is returned with spread', () => { + const componentStore = new ComponentStore({ + prop: 'init', + prop2: 'yeah!', + }); + componentStore.updater( + // @ts-expect-error updater callback return type must exactly match the state type. Remove excess properties. + (state, v: string) => ({ ...state, extraProp: 'bad' }) + ); + }); + + it('when extra property is returned with explicit object', () => { + const componentStore = new ComponentStore({ + prop: 'init', + prop2: 'yeah!', + }); + componentStore.updater( + // @ts-expect-error updater callback return type must exactly match the state type. Remove excess properties. + (state, v: string) => ({ + prop: v, + prop2: state.prop2, + extraProp: 'bad', + }) + ); + }); + + it('when extra property is returned from void updater', () => { + const componentStore = new ComponentStore({ + prop: 'init', + prop2: 'yeah!', + }); + componentStore.updater( + // @ts-expect-error updater callback return type must exactly match the state type. Remove excess properties. + (state) => ({ ...state, extraProp: true }) + ); + }); + + it('when required property is missing', () => { + const componentStore = new ComponentStore({ + prop: 'init', + prop2: 'yeah!', + }); + componentStore.updater( + // @ts-expect-error Property 'prop2' is missing in type '{ prop: string; }' + (state, v: string) => ({ prop: v }) + ); + }); + + it('when property has wrong type', () => { + const componentStore = new ComponentStore({ + prop: 'init', + prop2: 'yeah!', + }); + componentStore.updater( + // @ts-expect-error Type 'number' is not assignable to type 'string' + (state, v: string) => ({ ...state, prop: 123 }) + ); + }); + + it('allows spread with override', () => { + const componentStore = new ComponentStore({ + prop: 'init', + prop2: 'yeah!', + }); + const sub = componentStore.updater((state, v: string) => ({ + ...state, + prop: v, + }))('test'); + expectTypeOf(sub).toEqualTypeOf(); + }); + + it('allows full explicit return matching all state keys', () => { + const componentStore = new ComponentStore({ + prop: 'init', + prop2: 'yeah!', + }); + const sub = componentStore.updater((state, v: string) => ({ + prop: v, + prop2: state.prop2, + }))('test'); + expectTypeOf(sub).toEqualTypeOf(); + }); + + it('allows void updater with spread return', () => { + const componentStore = new ComponentStore({ + prop: 'init', + prop2: 'yeah!', + }); + const v = componentStore.updater((state) => ({ + ...state, + prop: 'updated', + }))(); + expectTypeOf(v).toBeVoid(); + }); + + it('allows direct state return', () => { + const componentStore = new ComponentStore({ + prop: 'init', + prop2: 'yeah!', + }); + const v = componentStore.updater((state) => state)(); + expectTypeOf(v).toBeVoid(); + }); + }); + + describe('with a generic state type parameter', () => { + class GenericStore extends ComponentStore { + // Keys guaranteed by the constraint can be overridden via spread. + readonly setIdViaSpread = this.updater((state, id: string) => ({ + ...state, + id, + })); + + readonly returnState = this.updater((state) => state); + + readonly copyState = this.updater((state) => ({ ...state })); + + readonly replaceViaDirectReturn = this.updater( + (_state, next: T) => next + ); + + readonly setIdViaAssertion = this.updater( + (state, id: string) => ({ ...state, id }) as T + ); + + // Keys not guaranteed by the constraint are excess properties. + readonly setExtra = this.updater( + // @ts-expect-error updater callback return type must exactly match the state type. Remove excess properties. + (state, extra: string) => ({ ...state, extra }) + ); + } + + class ListStore extends ComponentStore<{ + items: Item[]; + loading: boolean; + }> { + readonly setItems = this.updater((state, items: Item[]) => ({ + ...state, + items, + loading: false, + })); + } + + class ExtendableStore extends ComponentStore< + { loading: boolean } & T + > { + readonly setLoading = this.updater((state) => ({ + ...state, + loading: true, + })); + } + + it('allows overriding known keys and catches excess keys', () => { + expectTypeOf(GenericStore).toBeConstructibleWith({ id: '1' }); + expectTypeOf(ListStore).toBeConstructibleWith({ + items: [], + loading: false, + }); + expectTypeOf(ExtendableStore).toBeConstructibleWith({ loading: false }); + }); + }); + + describe('with union state', () => { + type FetchState = + { status: 'idle' } | { status: 'loaded'; data: string[] }; + + it('allows switching to a union member with its own keys', () => { + const componentStore = new ComponentStore({ + status: 'idle', + }); + componentStore.updater((_, data: string[]) => ({ + status: 'loaded' as const, + data, + })); + }); + + it('catches keys that exist on no union member', () => { + const componentStore = new ComponentStore({ + status: 'idle', + }); + componentStore.updater( + // @ts-expect-error updater callback return type must exactly match the state type. Remove excess properties. + () => ({ status: 'idle' as const, bogus: true }) + ); + }); + }); + + it('catches excess properties returned from one branch of a conditional', () => { + const componentStore = new ComponentStore({ prop: 'init' }); + componentStore.updater( + // @ts-expect-error updater callback return type must exactly match the state type. Remove excess properties. + (state, flag: boolean) => (flag ? { ...state, extra: true } : state) + ); + }); }); }); diff --git a/modules/component-store/spec/types/regression.types.spec.ts b/modules/component-store/spec/types/regression.types.spec.ts index bb8600a7d9..5f5afd1c33 100644 --- a/modules/component-store/spec/types/regression.types.spec.ts +++ b/modules/component-store/spec/types/regression.types.spec.ts @@ -30,4 +30,26 @@ describe('regression component-store', () => { } } }); + + describe('updater exact return type', () => { + it('should work with state containing optional properties', () => { + const store = new ComponentStore<{ req: string; opt?: number }>({ + req: 'a', + }); + store.updater((state) => ({ req: 'b' }))(); + }); + + it('should work with state containing index signature', () => { + const store = new ComponentStore<{ [key: string]: number }>({}); + store.updater((state, v: number) => ({ ...state, newKey: v }))(5); + }); + + it('should catch excess properties with concrete state type', () => { + const store = new ComponentStore<{ name: string }>({ name: 'test' }); + store.updater( + // @ts-expect-error updater callback return type must exactly match the state type. Remove excess properties. + (state, v: string) => ({ ...state, name: v, extra: true }) + ); + }); + }); }); diff --git a/modules/component-store/src/component-store.ts b/modules/component-store/src/component-store.ts index 2e2fe12448..79f79631f1 100644 --- a/modules/component-store/src/component-store.ts +++ b/modules/component-store/src/component-store.ts @@ -40,6 +40,33 @@ import { import { isOnStateInitDefined, isOnStoreInitDefined } from './lifecycle_hooks'; import { toSignal } from '@angular/core/rxjs-interop'; +const excessPropertiesAreNotAllowedMsg = + 'updater callback return type must exactly match the state type. Remove excess properties.'; +type ExcessPropertiesAreNotAllowed = typeof excessPropertiesAreNotAllowedMsg; + +type KeysOfUnion = T extends unknown ? keyof T : never; +// Keys a returned object may carry: any key of any union member, plus numeric keys for string index signatures. +type AllowedStateKeys = + keyof S | KeysOfUnion | (string extends keyof S ? number : never); +// `K` is inferred from the returned object's keys and constrained to AllowedStateKeys. Constraint checks use +// assignability, which respects generic constraints (e.g. `T extends { loading: boolean }`). When a key is not +// allowed, inference falls back to the constraint and the leftover keys are typed as the error message. +type ExactStateReturn = [R] extends [ + null | undefined, +] + ? unknown + : // Fast path: no key outside the state's keys (also resolves for identical generic types, e.g. `return state`) + [Exclude, AllowedStateKeys>] extends [never] + ? unknown + : { [P in K & PropertyKey]?: unknown } & { + [ + P in Exclude< + KeysOfUnion, + K | AllowedStateKeys + > as `${Msg} Excess property: ${P & string}` + ]: never; + }; + export interface SelectConfig { debounce?: boolean; equal?: ValueEqualityFn; @@ -132,7 +159,16 @@ export class ComponentStore implements OnDestroy { ReturnType = OriginType extends void ? () => void : (observableOrValue: ValueType | Observable) => Subscription, - >(updaterFn: (state: T, value: OriginType) => T): ReturnType { + // Captures the actual return type to enforce exact state shape + R extends T = T, + // Keys of the returned object, vetted against the state's keys via the constraint + K extends AllowedStateKeys = never, + >( + updaterFn: ( + state: T, + value: OriginType + ) => R & ExactStateReturn + ): ReturnType { return (( observableOrValue?: OriginType | Observable ): Subscription => { @@ -210,9 +246,7 @@ export class ComponentStore implements OnDestroy { */ patchState( partialStateOrUpdaterFn: - | Partial - | Observable> - | ((state: T) => Partial) + Partial | Observable> | ((state: T) => Partial) ): void { const patchedState = typeof partialStateOrUpdaterFn === 'function' @@ -379,9 +413,8 @@ export class ComponentStore implements OnDestroy { // This type quickly became part of effect 'API' ProvidedType = void, // The actual origin$ type, which could be unknown, when not specified - OriginType extends - | Observable - | unknown = Observable, + OriginType extends Observable | unknown = + Observable, // Unwrapped actual type of the origin$ Observable, after default was applied ObservableType = OriginType extends Observable ? A : never, // Return either an optional callback or a function requiring specific types as inputs diff --git a/modules/eslint-plugin/spec/rules/component-store/updater-explicit-return-type.spec.ts b/modules/eslint-plugin/spec/rules/component-store/updater-explicit-return-type.spec.ts deleted file mode 100644 index 4ee9de511a..0000000000 --- a/modules/eslint-plugin/spec/rules/component-store/updater-explicit-return-type.spec.ts +++ /dev/null @@ -1,222 +0,0 @@ -import type { ESLintUtils } from '@typescript-eslint/utils'; -import type { - InvalidTestCase, - ValidTestCase, -} from '@typescript-eslint/rule-tester'; -import * as path from 'path'; -import rule, { - messageId, -} from '../../../src/rules/component-store/updater-explicit-return-type'; -import { ruleTester, fromFixture } from '../../utils'; - -type MessageIds = ESLintUtils.InferMessageIdsTypeFromRule; -type Options = ESLintUtils.InferOptionsTypeFromRule; - -const validConstructor: () => (string | ValidTestCase)[] = () => [ - ` -import { ComponentStore } from '@ngrx/component-store' - -class Ok extends ComponentStore { - readonly addMovie = this.updater( - (state, movie): MoviesState => ({ movies: [...state.movies, movie] }), - ) - - constructor() { - super({ movies: [] }) - } -}`, - ` -import { ComponentStore } from '@ngrx/component-store' - -class Ok1 extends ComponentStore { - readonly addMovie = this.updater( - (state, movie): MoviesState => ({ movies: [...state.movies, movie] }), - ) - - constructor() { - super({ movies: [] }) - } -}`, - ` -import { ComponentStore } from '@ngrx/component-store' - -class Ok2 { - readonly addMovie = this.store.updater( - (state, movie): MoviesState => ({ - movies: [...state.movies, movie], - }), - ) - - constructor(private readonly store: ComponentStore) {} -}`, - ` -import { ComponentStore } from '@ngrx/component-store' - -class Ok3 { - readonly addMovie: Observable - - constructor(customStore: ComponentStore) { - this.addMovie = customStore.updater( - (state, movie): MoviesState => ({ - movies: [...state.movies, movie], - }), - ) - } -}`, -]; - -const validInject: () => (string | ValidTestCase)[] = () => [ - ` -import { ComponentStore } from '@ngrx/component-store' -import { inject } from '@angular/core' - -class Ok4 { - private readonly store = inject(ComponentStore); - readonly addMovie = this.store.updater( - (state, movie): MoviesState => ({ - movies: [...state.movies, movie], - }), - ) -}`, - ` -import { ComponentStore } from '@ngrx/component-store' -import { inject } from '@angular/core' - -class Ok5 { - readonly addMovie: Observable - customStore = inject(ComponentStore) - - constructor() { - this.addMovie = this.customStore.updater( - (state, movie): MoviesState => ({ - movies: [...state.movies, movie], - }), - ) - } -}`, -]; - -const invalidConstructor: () => InvalidTestCase[] = () => [ - fromFixture(` -import { ComponentStore } from '@ngrx/component-store' - -class NotOk extends ComponentStore { - readonly addMovie = this.updater((state, movie) => ({ movies: [...state.movies, movie] })) - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [${messageId}] - - constructor() { - super({ movies: [] }) - } -}`), - fromFixture(` -import { ComponentStore } from '@ngrx/component-store' - -class NotOk1 extends ComponentStore { - readonly updateMovie: Observable - readonly addMovie = this.updater((state, movie) => movie ? ({ movies: [...state.movies, movie] }) : ({ movies })) - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [${messageId}] - - constructor(componentStore: ComponentStore) { - super({ movies: [] }) - this.updateMovie = componentStore.updater(() => ({ movies: MOVIES })) - ~~~~~~~~~~~~~~~~~~~~~~~~~~ [${messageId}] - } -}`), - fromFixture(` -import { ComponentStore } from '@ngrx/component-store' - -class NotOk2 { - readonly addMovie = this.store.updater((state, movie) => ({ movies: [...state.movies, movie] })) - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [${messageId}] - - constructor(private readonly store: ComponentStore) {} -}`), - fromFixture(` -import { ComponentStore } from '@ngrx/component-store' - -class NotOk3 { - readonly addMovie: Observable - readonly updateMovie: Observable - - constructor( - customStore: ComponentStore, - private readonly store: ComponentStore - ) { - this.addMovie = customStore.updater((state, movie) => ({ movies: [...state.movies, movie] })) - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [${messageId}] - this.updateMovie = this.store.updater(() => ({ movies: MOVIES })) - ~~~~~~~~~~~~~~~~~~~~~~~~~~ [${messageId}] - } - - ngOnInit() { - const updater = (item: Movie) => item - updater() - } -}`), - fromFixture(` -@Injectable() -export class CompetitorsStore2 extends CompetitorsStore1 { - override updateName = this.updater((state, name: string) => ({ ...state, name })); - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [${messageId}] - - - updateName2 = this.updater(() => ({ name: 'test' })); - ~~~~~~~~~~~~~~~~~~~~~~~~ [${messageId}] -}`), -]; - -const invalidInject: () => InvalidTestCase[] = () => [ - fromFixture(` -import { ComponentStore } from '@ngrx/component-store' -import { inject } from '@angular/core' - -class NotOk4 { - componentStore = inject(ComponentStore) - readonly updateMovie: Observable - - constructor() { - this.updateMovie = this.componentStore.updater(() => ({ movies: MOVIES })) - ~~~~~~~~~~~~~~~~~~~~~~~~~~ [${messageId}] - } -}`), - fromFixture(` -import { ComponentStore } from '@ngrx/component-store' -import { inject } from '@angular/core' - -class NotOk5 { - private readonly store = inject(ComponentStore) - readonly addMovie = this.store.updater((state, movie) => ({ movies: [...state.movies, movie] })) - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [${messageId}] -}`), - fromFixture(` -import { ComponentStore } from '@ngrx/component-store' -import { inject } from '@angular/core' - -class NotOk6 { - customStore = inject(ComponentStore) - private readonly store = inject(ComponentStore) - readonly addMovie: Observable - readonly updateMovie: Observable - - constructor() { - this.addMovie = this.customStore.updater((state, movie) => ({ movies: [...state.movies, movie] })) - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [${messageId}] - this.updateMovie = this.store.updater(() => ({ movies: MOVIES })) - ~~~~~~~~~~~~~~~~~~~~~~~~~~ [${messageId}] - } - - ngOnInit() { - const updater = (item: Movie) => item - updater() - } -}`), -]; - -ruleTester(rule.meta.docs?.requiresTypeChecking).run( - path.parse(__filename).name, - rule, - { - valid: [...validConstructor(), ...validInject()], - invalid: [...invalidConstructor(), ...invalidInject()], - } -); diff --git a/modules/eslint-plugin/spec/rules/store/on-function-explicit-return-type.spec.ts b/modules/eslint-plugin/spec/rules/store/on-function-explicit-return-type.spec.ts deleted file mode 100644 index f261c51602..0000000000 --- a/modules/eslint-plugin/spec/rules/store/on-function-explicit-return-type.spec.ts +++ /dev/null @@ -1,242 +0,0 @@ -import type { ESLintUtils } from '@typescript-eslint/utils'; -import type { - InvalidTestCase, - ValidTestCase, -} from '@typescript-eslint/rule-tester'; -import * as path from 'path'; -import rule, { - onFunctionExplicitReturnType, - onFunctionExplicitReturnTypeSuggest, -} from '../../../src/rules/store/on-function-explicit-return-type'; -import { ruleTester } from '../../utils'; - -type MessageIds = ESLintUtils.InferMessageIdsTypeFromRule; -type Options = ESLintUtils.InferOptionsTypeFromRule; - -const valid: () => (string | ValidTestCase)[] = () => [ - ` -const reducer = createReducer( - initialState, - on( - increment, - (s): State => ({ - ...s, - counter: s.counter + 1, - }), - ), -)`, - ` -const reducer = createReducer( - initialState, - on(increment, incrementFunc), - on(increment, (s): State => incrementFunc(s)), -)`, - ` -const reducer = createReducer( - initialState, - on( - increment, - produce((draft: State, action) => { - draft.counter++; - }), - ), -)`, - // https://github.com/timdeschryver/ngrx-tslint-rules/pull/37 - ` -const reducer = createReducer( - on(increment, (s): State => ({ - ...s, - counter: (s => s.counter + 1)(s), - })), -)`, -]; - -const invalid: () => InvalidTestCase[] = () => [ - { - code: ` -const reducer = createReducer( - initialState, - on(increment, s => s), -)`, - errors: [ - { - column: 17, - endColumn: 23, - line: 4, - messageId: onFunctionExplicitReturnType, - suggestions: [ - { - messageId: onFunctionExplicitReturnTypeSuggest, - output: ` -const reducer = createReducer( - initialState, - on(increment, (s): State => s), -)`, - }, - ], - }, - ], - }, - { - code: ` -const reducer = createReducer( - initialState, - on(increment, s => ({ ...s, counter: s.counter + 1 })), -)`, - errors: [ - { - column: 17, - endColumn: 56, - line: 4, - messageId: onFunctionExplicitReturnType, - suggestions: [ - { - messageId: onFunctionExplicitReturnTypeSuggest, - output: ` -const reducer = createReducer( - initialState, - on(increment, (s): State => ({ ...s, counter: s.counter + 1 })), -)`, - }, - ], - }, - ], - }, - { - code: ` -const reducer = createReducer( - initialState, - on(increase, (s, action) => ({ ...s, counter: s.counter + action.value })), -)`, - errors: [ - { - column: 16, - endColumn: 76, - line: 4, - messageId: onFunctionExplicitReturnType, - suggestions: [ - { - messageId: onFunctionExplicitReturnTypeSuggest, - output: ` -const reducer = createReducer( - initialState, - on(increase, (s, action): State => ({ ...s, counter: s.counter + action.value })), -)`, - }, - ], - }, - ], - }, - - { - code: ` -const reducer = createReducer( - initialState, - on(increase, (s, { value }) => ( { ...s, counter: s.counter + value } ) ), -)`, - errors: [ - { - column: 16, - endColumn: 81, - line: 4, - messageId: onFunctionExplicitReturnType, - suggestions: [ - { - messageId: onFunctionExplicitReturnTypeSuggest, - output: ` -const reducer = createReducer( - initialState, - on(increase, (s, { value }): State => ( { ...s, counter: s.counter + value } ) ), -)`, - }, - ], - }, - ], - }, - - { - code: ` -const reducer = createReducer( - initialState, - on(reset, () => initialState ), -)`, - errors: [ - { - column: 13, - endColumn: 33, - line: 4, - messageId: onFunctionExplicitReturnType, - suggestions: [ - { - messageId: onFunctionExplicitReturnTypeSuggest, - output: ` -const reducer = createReducer( - initialState, - on(reset, (): State => initialState ), -)`, - }, - ], - }, - ], - }, - { - code: ` -const reducer = createReducer( - initialState, - on(reset, s => foo(s)), -)`, - errors: [ - { - column: 13, - endColumn: 24, - line: 4, - messageId: onFunctionExplicitReturnType, - suggestions: [ - { - messageId: onFunctionExplicitReturnTypeSuggest, - output: ` -const reducer = createReducer( - initialState, - on(reset, (s): State => foo(s)), -)`, - }, - ], - }, - ], - }, - // https://github.com/ngrx/platform/issues/4901 - { - code: ` -const reducer = createReducer( - initialState, - on(reset, s => ({ ...s, counter: Number(1) })), -)`, - errors: [ - { - column: 13, - endColumn: 48, - line: 4, - messageId: onFunctionExplicitReturnType, - suggestions: [ - { - messageId: onFunctionExplicitReturnTypeSuggest, - output: ` -const reducer = createReducer( - initialState, - on(reset, (s): State => ({ ...s, counter: Number(1) })), -)`, - }, - ], - }, - ], - }, -]; - -ruleTester(rule.meta.docs?.requiresTypeChecking).run( - path.parse(__filename).name, - rule, - { - valid: valid(), - invalid: invalid(), - } -); diff --git a/modules/eslint-plugin/src/configs/all-type-checked.ts b/modules/eslint-plugin/src/configs/all-type-checked.ts index 9c9355537f..d12db84efe 100644 --- a/modules/eslint-plugin/src/configs/all-type-checked.ts +++ b/modules/eslint-plugin/src/configs/all-type-checked.ts @@ -27,7 +27,6 @@ export default ( '@ngrx/avoid-combining-component-store-selectors': 'error', '@ngrx/avoid-mapping-component-store-selectors': 'error', '@ngrx/require-super-ondestroy': 'error', - '@ngrx/updater-explicit-return-type': 'error', '@ngrx/avoid-cyclic-effects': 'error', '@ngrx/no-dispatch-in-effects': 'error', '@ngrx/no-effects-in-providers': 'error', @@ -50,7 +49,6 @@ export default ( '@ngrx/no-reducer-in-key-names': 'error', '@ngrx/no-store-subscription': 'error', '@ngrx/no-typed-global-store': 'error', - '@ngrx/on-function-explicit-return-type': 'error', '@ngrx/prefer-action-creator-in-dispatch': 'error', '@ngrx/prefer-action-creator': 'error', '@ngrx/prefer-inline-action-props': 'error', diff --git a/modules/eslint-plugin/src/configs/all.ts b/modules/eslint-plugin/src/configs/all.ts index 1c34b57d30..934d1c14cf 100644 --- a/modules/eslint-plugin/src/configs/all.ts +++ b/modules/eslint-plugin/src/configs/all.ts @@ -27,7 +27,6 @@ export default ( '@ngrx/avoid-combining-component-store-selectors': 'error', '@ngrx/avoid-mapping-component-store-selectors': 'error', '@ngrx/require-super-ondestroy': 'error', - '@ngrx/updater-explicit-return-type': 'error', '@ngrx/no-dispatch-in-effects': 'error', '@ngrx/no-effects-in-providers': 'error', '@ngrx/prefer-action-creator-in-of-type': 'error', @@ -46,7 +45,6 @@ export default ( '@ngrx/no-reducer-in-key-names': 'error', '@ngrx/no-store-subscription': 'error', '@ngrx/no-typed-global-store': 'error', - '@ngrx/on-function-explicit-return-type': 'error', '@ngrx/prefer-action-creator-in-dispatch': 'error', '@ngrx/prefer-action-creator': 'error', '@ngrx/prefer-inline-action-props': 'error', diff --git a/modules/eslint-plugin/src/configs/component-store.ts b/modules/eslint-plugin/src/configs/component-store.ts index d233093879..2a9fbe0da7 100644 --- a/modules/eslint-plugin/src/configs/component-store.ts +++ b/modules/eslint-plugin/src/configs/component-store.ts @@ -27,7 +27,6 @@ export default ( '@ngrx/avoid-combining-component-store-selectors': 'error', '@ngrx/avoid-mapping-component-store-selectors': 'error', '@ngrx/require-super-ondestroy': 'error', - '@ngrx/updater-explicit-return-type': 'error', }, }, ]; diff --git a/modules/eslint-plugin/src/configs/store.ts b/modules/eslint-plugin/src/configs/store.ts index 7c0f2c89ce..9004d78df6 100644 --- a/modules/eslint-plugin/src/configs/store.ts +++ b/modules/eslint-plugin/src/configs/store.ts @@ -33,7 +33,6 @@ export default ( '@ngrx/no-reducer-in-key-names': 'error', '@ngrx/no-store-subscription': 'error', '@ngrx/no-typed-global-store': 'error', - '@ngrx/on-function-explicit-return-type': 'error', '@ngrx/prefer-action-creator-in-dispatch': 'error', '@ngrx/prefer-action-creator': 'error', '@ngrx/prefer-inline-action-props': 'error', diff --git a/modules/eslint-plugin/src/rules/component-store/updater-explicit-return-type.ts b/modules/eslint-plugin/src/rules/component-store/updater-explicit-return-type.ts deleted file mode 100644 index f7a8446988..0000000000 --- a/modules/eslint-plugin/src/rules/component-store/updater-explicit-return-type.ts +++ /dev/null @@ -1,48 +0,0 @@ -import type { TSESTree } from '@typescript-eslint/utils'; -import * as path from 'path'; -import { createRule } from '../../rule-creator'; -import { getNgrxComponentStoreNames, namedExpression } from '../../utils'; - -export const messageId = 'updaterExplicitReturnType'; - -type MessageIds = typeof messageId; -type Options = readonly []; - -export default createRule({ - name: path.parse(__filename).name, - meta: { - type: 'problem', - docs: { - description: '`Updater` should have an explicit return type.', - ngrxModule: 'component-store', - }, - schema: [], - messages: { - [messageId]: - '`Updater` should have an explicit return type when using arrow functions: `this.store.updater((state, value): State => {}`.', - }, - }, - defaultOptions: [], - create: (context) => { - const storeNames = getNgrxComponentStoreNames(context); - const withoutTypeAnnotation = `ArrowFunctionExpression:not([returnType.typeAnnotation])`; - const selectors = [ - `ClassDeclaration[superClass.name=/Store/] CallExpression[callee.object.type='ThisExpression'][callee.property.name='updater'] > ${withoutTypeAnnotation}`, - storeNames && - `${namedExpression( - storeNames - )}[callee.property.name='updater'] > ${withoutTypeAnnotation}`, - ] - .filter(Boolean) - .join(','); - - return { - [selectors](node: TSESTree.ArrowFunctionExpression) { - context.report({ - node, - messageId, - }); - }, - }; - }, -}); diff --git a/modules/eslint-plugin/src/rules/index.ts b/modules/eslint-plugin/src/rules/index.ts index adc6f88979..1874e74ae1 100644 --- a/modules/eslint-plugin/src/rules/index.ts +++ b/modules/eslint-plugin/src/rules/index.ts @@ -1,7 +1,6 @@ // component-store import avoidCombiningComponentStoreSelectors from './component-store/avoid-combining-component-store-selectors'; import avoidMappingComponentStoreSelectors from './component-store/avoid-mapping-component-store-selectors'; -import updaterExplicitReturnType from './component-store/updater-explicit-return-type'; import requireSuperOnDestroy from './component-store/require-super-ondestroy'; // effects import avoidCyclicEffects from './effects/avoid-cyclic-effects'; @@ -21,7 +20,6 @@ import noMultipleGlobalStores from './store/no-multiple-global-stores'; import noReducerInKeyNames from './store/no-reducer-in-key-names'; import noStoreSubscription from './store/no-store-subscription'; import noTypedGlobalStore from './store/no-typed-global-store'; -import onFunctionExplicitReturnType from './store/on-function-explicit-return-type'; import preferActionCreator from './store/prefer-action-creator'; import preferActionCreatorInDispatch from './store/prefer-action-creator-in-dispatch'; import preferInlineActionProps from './store/prefer-inline-action-props'; @@ -45,7 +43,6 @@ export const rules = { avoidCombiningComponentStoreSelectors, 'avoid-mapping-component-store-selectors': avoidMappingComponentStoreSelectors, - 'updater-explicit-return-type': updaterExplicitReturnType, 'require-super-ondestroy': requireSuperOnDestroy, //effects 'avoid-cyclic-effects': avoidCyclicEffects, @@ -67,7 +64,6 @@ export const rules = { 'no-reducer-in-key-names': noReducerInKeyNames, 'no-store-subscription': noStoreSubscription, 'no-typed-global-store': noTypedGlobalStore, - 'on-function-explicit-return-type': onFunctionExplicitReturnType, 'prefer-action-creator': preferActionCreator, 'prefer-action-creator-in-dispatch': preferActionCreatorInDispatch, 'prefer-inline-action-props': preferInlineActionProps, diff --git a/modules/eslint-plugin/src/rules/store/on-function-explicit-return-type.ts b/modules/eslint-plugin/src/rules/store/on-function-explicit-return-type.ts deleted file mode 100644 index 9eca1e89da..0000000000 --- a/modules/eslint-plugin/src/rules/store/on-function-explicit-return-type.ts +++ /dev/null @@ -1,79 +0,0 @@ -import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; -import { ASTUtils } from '@typescript-eslint/utils'; -import * as path from 'path'; -import { createRule } from '../../rule-creator'; -import { getLast, onFunctionWithoutType } from '../../utils'; - -export const onFunctionExplicitReturnType = 'onFunctionExplicitReturnType'; -export const onFunctionExplicitReturnTypeSuggest = - 'onFunctionExplicitReturnTypeSuggest'; - -type MessageIds = - | typeof onFunctionExplicitReturnType - | typeof onFunctionExplicitReturnTypeSuggest; -type Options = readonly []; - -export default createRule({ - name: path.parse(__filename).name, - meta: { - type: 'suggestion', - hasSuggestions: true, - docs: { - description: '`On` function should have an explicit return type.', - ngrxModule: 'store', - }, - schema: [], - messages: { - [onFunctionExplicitReturnType]: - '`On` functions should have an explicit return type when using arrow functions: `on(action, (state): State => {}`.', - [onFunctionExplicitReturnTypeSuggest]: - 'Add the explicit return type `State` (if the interface/type is named differently you need to manually correct the return type).', - }, - }, - defaultOptions: [], - create: (context) => { - return { - [onFunctionWithoutType](node: TSESTree.ArrowFunctionExpression) { - context.report({ - node, - messageId: onFunctionExplicitReturnType, - suggest: [ - { - messageId: onFunctionExplicitReturnTypeSuggest, - fix: (fixer) => getFixes(node, context.sourceCode, fixer), - }, - ], - }); - }, - }; - }, -}); - -function getFixes( - node: TSESTree.ArrowFunctionExpression, - sourceCode: Readonly, - fixer: TSESLint.RuleFixer -) { - const { params } = node; - - if (params.length === 0) { - const [, closingParen] = sourceCode.getTokens(node); - return fixer.insertTextAfter(closingParen, ': State'); - } - - const [firstParam] = params; - const lastParam = getLast(params); - const previousToken = sourceCode.getTokenBefore(firstParam); - const isParenthesized = - previousToken && ASTUtils.isOpeningParenToken(previousToken); - - if (isParenthesized) { - const nextToken = sourceCode.getTokenAfter(lastParam); - return fixer.insertTextAfter(nextToken ?? lastParam, ': State'); - } - - return [ - fixer.insertTextBefore(firstParam, '('), - fixer.insertTextAfter(lastParam, '): State'), - ] as const; -} diff --git a/modules/store/spec/types/reducer_creator.spec.ts b/modules/store/spec/types/reducer_creator.spec.ts index 65916355c5..3984d7137d 100644 --- a/modules/store/spec/types/reducer_creator.spec.ts +++ b/modules/store/spec/types/reducer_creator.spec.ts @@ -99,9 +99,7 @@ describe('createReducer()', () => { `).toInfer( 'onFn', ` - ReducerTypes<{ - name: string; - }, [ActionCreator<"FOO", (props: { + ReducerTypes { foo: string; @@ -118,11 +116,390 @@ describe('createReducer()', () => { `).toInfer( 'onFn', ` - ReducerTypes<{ - name: string; - }, [ActionCreator<"FOO", () => Action<"FOO">>]> + ReducerTypes Action<"FOO">>]> ` ); }); + + describe('valid patterns', () => { + it('should allow spread with property override inside createReducer', () => { + expectSnippet(` + interface State { name: string; count: number }; + const initialState: State = { name: 'test', count: 0 }; + const setName = createAction('setName', props<{ name: string }>()); + + const reducer = createReducer( + initialState, + on(setName, (state, { name }) => ({ ...state, name })), + ); + `).toSucceed(); + }); + + it('should allow returning initialState inside createReducer', () => { + expectSnippet(` + interface State { name: string; count: number }; + const initialState: State = { name: 'test', count: 0 }; + const reset = createAction('reset'); + + const reducer = createReducer( + initialState, + on(reset, () => initialState), + ); + `).toSucceed(); + }); + + it('should allow returning state directly inside createReducer', () => { + expectSnippet(` + interface State { name: string; count: number }; + const initialState: State = { name: 'test', count: 0 }; + const noop = createAction('noop'); + + const reducer = createReducer( + initialState, + on(noop, (state) => state), + ); + `).toSucceed(); + }); + + it('should allow standalone on() with explicit state type', () => { + expectSnippet(` + interface State { name: string; count: number }; + const setName = createAction('setName', props<{ name: string }>()); + + const onFn = on(setName, (state: State, { name }) => ({ ...state, name })); + `).toSucceed(); + }); + + it('should allow explicit return of all properties inside createReducer', () => { + expectSnippet(` + interface State { name: string; count: number }; + const initialState: State = { name: 'test', count: 0 }; + const setName = createAction('setName', props<{ name: string }>()); + + const reducer = createReducer( + initialState, + on(setName, (state, { name }) => ({ name, count: state.count })), + ); + `).toSucceed(); + }); + + it('should allow on() with multiple action creators', () => { + expectSnippet(` + interface State { name: string; count: number }; + const initialState: State = { name: 'test', count: 0 }; + const action1 = createAction('action1'); + const action2 = createAction('action2'); + + const reducer = createReducer( + initialState, + on(action1, action2, (state) => ({ ...state, count: state.count + 1 })), + ); + `).toSucceed(); + }); + }); + + describe('catches excess properties', () => { + it('should catch excess properties in on() callback inside createReducer', () => { + expectSnippet(` + interface State { name: string; count: number }; + const initialState: State = { name: 'test', count: 0 }; + const setName = createAction('setName', props<{ name: string }>()); + + const reducer = createReducer( + initialState, + on(setName, (state, { name }) => ({ ...state, name, extra: true })), + ); + `).toFail(/Remove excess properties/); + }); + + it('should catch excess properties in standalone on() with explicit state type', () => { + expectSnippet(` + interface State { name: string; count: number }; + const setName = createAction('setName', props<{ name: string }>()); + + const onFn = on(setName, (state: State, { name }) => ({ ...state, name, extra: true })); + `).toFail(/Remove excess properties/); + }); + + it('should catch excess properties when returning explicit object', () => { + expectSnippet(` + interface State { name: string; count: number }; + const initialState: State = { name: 'test', count: 0 }; + const setName = createAction('setName', props<{ name: string }>()); + + const reducer = createReducer( + initialState, + on(setName, (state, { name }) => ({ name, count: state.count, extra: 'bad' })), + ); + `).toFail(/Remove excess properties/); + }); + + it('should catch excess properties when explicit return type annotation is used', () => { + expectSnippet(` + interface State { name: string; count: number }; + const initialState: State = { name: 'test', count: 0 }; + const setName = createAction('setName', props<{ name: string }>()); + + const reducer = createReducer( + initialState, + on(setName, (state, { name }): State => ({ ...state, name, extra: true })), + ); + `).toFail(/does not exist in type/); + }); + + it('should catch excess properties from void on() callback', () => { + expectSnippet(` + interface State { name: string; count: number }; + const initialState: State = { name: 'test', count: 0 }; + const noop = createAction('noop'); + + const reducer = createReducer( + initialState, + on(noop, (state) => ({ ...state, extra: true })), + ); + `).toFail(/Remove excess properties/); + }); + }); + + describe('edge cases', () => { + it('should work with state containing optional properties', () => { + expectSnippet(` + interface State { req: string; opt?: number }; + const initialState: State = { req: 'a' }; + const update = createAction('update'); + + const reducer = createReducer( + initialState, + on(update, (state) => ({ req: 'b' })), + ); + `).toSucceed(); + }); + + it('should work with state containing index signature', () => { + expectSnippet(` + const initialState: { [key: string]: number } = {}; + const add = createAction('add', props<{ key: string; value: number }>()); + + const reducer = createReducer( + initialState, + on(add, (state, { key, value }) => ({ ...state, [key]: value })), + ); + `).toSucceed(); + }); + + it('should catch excess properties with index signature state', () => { + expectSnippet(` + interface State { name: string }; + const initialState: State = { name: 'test' }; + const update = createAction('update'); + + const reducer = createReducer( + initialState, + on(update, (state) => ({ ...state, extra: true })), + ); + `).toFail(/Remove excess properties/); + }); + }); + + describe('already enforced type checks', () => { + it('should catch missing required properties inside createReducer', () => { + expectSnippet(` + interface State { name: string; count: number }; + const initialState: State = { name: 'test', count: 0 }; + const setName = createAction('setName', props<{ name: string }>()); + + const reducer = createReducer( + initialState, + on(setName, (state, { name }) => ({ name })), + ); + `).toFail(/is missing in type/); + }); + + it('should catch wrong property types inside createReducer', () => { + expectSnippet(` + interface State { name: string; count: number }; + const initialState: State = { name: 'test', count: 0 }; + const setName = createAction('setName', props<{ name: string }>()); + + const reducer = createReducer( + initialState, + on(setName, (state, { name }) => ({ ...state, name: 123 })), + ); + `).toFail(/not assignable to type/); + }); + }); + + describe('union, nullable and record state', () => { + it('should allow switching to a discriminated union member with its own keys', () => { + expectSnippet(` + type State = { status: 'idle' } | { status: 'loaded'; data: string[] }; + const load = createAction('load'); + const loaded = createAction('loaded', props<{ data: string[] }>()); + + const reducer = createReducer( + { status: 'idle' }, + on(load, () => ({ status: 'idle' as const })), + on(loaded, (_, { data }) => ({ status: 'loaded' as const, data })), + ); + `).toSucceed(); + }); + + it('should catch keys that exist on no union member', () => { + expectSnippet(` + type State = { status: 'idle' } | { status: 'loaded'; data: string[] }; + const load = createAction('load'); + + const reducer = createReducer( + { status: 'idle' }, + on(load, () => ({ status: 'idle' as const, bogus: true })), + ); + `).toFail(/Remove excess properties/); + }); + + it('should allow nullable state', () => { + expectSnippet(` + interface User { id: string; name: string }; + const login = createAction('login', props<{ user: User }>()); + const logout = createAction('logout'); + + const reducer = createReducer( + null, + on(login, (_, { user }) => user), + on(logout, () => null), + ); + `).toSucceed(); + }); + + it('should catch excess properties with nullable state', () => { + expectSnippet(` + interface User { id: string; name: string }; + const login = createAction('login'); + + const reducer = createReducer( + null, + on(login, () => ({ id: '1', name: 'a', extra: true })), + ); + `).toFail(/Remove excess properties/); + }); + + it('should allow Record state with computed keys', () => { + expectSnippet(` + const set = createAction('set', props<{ key: string; value: number }>()); + + const reducer = createReducer>( + {}, + on(set, (state, { key, value }) => ({ ...state, [key]: value })), + ); + `).toSucceed(); + }); + + it('should catch excess properties returned from one branch of a conditional', () => { + expectSnippet(` + interface State { name: string }; + const initialState: State = { name: 'test' }; + const update = createAction('update', props<{ flag: boolean }>()); + + const reducer = createReducer( + initialState, + on(update, (state, { flag }) => (flag ? { ...state, extra: true } : state)), + ); + `).toFail(/Remove excess properties/); + }); + + it('should name the excess property in the error', () => { + expectSnippet(` + interface State { name: string }; + const initialState: State = { name: 'test' }; + const update = createAction('update'); + + const reducer = createReducer( + initialState, + on(update, (state) => ({ ...state, extra: true })), + ); + `).toFail(/Excess property: extra/); + }); + }); + + describe('generic state', () => { + it('should allow overriding a key guaranteed by the constraint', () => { + expectSnippet(` + const load = createAction('load'); + + export function createLoadingReducer(initialState: TState) { + return createReducer( + initialState, + on(load, (state) => ({ ...state, loading: true })), + ); + } + `).toSucceed(); + }); + + it('should allow returning state with an unconstrained state type', () => { + expectSnippet(` + const noop = createAction('noop'); + + export function createNoopReducer(initialState: TState) { + return createReducer( + initialState, + on(noop, (state) => state), + ); + } + `).toSucceed(); + }); + + it('should allow a generic item type inside a concrete state shape', () => { + expectSnippet(` + interface ListState { items: T[]; loading: boolean }; + const load = createAction('load'); + + export function createListReducer(initialState: ListState) { + return createReducer( + initialState, + on(load, (state) => ({ ...state, loading: true })), + ); + } + `).toSucceed(); + }); + + it('should allow overriding a base key of an intersected generic state', () => { + expectSnippet(` + interface BaseState { loading: boolean }; + const load = createAction('load'); + + export function createExtendableReducer(initialState: BaseState & TExtra) { + return createReducer( + initialState, + on(load, (state) => ({ ...state, loading: true })), + ); + } + `).toSucceed(); + }); + + it('should allow asserting the return value as the generic state type', () => { + expectSnippet(` + const load = createAction('load'); + + export function createLoadingReducer(initialState: TState) { + return createReducer( + initialState, + on(load, (state) => ({ ...state, loading: true }) as TState), + ); + } + `).toSucceed(); + }); + + it('should catch keys not guaranteed by the constraint', () => { + expectSnippet(` + const load = createAction('load'); + + export function createLoadingReducer(initialState: TState) { + return createReducer( + initialState, + on(load, (state) => ({ ...state, extra: true })), + ); + } + `).toFail(/Remove excess properties/); + }); + }); }); }, 8_000); diff --git a/modules/store/src/models.ts b/modules/store/src/models.ts index e66f05cd5b..9f6ad8247e 100644 --- a/modules/store/src/models.ts +++ b/modules/store/src/models.ts @@ -94,13 +94,7 @@ export type Creator< > = FunctionWithParametersType; export type Primitive = - | string - | number - | bigint - | boolean - | symbol - | null - | undefined; + string | number | bigint | boolean | symbol | null | undefined; export type NotAllowedCheck = T extends any[] ? ArraysAreNotAllowed @@ -175,4 +169,32 @@ export interface SelectSignalOptions { equal?: ValueEqualityFn; } +export const excessPropertiesAreNotAllowedMsg = + 'callback return type must exactly match the state type. Remove excess properties.'; +export type ExcessPropertiesAreNotAllowed = + typeof excessPropertiesAreNotAllowedMsg; + +type KeysOfUnion = T extends unknown ? keyof T : never; +// Keys a returned object may carry: any key of any union member, plus numeric keys for string index signatures. +export type AllowedStateKeys = + keyof S | KeysOfUnion | (string extends keyof S ? number : never); +// `K` is inferred from the returned object's keys and constrained to AllowedStateKeys. Constraint checks use +// assignability, which respects generic constraints (e.g. `T extends { loading: boolean }`). When a key is not +// allowed, inference falls back to the constraint and the leftover keys are typed as the error message. +export type ExactStateReturn = [R] extends [ + null | undefined, +] + ? unknown + : // Fast path: no key outside the state's keys (also resolves for identical generic types, e.g. `return state`) + [Exclude, AllowedStateKeys>] extends [never] + ? unknown + : { [P in K & PropertyKey]?: unknown } & { + [ + P in Exclude< + KeysOfUnion, + K | AllowedStateKeys + > as `${Msg} Excess property: ${P & string}` + ]: never; + }; + export type Prettify = { [K in keyof T]: T[K] } & {}; diff --git a/modules/store/src/reducer_creator.ts b/modules/store/src/reducer_creator.ts index a0f4974719..61f0dee7e9 100644 --- a/modules/store/src/reducer_creator.ts +++ b/modules/store/src/reducer_creator.ts @@ -1,4 +1,12 @@ -import { ActionCreator, ActionReducer, ActionType, Action } from './models'; +import { + ActionCreator, + ActionReducer, + ActionType, + Action, + ExcessPropertiesAreNotAllowed, + AllowedStateKeys, + ExactStateReturn, +} from './models'; // Goes over the array of ActionCreators, pulls the action type out of each one // and returns the array of these action types. @@ -62,14 +70,26 @@ export function on< // is created outside of `createReducer` and state type is either explicitly set OR inferred by return type. // For example: `const onFn = on(action, (state: State, {prop}) => ({ ...state, name: prop }));` InferredState = State, + // Compute the effective state type: either State (when known from createReducer) or InferredState (when standalone) + EffectiveState = unknown extends State ? InferredState : State, + // Captures the actual return type to enforce exact state shape — excess properties produce a descriptive type error + R extends EffectiveState = EffectiveState, + // Keys of the returned object, vetted against the state's keys via the constraint + K extends AllowedStateKeys = + never, >( ...args: [ ...creators: Creators, - reducer: OnReducer< - State extends infer S ? S : never, - Creators, - InferredState - >, + reducer: ( + state: unknown extends State ? InferredState : State, + action: ActionType + ) => R & + ExactStateReturn< + R, + unknown extends State ? InferredState : State, + K, + ExcessPropertiesAreNotAllowed + >, ] ): ReducerTypes { const reducer = args.pop() as unknown as OnReducer< diff --git a/projects/www/src/app/pages/guide/component-store/write.md b/projects/www/src/app/pages/guide/component-store/write.md index 2e54125f58..4d07dffedf 100644 --- a/projects/www/src/app/pages/guide/component-store/write.md +++ b/projects/www/src/app/pages/guide/component-store/write.md @@ -46,6 +46,34 @@ export class MoviesStore extends ComponentStore { +The `updater` method enforces that callbacks return an object matching the state type exactly. Returning an object with extra properties that don't exist on the state type produces a TypeScript compilation error: + + + +```ts +@Injectable() +export class MoviesStore extends ComponentStore { + constructor() { + super({ movies: [] }); + } + + readonly addMovie = this.updater((state, movie: Movie) => ({ + movies: [...state.movies, movie], + // TS error: updater callback return type must exactly match the + // state type. Remove excess properties. Excess property: extra + extra: true, + })); +} +``` + + + + + +**Note:** When `ComponentStore` is extended with a generic state type parameter (e.g., `class MyStore extends ComponentStore`), properties guaranteed by the constraint can be overridden (`{ ...state, loading: true }`), while properties the constraint doesn't guarantee are reported as excess. + + + Updater then can be called with the values imperatively or could take an Observable. diff --git a/projects/www/src/app/pages/guide/eslint-plugin/index.md b/projects/www/src/app/pages/guide/eslint-plugin/index.md index cc7c8dde25..d027eaac81 100644 --- a/projects/www/src/app/pages/guide/eslint-plugin/index.md +++ b/projects/www/src/app/pages/guide/eslint-plugin/index.md @@ -80,7 +80,6 @@ module.exports = tseslint.config({ | [@ngrx/avoid-combining-component-store-selectors](/guide/eslint-plugin/rules/avoid-combining-component-store-selectors) | Prefer combining selectors at the selector level. | suggestion | No | No | No | No | | [@ngrx/avoid-mapping-component-store-selectors](/guide/eslint-plugin/rules/avoid-mapping-component-store-selectors) | Avoid mapping logic outside the selector level. | problem | No | No | No | No | | [@ngrx/require-super-ondestroy](/guide/eslint-plugin/rules/require-super-ondestroy) | Overriden ngOnDestroy method in component stores require a call to super.ngOnDestroy(). | problem | No | No | No | No | -| [@ngrx/updater-explicit-return-type](/guide/eslint-plugin/rules/updater-explicit-return-type) | `Updater` should have an explicit return type. | problem | No | No | No | No | ### effects @@ -123,7 +122,6 @@ module.exports = tseslint.config({ | [@ngrx/no-reducer-in-key-names](/guide/eslint-plugin/rules/no-reducer-in-key-names) | Avoid the word "reducer" in the key names. | suggestion | No | Yes | No | No | | [@ngrx/no-store-subscription](/guide/eslint-plugin/rules/no-store-subscription) | Using the `async` pipe is preferred over `store` subscription. | suggestion | No | No | No | No | | [@ngrx/no-typed-global-store](/guide/eslint-plugin/rules/no-typed-global-store) | The global store should not be typed. | suggestion | No | Yes | No | No | -| [@ngrx/on-function-explicit-return-type](/guide/eslint-plugin/rules/on-function-explicit-return-type) | `On` function should have an explicit return type. | suggestion | No | Yes | No | No | | [@ngrx/prefer-action-creator-in-dispatch](/guide/eslint-plugin/rules/prefer-action-creator-in-dispatch) | Using `action creator` in `dispatch` is preferred over `object` or old `Action`. | suggestion | No | No | No | No | | [@ngrx/prefer-action-creator](/guide/eslint-plugin/rules/prefer-action-creator) | Using `action creator` is preferred over `Action class`. | suggestion | No | No | No | No | | [@ngrx/prefer-inline-action-props](/guide/eslint-plugin/rules/prefer-inline-action-props) | Prefer using inline types instead of interfaces, types or classes. | suggestion | No | Yes | No | No | diff --git a/projects/www/src/app/pages/guide/eslint-plugin/rules/on-function-explicit-return-type.md b/projects/www/src/app/pages/guide/eslint-plugin/rules/on-function-explicit-return-type.md deleted file mode 100644 index 6d0e52c30f..0000000000 --- a/projects/www/src/app/pages/guide/eslint-plugin/rules/on-function-explicit-return-type.md +++ /dev/null @@ -1,61 +0,0 @@ -# on-function-explicit-return-type - -`On` function should have an explicit return type. - -- **Type**: suggestion -- **Fixable**: No -- **Suggestion**: Yes -- **Requires type checking**: No -- **Configurable**: No - - - - -## Rule Details - -When we use the `on` function to create reducers, we usually copy the state into a new object, and then add the properties that are being modified after that certain action. This may result in unexpected typing problems, we can add new properties into the state that did not exist previously. TypeScript doesn't see this as a problem and might change the state's interface. The solution is to provide an explicit return type to the `on` function callback. - -Examples of **incorrect** code for this rule: - - - -```ts -export interface AppState { - username: string; -} - -const reducer = createReducer( - { username: '' }, - on(setUsername, (state, action) => ({ - ...state, - username: action.payload, - newProperty: 1, // we added a property that does not exist on `AppState`, and TS won't catch this problem - })) -); -``` - - - -Examples of **correct** code for this rule: - - - -```ts -export interface AppState { - username: string; -} - -const reducer = createReducer( - { username: '' }, - on( - setUsername, - (state, action): AppState => ({ - ...state, - username: action.payload, - // adding new properties that do not exist on `AppState` is impossible, as the function return type is explicitly stated - }) - ) -); -``` - - diff --git a/projects/www/src/app/pages/guide/eslint-plugin/rules/updater-explicit-return-type.md b/projects/www/src/app/pages/guide/eslint-plugin/rules/updater-explicit-return-type.md deleted file mode 100644 index 3bc4ee1657..0000000000 --- a/projects/www/src/app/pages/guide/eslint-plugin/rules/updater-explicit-return-type.md +++ /dev/null @@ -1,58 +0,0 @@ -# updater-explicit-return-type - -`Updater` should have an explicit return type. - -- **Type**: problem -- **Fixable**: No -- **Suggestion**: No -- **Requires type checking**: No -- **Configurable**: No - - - - -## Rule Details - -To enforce that the `updater` method from `@ngrx/component-store` returns the expected state interface, we must explicitly add the return type. - -Examples of **incorrect** code for this rule: - - - -```ts -interface MoviesState { - movies: Movie[]; -} - -class MoviesStore extends ComponentStore { - readonly addMovie = this.updater((state, movie: Movie) => ({ - movies: [...state.movies, movie], - // ⚠ this doesn't throw, but is caught by the linter - extra: 'property', - })); -} -``` - - - -Examples of **correct** code for this rule: - - - -```ts -interface MoviesState { - movies: Movie[]; -} - -class MoviesStore extends ComponentStore { - readonly addMovie = this.updater( - (state, movie: Movie): MoviesState => ({ - movies: [...state.movies, movie], - // ⚠ this does throw - extra: 'property', - }) - ); -} -``` - - diff --git a/projects/www/src/app/pages/guide/store/reducers.md b/projects/www/src/app/pages/guide/store/reducers.md index 42772b49ca..73b2fde1b8 100644 --- a/projects/www/src/app/pages/guide/store/reducers.md +++ b/projects/www/src/app/pages/guide/store/reducers.md @@ -120,6 +120,33 @@ In the example above, the reducer is handling 4 actions: `[Scoreboard Page] Home When an action is dispatched, _all registered reducers_ receive the action. Whether they handle the action is determined by the `on` functions that associate one or more actions with a given state change. +### Exact return type enforcement + +The `on` function enforces that callbacks return an object matching the state type exactly. Returning an object with extra properties that don't exist on the state type produces a TypeScript compilation error: + + + +```ts +export const scoreboardReducer = createReducer( + initialState, + on(ScoreboardPageActions.homeScore, (state) => ({ + ...state, + home: state.home + 1, + // TS error: callback return type must exactly match the state + // type. Remove excess properties. Excess property: extra + extra: true, + })) +); +``` + + + + + +**Note:** When `on` is used inside a generic reducer factory (e.g., `function createLoadingReducer()`), properties guaranteed by the constraint can be overridden (`{ ...state, loading: true }`), while properties the constraint doesn't guarantee are reported as excess. + + + **Note:** You can also write reducers using switch statements, which was the previously defined way before reducer creators were introduced in NgRx. If you are looking for examples of reducers using switch statements, visit the documentation for [versions 7.x and prior](https://v7.ngrx.io/guide/store/reducers).