Skip to content

Commit f5a3b4b

Browse files
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. 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 6582ebd commit f5a3b4b

File tree

5 files changed

+241
-13
lines changed

5 files changed

+241
-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.
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.

0 commit comments

Comments
 (0)