Skip to content

Commit 23aa053

Browse files
david-shortmandavid-shortman
committed
feat(store): enforce exact return types in on() callbacks
The on() function now enforces exact return types at the type level, preventing excess properties in callback return values without requiring explicit return type annotations. This eliminates the need for the on-function-explicit-return-type ESLint rule, which is now deprecated. BREAKING CHANGES: Reducer callbacks passed to on() that return objects with properties not present in the state type will now produce TypeScript compilation errors. When on() is used inside a generic reducer factory where the state type is an unresolved generic parameter (e.g. createGenericReducer<TState>), callbacks that spread state and override known properties may produce a false type error. Return state directly or use a type assertion (as TState) in those cases. BEFORE: ```ts interface State { name: string; count: number } const initialState: State = { name: '', count: 0 }; const reducer = createReducer( initialState, on(setName, (state, { name }) => ({ ...state, name, extraProp: true, // no error })), ); ``` AFTER: ```ts // Now produces a TypeScript error: // Type '{ extraProp: boolean; ... }' is not assignable to type // '"callback return type must exactly match the state type. // Remove excess properties."' const reducer = createReducer( initialState, on(setName, (state, { name }) => ({ ...state, name, extraProp: true, // TS error })), ); ``` Closes #4280
1 parent d57fa8c commit 23aa053

File tree

6 files changed

+268
-13
lines changed

6 files changed

+268
-13
lines changed

