Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Require action types to be strings #4544

Merged
merged 4 commits into from
May 9, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
throw error if action type is not string
  • Loading branch information
EskiMojo14 committed May 8, 2023
commit 1ff7eddba5df08e937c45ac24b7b437b742c773c
8 changes: 8 additions & 0 deletions src/createStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,14 @@ export function createStore<
)
}

if (typeof action.type !== 'string') {
throw new Error(
`Action "type" property must be a string. Instead, the actual type was: ${kindOf(
action.type
)}.`
)
}

if (isDispatching) {
throw new Error('Reducers may not dispatch actions.')
}
Expand Down
5 changes: 2 additions & 3 deletions src/types/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,15 @@
*
* Actions must have a `type` field that indicates the type of action being
* performed. Types can be defined as constants and imported from another
* module. It's better to use strings for `type` than Symbols because strings
* are serializable.
* module. These must be strings, as strings are serializable.
*
* Other than `type`, the structure of an action object is really up to you.
* If you're interested, check out Flux Standard Action for recommendations on
* how actions should be constructed.
*
* @template T the type of the action's `type` tag.
*/
export interface Action<T = any> {
export interface Action<T extends string = string> {
timdorr marked this conversation as resolved.
Show resolved Hide resolved
type: T
}

Expand Down
28 changes: 4 additions & 24 deletions test/combineReducers.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ describe('Utils', () => {

it('throws an error if a reducer returns undefined handling an action', () => {
const reducer = combineReducers({
counter(state: number = 0, action: Action<unknown>) {
counter(state: number = 0, action: Action) {
switch (action && action.type) {
case 'increment':
return state + 1
Expand Down Expand Up @@ -95,7 +95,7 @@ describe('Utils', () => {

it('throws an error on first call if a reducer returns undefined initializing', () => {
const reducer = combineReducers({
counter(state: number, action: Action<unknown>) {
counter(state: number, action: Action) {
switch (action.type) {
case 'increment':
return state + 1
Expand All @@ -122,23 +122,6 @@ describe('Utils', () => {
).toThrow(/Error thrown in reducer/)
})

it('allows a symbol to be used as an action type', () => {
const increment = Symbol('INCREMENT')

const reducer = combineReducers({
counter(state: number = 0, action: Action<unknown>) {
switch (action.type) {
case increment:
return state + 1
default:
return state
}
}
})

expect(reducer({ counter: 0 }, { type: increment }).counter).toEqual(1)
})

it('maintains referential equality if the reducers it is combining do', () => {
const reducer = combineReducers({
child1(state = {}) {
Expand All @@ -161,10 +144,7 @@ describe('Utils', () => {
child1(state = {}) {
return state
},
child2(
state: { count: number } = { count: 0 },
action: Action<unknown>
) {
child2(state: { count: number } = { count: 0 }, action: Action) {
switch (action.type) {
case 'increment':
return { count: state.count + 1 }
Expand All @@ -185,7 +165,7 @@ describe('Utils', () => {

it('throws an error on first call if a reducer attempts to handle a private action', () => {
const reducer = combineReducers({
counter(state: number, action: Action<unknown>) {
counter(state: number, action: Action) {
switch (action.type) {
case 'increment':
return state + 1
Expand Down
29 changes: 20 additions & 9 deletions test/createStore.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import {
StoreEnhancer,
Action,
Store,
Reducer
Reducer,
AnyAction
} from 'redux'
import { vi } from 'vitest'
import {
Expand Down Expand Up @@ -567,17 +568,27 @@ describe('createStore', () => {

it('throws if action type is undefined', () => {
const store = createStore(reducers.todos)
expect(() => store.dispatch({ type: undefined })).toThrow(
/Actions may not have an undefined "type" property/
)
expect(() =>
store.dispatch({ type: undefined } as unknown as AnyAction)
).toThrow(/Actions may not have an undefined "type" property/)
})

it('does not throw if action type is falsy', () => {
it('throws if action type is not string', () => {
const store = createStore(reducers.todos)
expect(() => store.dispatch({ type: false })).not.toThrow()
expect(() => store.dispatch({ type: 0 })).not.toThrow()
expect(() => store.dispatch({ type: null })).not.toThrow()
expect(() => store.dispatch({ type: '' })).not.toThrow()
const expectedErr = /Action "type" property must be a string/
expect(() =>
store.dispatch({ type: false } as unknown as AnyAction)
).toThrow(expectedErr)
expect(() => store.dispatch({ type: 0 } as unknown as AnyAction)).toThrow(
expectedErr
)
expect(() =>
store.dispatch({ type: null } as unknown as AnyAction)
).toThrow(expectedErr)

expect(() =>
store.dispatch({ type: '' } as unknown as AnyAction)
).not.toThrow()
})

it('accepts enhancer as the third argument', () => {
Expand Down
18 changes: 0 additions & 18 deletions test/typescript/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,21 +39,3 @@ namespace StringLiteralTypeAction {

const type: ActionType = action.type
}

namespace EnumTypeAction {
enum ActionType {
A,
B,
C
}

interface Action extends ReduxAction {
type: ActionType
}

const action: Action = {
type: ActionType.A
}

const type: ActionType = action.type
}
16 changes: 8 additions & 8 deletions test/typescript/enhancers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,12 +159,12 @@ function replaceReducerExtender() {
test?: boolean
}

const initialReducer: Reducer<PartialState, Action<unknown>> = () => ({
const initialReducer: Reducer<PartialState, Action> = () => ({
someField: 'string'
})
const store = createStore<
PartialState,
Action<unknown>,
Action,
{ method(): string },
ExtraState
>(initialReducer, enhancer)
Expand Down Expand Up @@ -246,10 +246,10 @@ function mhelmersonExample() {
test?: boolean
}

const initialReducer: Reducer<PartialState, Action<unknown>> = () => ({
const initialReducer: Reducer<PartialState, Action> = () => ({
someField: 'string'
})
const store = createStore<PartialState, Action<unknown>, {}, ExtraState>(
const store = createStore<PartialState, Action, {}, ExtraState>(
initialReducer,
enhancer
)
Expand All @@ -276,7 +276,7 @@ function finalHelmersonExample() {
foo: string
}

function persistReducer<S, A extends Action<unknown>, PreloadedState>(
function persistReducer<S, A extends Action, PreloadedState>(
config: any,
reducer: Reducer<S, A, PreloadedState>
) {
Expand All @@ -300,7 +300,7 @@ function finalHelmersonExample() {
persistConfig: any
): StoreEnhancer<{}, ExtraState> {
return createStore =>
<S, A extends Action<unknown>, PreloadedState>(
<S, A extends Action, PreloadedState>(
reducer: Reducer<S, A, PreloadedState>,
preloadedState?: PreloadedState | undefined
) => {
Expand All @@ -323,10 +323,10 @@ function finalHelmersonExample() {
test?: boolean
}

const initialReducer: Reducer<PartialState, Action<unknown>> = () => ({
const initialReducer: Reducer<PartialState, Action> = () => ({
someField: 'string'
})
const store = createStore<PartialState, Action<unknown>, {}, ExtraState>(
const store = createStore<PartialState, Action, {}, ExtraState>(
initialReducer,
createPersistEnhancer('hi')
)
Expand Down