Incomplete comment for AnyAction in index.d.ts

[joshburgess]

Do you want to request a feature or report a bug?

Bug -- Sort of... it's an incomplete comment in index.d.ts

What is the current behavior?

The comment above AnyAction is incomplete. See here:

/**
 * An Action type which accepts any other properties.
 * This is mainly for the use of the `Reducer` type.
 * This is not part of `Action` itself to prevent users who are extending `Action.
 */
export interface AnyAction extends Action {
  // Allows any extra properties to be defined in an action.
  [extraProps: string]: any
}

This sentence never finishes, failing to explain the reasoning for the AnyAction type to exist.

This is not part of `Action` itself to prevent users who are extending `Action. ...???

What is the expected behavior?

This comment should finish its sentence and be more clear.

---

I can submit a PR once told what this is supposed to say, but I'm not quite sure just by looking at it... It's obvious the rest of the sentence was not included, but not obvious what it was going to say.

However, honestly, I'm not quite sure having AnyAction makes much sense here. It appears to only be adding an index signature to the base Action type, but I think we actually want that index signature on Action too, as we should be able to do things like someAction['type'].... and, if that's the case, what does AnyAction even do for us here? TypeScript does not enforce exactly matching properties in object types. It only requires us to meet the requirements of the interface/type, ignoring any extra properties. So, we can still include extra properties with the Action type.

This leaves me wondering... do we even need this AnyAction type at all?

[timdorr]

Yes, we need AnyAction. Here's the PR that added it and here's the specific commit that added it. In fact, there's even a comment specifically about AnyAction, which should be particularly helpful.

As for the comment in question, I'm not actually sure where that was going. @aikoven, any chance you remember?

[aikoven]

I think that it has to be something like

This is not part of `Action` itself to prevent types that extend `Action` from having an index signature.

AnyAction is indeed not the same as Action. AnyAction is a type that covers all action types, and Action is a type that is covered by any action type. I.e. union vs. intersection.

[aikoven]

The actual PR that added AnyAction: #2467

[joshburgess]

You could easily add the same index signature to Action though. In other words, Action and AnyAction could easily just be one type. I don't really see the benefit of not doing this.

interface Action<T = any> {
  [key: string]: any
  type: T
}

I actually don't really understand the reasoning behind defaulting T to any there either. It seems to have been done for convenience so that users don't have to supply a generic, but that seems somewhat non-idiomatic to me. It's not actually useful, because there's not really ever a case where we want type to be seen as any... particularly because we want to be able to narrow down types in reducers.

Right now, the tests in tests/typescript only use it in this way. They don't supply a generic. But, in real life usage, you'd most likely always want to provide the generic... You'd usually either make your type that extends Action also take the same generic and pass it through or just hard-code the generic for Action. Examples:

interface MyAction<T extends string, P> extends Action<T> {
  payload: P
}

or

interface MyAction<M> extends Action<'MY_ACTION'> {
  meta: M
}

I think it would make more sense to default to the most common use case, string, instead of any, if providing a default, but it probably doesn't need a default at all.

I think the types & the tests for the types could be improved in other ways too, but that's probably better for a separate conversation. Back to the AnyAction comment:

This is not part of Actionitself to prevent types that extendAction from having an index signature.

Why do we want to do this? You should be able to to access the type property via index even on an action that only contains type & no other properties. Ex:

const someActionType = someAction['type']

Yes, you don't need an index signature to access that property, since you can just do someAction.type, but I don't see a reason why it shouldn't be supported. 'type' is a property on the object just like any other. I would expect all actions to have an index signature.

[joshburgess]

You could also go further:

interface Action<T, Values = any> {
  [key: string]: T | Values
  type: T
}

if you wanted to allow the user to better lock down the possible value types in the action. This seems like a better use case for defaulting a generic to any to me.

[aikoven]

I don't really see the benefit of not doing this.

interface Action<T = any> {
 [key: string]: any
 type: T
}

This would kill type safety. Imagine you had an action like:

interface MyAction extends Action {
  payload: number;
}

With index signature, TS won't catch errors like this:

const action: MyAction = ...
action.payloa; // mistake, not caught by the compiler

It's not actually useful, because there's not really ever a case where we want type to be seen as any... particularly because we want to be able to narrow down types in reducers.

Redux may be used in lots of different ways, and it's not true that you'd always want to narrow the type of type. There are even cases when possible values for type are unknown in advance. You could also use numbers for types, and that's fine. That's why any. A generic is needed when you want your actions to be more strict.

You could also go further:

interface Action<T, Values = any> {
 [key: string]: T | Values
 type: T
}

This way we'd make actions look like dictionaries, which they are generally not.

[joshburgess]

...

This would kill type safety. Imagine you had an action like:

interface MyAction extends Action {
payload: number;
}
With index signature, TS won't catch errors like this:

const action: MyAction = ...
action.payloa; // mistake, not caught by the compiler

Ah, sorry. You're right. I just tried this. I didn't realize TS would allow any extra misc properties here. I was thinking it was solely about index access for annotated properties & the types of the values assigned to them for some reason. I guess the index signature is more about using Records to dynamically perform lookups based on IDs, like string UUIDs.

Redux may be used in lots of different ways, and it's not true that you'd always want to narrow the type of type.

I'm not sure I follow you here. Why would you ever not want the capability to narrow down action types in reducers (based on the type property)? In a statically typed environment, you want to know which action you're working with so that your update logic is safe. You may not always need to narrow down the action types, depending on your update logic (like if you want to do the same thing in response to a variety of different actions), but I'm having trouble thinking of how always having that capability would be a bad thing.

There are even cases when possible values for type are unknown in advance.

What do you mean? The value may be unknown (if dynamically generated), but you would, at least, know the type of that value.

Side-note -- It is possible to dynamically generate unique types with some type system trickery to simulate nominal typing. See here if interested: https://gist.github.com/joshburgess/729a2aeb7a9d6c09e3ac5a95cd2071be

You could also use numbers for types, and that's fine. That's why any. A generic is needed when you want your actions to be more strict.

Why would any be needed? The generic could be supplied as string | number, like:

interface MyAction extends Action<string | number> {}

Or, if you wanted specific literals (more useful, because it allows you to narrow down the action types in reducers), something like:

interface MyAction<T extends string | number> extends Action<T> {}

const actionA: MyAction<'TEST_ACTION'> = { type: 'TEST_ACTION' } // ok
const actionB: MyAction<'TEST_ACTION'> = { type: 'OTHER_ACTION' } // type error
const actionC: MyAction<'0'> = { type: '0' } // ok
const actionD: MyAction<'0'> = { type: '2' } // type error

These things could even be inferred via action creators generically if the action creator functions themselves used generics and passed them along to the return type.

I think it would be extremely rare to want type to be any... I'm not sure I can think of a good use case for it. Wouldn't we want the user to supply the rare case rather than default to it?

You could also go further:

interface Action<T, Values = any> {
[key: string]: T | Values
type: T
}
This way we'd make actions look like dictionaries, which they are generally not.

Values can be a discriminated union here. For example, it could be your Payload type or Meta type.

type Payload = { users: Users[], count: number }
type Meta = { lastModified: Date }

interface Action<T, Values = any> {
 [key: string]: T | Values
 type: T
}

interface MyAction<T extends string> extends Action<T, Payload | Meta> {
  payload: Payload
  meta: Meta
}

However, you were right with the first point about this index signature allowing extra properties, and that definitely is a problem & a good reason not to include that index signature on the base Action type.

Sorry if this is too off topic vs the original post. Maybe, I should make a PR to fix that AnyAction comment and discuss potential updates to the typings in a separate issue or PR?

[joshburgess]

Here is that PR for the AnyAction comment: #3316