Skip to content

Commit

Permalink
feat(store): provide better TS errors for action creator props (#3060)
Browse files Browse the repository at this point in the history
Closes #2892 

BREAKING CHANGES:

Types for props outside an action creator is more strictly checked

BEFORE:

Usage of `props` outside of an action creator with invalid types was allowed

AFTER:

Usage of `props` outside of an action creator now breaks for invalid types
  • Loading branch information
david-shortman authored Nov 2, 2021
1 parent 2b53495 commit 5ed3c3d
Show file tree
Hide file tree
Showing 4 changed files with 107 additions and 12 deletions.
66 changes: 61 additions & 5 deletions modules/store/spec/types/action_creator.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,23 +39,79 @@ describe('createAction()', () => {
expectSnippet(`
const foo = createAction('FOO', props<{ type: number }>());
`).toFail(
/ Type 'ActionCreatorProps<\{ type: number; \}>' is not assignable to type '"type property is not allowed in action creators"'/
/Type '{ type: number; }' does not satisfy the constraint '"action creator props cannot have a property named `type`"'/
);
});

it('should not allow arrays', () => {
expectSnippet(`
const foo = createAction('FOO', props<[]>());
`).toFail(
/Type 'ActionCreatorProps<\[\]>' is not assignable to type '"arrays are not allowed in action creators"'/
/Type '\[]' does not satisfy the constraint '"action creator props cannot be an array"'/
);
});

it('should not allow empty objects', () => {
expectSnippet(`
const foo = createAction('FOO', props<{}>());
`).toFail(
/Type 'ActionCreatorProps<\{\}>' is not assignable to type '"empty objects are not allowed in action creators"'/
/Type '{}' does not satisfy the constraint '"action creator props cannot be an empty object"'/
);
});

it('should not allow strings', () => {
expectSnippet(`
const foo = createAction('FOO', props<string>());
`).toFail(
/Type 'string' does not satisfy the constraint '"action creator props cannot be a primitive value"'/
);
});

it('should not allow numbers', () => {
expectSnippet(`
const foo = createAction('FOO', props<number>());
`).toFail(
/Type 'number' does not satisfy the constraint '"action creator props cannot be a primitive value"'/
);
});

it('should not allow bigints', () => {
expectSnippet(`
const foo = createAction('FOO', props<bigint>());
`).toFail(
/Type 'bigint' does not satisfy the constraint '"action creator props cannot be a primitive value"'/
);
});

it('should not allow booleans', () => {
expectSnippet(`
const foo = createAction('FOO', props<boolean>());
`).toFail(
/Type 'boolean' does not satisfy the constraint '"action creator props cannot be a primitive value"'/
);
});

it('should not allow symbols', () => {
expectSnippet(`
const foo = createAction('FOO', props<symbol>());
`).toFail(
/Type 'symbol' does not satisfy the constraint '"action creator props cannot be a primitive value"'/
);
});

it('should not allow null', () => {
expectSnippet(`
const foo = createAction('FOO', props<null>());
`).toFail(
/Type 'ActionCreatorProps<null>' is not assignable to type '"action creator cannot return an array"'/
);
});

it('should not allow undefined', () => {
expectSnippet(`
const foo = createAction('FOO', props<undefined>());
`).toFail(
/Type 'ActionCreatorProps<undefined>' is not assignable to type '"action creator cannot return an array"'/
);
});
});
Expand Down Expand Up @@ -88,7 +144,7 @@ describe('createAction()', () => {
expectSnippet(`
const foo = createAction('FOO', (type: string) => ({type}));
`).toFail(
/Type '\{ type: string; \}' is not assignable to type '"type property is not allowed in action creators"'/
/Type '\{ type: string; \}' is not assignable to type '"action creator cannot return an object with a property named `type`"'/
);
});