‎modules/eslint-plugin/src/rules/store/on-function-explicit-return-type.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,11 @@ export default createRule<Options, MessageIds>({
1717
name: path.parse(__filename).name,
1818
meta: {
1919
type: 'suggestion',
20+
deprecated: true,
2021
hasSuggestions: true,
2122
docs: {
22-
description: '`On` function should have an explicit return type.',
23+
description:
24+
'`On` function should have an explicit return type. Deprecated: The `on` function now enforces exact return types at the type level.',
2325
ngrxModule: 'store',
2426
},
2527
schema: [],

‎modules/store/spec/types/reducer_creator.spec.ts‎

Lines changed: 210 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -99,9 +99,7 @@ describe('createReducer()', () => {
9999
`).toInfer(
100100
'onFn',
101101
`
102-
ReducerTypes<{
103-
name: string;
104-
}, [ActionCreator<"FOO", (props: {
102+
ReducerTypes<State, [ActionCreator<"FOO", (props: {
105103
foo: string;
106104
}) => {
107105
foo: string;
@@ -118,11 +116,217 @@ describe('createReducer()', () => {
118116
`).toInfer(
119117
'onFn',
120118
`
121-
ReducerTypes<{
122-
name: string;
123-
}, [ActionCreator<"FOO", () => Action<"FOO">>]>
119+
ReducerTypes<State, [ActionCreator<"FOO", () => Action<"FOO">>]>
124120
`
125121
);
126122
});
123+
124+
describe('valid patterns', () => {
125+
it('should allow spread with property override inside createReducer', () => {
126+
expectSnippet(`
127+
interface State { name: string; count: number };
128+
const initialState: State = { name: 'test', count: 0 };
129+
const setName = createAction('setName', props<{ name: string }>());
130+
131+
const reducer = createReducer(
132+
initialState,
133+
on(setName, (state, { name }) => ({ ...state, name })),
134+
);
135+
`).toSucceed();
136+
});
137+
138+
it('should allow returning initialState inside createReducer', () => {
139+
expectSnippet(`
140+
interface State { name: string; count: number };
141+
const initialState: State = { name: 'test', count: 0 };
142+
const reset = createAction('reset');
143+
144+
const reducer = createReducer(
145+
initialState,
146+
on(reset, () => initialState),
147+
);
148+
`).toSucceed();
149+
});
150+
151+
it('should allow returning state directly inside createReducer', () => {
152+
expectSnippet(`
153+
interface State { name: string; count: number };
154+
const initialState: State = { name: 'test', count: 0 };
155+
const noop = createAction('noop');
156+
157+
const reducer = createReducer(
158+
initialState,
159+
on(noop, (state) => state),
160+
);
161+
`).toSucceed();
162+
});
163+
164+
it('should allow standalone on() with explicit state type', () => {
165+
expectSnippet(`
166+
interface State { name: string; count: number };
167+
const setName = createAction('setName', props<{ name: string }>());
168+
169+
const onFn = on(setName, (state: State, { name }) => ({ ...state, name }));
170+
`).toSucceed();
171+
});
172+
173+
it('should allow explicit return of all properties inside createReducer', () => {
174+
expectSnippet(`
175+
interface State { name: string; count: number };
176+
const initialState: State = { name: 'test', count: 0 };
177+
const setName = createAction('setName', props<{ name: string }>());
178+
179+
const reducer = createReducer(
180+
initialState,
181+
on(setName, (state, { name }) => ({ name, count: state.count })),
182+
);
183+
`).toSucceed();
184+
});
185+
186+
it('should allow on() with multiple action creators', () => {
187+
expectSnippet(`
188+
interface State { name: string; count: number };
189+
const initialState: State = { name: 'test', count: 0 };
190+
const action1 = createAction('action1');
191+
const action2 = createAction('action2');
192+
193+
const reducer = createReducer(
194+
initialState,
195+
on(action1, action2, (state) => ({ ...state, count: state.count + 1 })),
196+
);
197+
`).toSucceed();
198+
});
199+
});
200+
201+
describe('catches excess properties', () => {
202+
it('should catch excess properties in on() callback inside createReducer', () => {
203+
expectSnippet(`
204+
interface State { name: string; count: number };
205+
const initialState: State = { name: 'test', count: 0 };
206+
const setName = createAction('setName', props<{ name: string }>());
207+
208+
const reducer = createReducer(
209+
initialState,
210+
on(setName, (state, { name }) => ({ ...state, name, extra: true })),
211+
);
212+
`).toFail(/Remove excess properties/);
213+
});
214+
215+
it('should catch excess properties in standalone on() with explicit state type', () => {
216+
expectSnippet(`
217+
interface State { name: string; count: number };
218+
const setName = createAction('setName', props<{ name: string }>());
219+
220+
const onFn = on(setName, (state: State, { name }) => ({ ...state, name, extra: true }));
221+
`).toFail(/Remove excess properties/);
222+
});
223+
224+
it('should catch excess properties when returning explicit object', () => {
225+
expectSnippet(`
226+
interface State { name: string; count: number };
227+
const initialState: State = { name: 'test', count: 0 };
228+
const setName = createAction('setName', props<{ name: string }>());
229+
230+
const reducer = createReducer(
231+
initialState,
232+
on(setName, (state, { name }) => ({ name, count: state.count, extra: 'bad' })),
233+
);
234+
`).toFail(/Remove excess properties/);
235+
});
236+
237+
it('should catch excess properties when explicit return type annotation is used', () => {
238+
expectSnippet(`
239+
interface State { name: string; count: number };
240+
const initialState: State = { name: 'test', count: 0 };
241+
const setName = createAction('setName', props<{ name: string }>());
242+
243+
const reducer = createReducer(
244+
initialState,
245+
on(setName, (state, { name }): State => ({ ...state, name, extra: true })),
246+
);
247+
`).toFail(/does not exist in type/);
248+
});
249+
250+
it('should catch excess properties from void on() callback', () => {
251+
expectSnippet(`
252+
interface State { name: string; count: number };
253+
const initialState: State = { name: 'test', count: 0 };
254+
const noop = createAction('noop');
255+
256+
const reducer = createReducer(
257+
initialState,
258+
on(noop, (state) => ({ ...state, extra: true })),
259+
);
260+
`).toFail(/Remove excess properties/);
261+
});
262+
});
263+
264+
describe('edge cases', () => {
265+
it('should work with state containing optional properties', () => {
266+
expectSnippet(`
267+
interface State { req: string; opt?: number };
268+
const initialState: State = { req: 'a' };
269+
const update = createAction('update');
270+
271+
const reducer = createReducer(
272+
initialState,
273+
on(update, (state) => ({ req: 'b' })),
274+
);
275+
`).toSucceed();
276+
});
277+
278+
it('should work with state containing index signature', () => {
279+
expectSnippet(`
280+
const initialState: { [key: string]: number } = {};
281+
const add = createAction('add', props<{ key: string; value: number }>());
282+
283+
const reducer = createReducer(
284+
initialState,
285+
on(add, (state, { key, value }) => ({ ...state, [key]: value })),
286+
);
287+
`).toSucceed();
288+
});
289+
290+
it('should catch excess properties with index signature state', () => {
291+
expectSnippet(`
292+
interface State { name: string };
293+
const initialState: State = { name: 'test' };
294+
const update = createAction('update');
295+
296+
const reducer = createReducer(
297+
initialState,
298+
on(update, (state) => ({ ...state, extra: true })),
299+
);
300+
`).toFail(/Remove excess properties/);
301+
});
302+
});
303+
304+
describe('already enforced type checks', () => {
305+
it('should catch missing required properties inside createReducer', () => {
306+
expectSnippet(`
307+
interface State { name: string; count: number };
308+
const initialState: State = { name: 'test', count: 0 };
309+
const setName = createAction('setName', props<{ name: string }>());
310+
311+
const reducer = createReducer(
312+
initialState,
313+
on(setName, (state, { name }) => ({ name })),
314+
);
315+
`).toFail(/is missing in type/);
316+
});
317+
318+
it('should catch wrong property types inside createReducer', () => {
319+
expectSnippet(`
320+
interface State { name: string; count: number };
321+
const initialState: State = { name: 'test', count: 0 };
322+
const setName = createAction('setName', props<{ name: string }>());
323+
324+
const reducer = createReducer(
325+
initialState,
326+
on(setName, (state, { name }) => ({ ...state, name: 123 })),
327+
);
328+
`).toFail(/not assignable to type/);
329+
});
330+
});
127331
});
128332
}, 8_000);

‎modules/store/src/models.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,4 +175,9 @@ export interface SelectSignalOptions<T> {
175175
equal?: ValueEqualityFn<T>;
176176
}
177177

178+
export const excessPropertiesAreNotAllowedMsg =
179+
'callback return type must exactly match the state type. Remove excess properties.';
180+
export type ExcessPropertiesAreNotAllowed =
181+
typeof excessPropertiesAreNotAllowedMsg;
182+
178183
export type Prettify<T> = { [K in keyof T]: T[K] } & {};

‎modules/store/src/reducer_creator.ts‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,10 @@
1-
import { ActionCreator, ActionReducer, ActionType, Action } from './models';
1+
import {
2+
ActionCreator,
3+
ActionReducer,
4+
ActionType,
5+
Action,
6+
ExcessPropertiesAreNotAllowed,
7+
} from './models';
28

39
// Goes over the array of ActionCreators, pulls the action type out of each one
410
// and returns the array of these action types.
@@ -62,14 +68,20 @@ export function on<
6268
// is created outside of `createReducer` and state type is either explicitly set OR inferred by return type.
6369
// For example: `const onFn = on(action, (state: State, {prop}) => ({ ...state, name: prop }));`
6470
InferredState = State,
71+
// Compute the effective state type: either State (when known from createReducer) or InferredState (when standalone)
72+
EffectiveState = unknown extends State ? InferredState : State,
73+
// Captures the actual return type to enforce exact state shape — excess properties produce a descriptive type error
74+
R extends EffectiveState = EffectiveState,
6575
>(
6676
...args: [
6777
...creators: Creators,
68-
reducer: OnReducer<
69-
State extends infer S ? S : never,
70-
Creators,
71-
InferredState
72-
>,
78+
reducer: (
79+
state: unknown extends State ? InferredState : State,
80+
action: ActionType<Creators[number]>
81+
) => R &
82+
(Exclude<keyof R, keyof EffectiveState> extends never
83+
? unknown
84+
: ExcessPropertiesAreNotAllowed),
7385
]
7486
): ReducerTypes<unknown extends State ? InferredState : State, Creators> {
7587
const reducer = args.pop() as unknown as OnReducer<

‎projects/www/src/app/pages/guide/eslint-plugin/rules/on-function-explicit-return-type.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
`On` function should have an explicit return type.
44

55
- **Type**: suggestion
6+
- **Deprecated**: Yes
67
- **Fixable**: No
78
- **Suggestion**: Yes
89
- **Requires type checking**: No
@@ -11,6 +12,10 @@
1112
<!-- Everything above this generated, do not edit -->
1213
<!-- MANUAL-DOC:START -->
1314

15+
## Deprecation Notice
16+
17+
This rule is deprecated. The `on` function now enforces exact return types at the type level, so excess properties in `on` callbacks will produce TypeScript compilation errors without needing an explicit return type annotation. It is safe to remove this rule from your ESLint configuration. See the [Reducers guide](guide/store/reducers) for details and known limitations.
18+
1419
## Rule Details
1520

1621
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.

‎projects/www/src/app/pages/guide/store/reducers.md‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,33 @@ In the example above, the reducer is handling 4 actions: `[Scoreboard Page] Home
120120

121121
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.
122122

123+
### Exact return type enforcement
124+
125+
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 will produce a TypeScript compilation error:
126+
127+
<ngrx-code-example header="scoreboard.reducer.ts">
128+
129+
```ts
130+
export const scoreboardReducer = createReducer(
131+
initialState,
132+
on(ScoreboardPageActions.homeScore, (state) => ({
133+
...state,
134+
home: state.home + 1,
135+
// TS error: 'on()' callback return type must exactly match
136+
// the state type. Remove excess properties.
137+
extra: true,
138+
}))
139+
);
140+
```
141+
142+
</ngrx-code-example>
143+
144+
<ngrx-docs-alert type="inform">
145+
146+
**Note:** When `on` is used inside a generic reducer factory where the state type is an unresolved generic parameter (e.g., `function createGenericReducer<TState>()`), TypeScript cannot fully resolve the excess property check because `keyof TState` is deferred. In those cases, callbacks that spread state and override known properties may produce a false type error. Return `state` directly or use a type assertion (`as TState`) as a workaround.
147+
148+
</ngrx-docs-alert>
149+
123150
<ngrx-docs-alert type="inform">
124151

125152
**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).

0 commit comments

Comments
 (0)