Expand All @@ -102,7 +158,7 @@ describe('createAction()', () => {
expectSnippet(`
const foo = createAction('FOO', () => [ ]);
`).toFail(
/Type 'any\[\]' is not assignable to type '"arrays are not allowed in action creators"'/
/Type 'any\[\]' is not assignable to type '"action creator cannot return an array"'/
);
});
});
Expand Down
6 changes: 5 additions & 1 deletion modules/store/src/action_creator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
FunctionWithParametersType,
NotAllowedCheck,
ActionCreatorProps,
NotAllowedInPropsCheck,
} from './models';
import { REGISTERED_ACTION_TYPES } from './globals';

Expand Down Expand Up @@ -124,7 +125,10 @@ export function createAction<T extends string, C extends Creator>(
}
}

export function props<P extends object>(): ActionCreatorProps<P> {
export function props<
P extends SafeProps,
SafeProps = NotAllowedInPropsCheck<P>
>(): ActionCreatorProps<P> {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion, @typescript-eslint/naming-convention
return { _as: 'props', _p: undefined! };
}
Expand Down
3 changes: 1 addition & 2 deletions modules/store/src/feature_creator_models.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { MemoizedSelector } from './selector';
import { Primitive } from './models';

// Generating documentation for `createFeature` function is solved by moving types that use
// template literal types (`FeatureSelector` and `NestedSelectors`) from `feature_creator.ts`.
Expand Down Expand Up @@ -28,5 +29,3 @@ export type NestedSelectors<
FeatureState[K]
>;
};

type Primitive = string | number | bigint | boolean | null | undefined;
44 changes: 40 additions & 4 deletions modules/store/src/models.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,18 +56,33 @@ export type SelectorWithProps<State, Props, Result> = (
props: Props
) => Result;

export const arraysAreNotAllowedMsg =
'arrays are not allowed in action creators';
export const arraysAreNotAllowedMsg = 'action creator cannot return an array';
type ArraysAreNotAllowed = typeof arraysAreNotAllowedMsg;

export const typePropertyIsNotAllowedMsg =
'type property is not allowed in action creators';
'action creator cannot return an object with a property named `type`';
type TypePropertyIsNotAllowed = typeof typePropertyIsNotAllowedMsg;

export const emptyObjectsAreNotAllowedMsg =
'empty objects are not allowed in action creators';
'action creator cannot return an empty object';
type EmptyObjectsAreNotAllowed = typeof emptyObjectsAreNotAllowedMsg;

export const arraysAreNotAllowedInProps =
'action creator props cannot be an array';
type ArraysAreNotAllowedInProps = typeof arraysAreNotAllowedInProps;

export const typePropertyIsNotAllowedInProps =
'action creator props cannot have a property named `type`';
type TypePropertyIsNotAllowedInProps = typeof typePropertyIsNotAllowedInProps;

export const emptyObjectsAreNotAllowedInProps =
'action creator props cannot be an empty object';
type EmptyObjectsAreNotAllowedInProps = typeof emptyObjectsAreNotAllowedInProps;

export const primitivesAreNotAllowedInProps =
'action creator props cannot be a primitive value';
type PrimitivesAreNotAllowedInProps = typeof primitivesAreNotAllowedInProps;

export type FunctionIsNotAllowed<
T,
ErrorMessage extends string
Expand All @@ -80,6 +95,15 @@ export type Creator<
R extends object = object
> = FunctionWithParametersType<P, R>;

export type Primitive =
| string
| number
| bigint
| boolean
| symbol
| null
| undefined;

export type NotAllowedCheck<T extends object> = T extends any[]
? ArraysAreNotAllowed
: T extends { type: any }
Expand All @@ -88,6 +112,18 @@ export type NotAllowedCheck<T extends object> = T extends any[]
? EmptyObjectsAreNotAllowed
: unknown;

export type NotAllowedInPropsCheck<T> = T extends object
? T extends any[]
? ArraysAreNotAllowedInProps
: T extends { type: any }
? TypePropertyIsNotAllowedInProps
: keyof T extends never
? EmptyObjectsAreNotAllowedInProps
: unknown
: T extends Primitive
? PrimitivesAreNotAllowedInProps
: never;

/**
* See `Creator`.
*/
Expand Down

0 comments on commit 5ed3c3d

Please sign in to comment.