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

Update TypeScript typings #2773

Merged
merged 8 commits into from
Jan 24, 2018
Merged

Conversation

aikoven
Copy link
Collaborator

@aikoven aikoven commented Dec 21, 2017

This PR continues work from #2563 and fixes #1648, #2602 and #2740.

Tests are rewritten to cover different supported approaches. All TS strict checks are enabled in tests.

Reducers

  • Reducers support simple definition where action shape is not type-checked to aid the transition from JS to TS.
  • Discriminated union for actions is still supported as well.
  • A test is added for writing reducers with type guards.
  • Typings for combineReducers are updated to allow strict combination using discriminated unions, as well as combining custom and third-party reducers.

See tests.

Dispatch

  • Default Dispatch type only accepts plain actions. It also supports restricting action types using discriminated unions.
  • Extra signatures are added using intersection type e.g. Dispatch & PromiseDispatch.

See tests and tests for injected dispatch e.g. connect from react-redux.

Middleware

  • Middleware type supports declaring extra Dispatch signatures which are added to the Store type by applyMiddleware enhancer.
  • Middleware type supports restricting the state type.

See tests.

Store Enhancers

  • Store enhancer type supports extending Store type. Particularly, Dispatch signature is extending using this mechanism. It also supports for adding custom methods.
  • Support for enhancers extending the state type.

See tests.

cc. @pelotom @KingHenne

@@ -46,7 +56,7 @@ export interface Action<T = any> {
* @template S The type of state consumed and produced by this reducer.
* @template A The type of actions the reducer can potentially respond to.
*/
export type Reducer<S = any, A extends Action = Action> = (state: S | undefined, action: A) => S;
export type Reducer<S = any, A extends Action = AnyAction> = (state: S | undefined, action: A) => S;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, this is not needed, because any interface with additional properties extends Action interface.

Copy link
Collaborator Author

@aikoven aikoven Jan 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default type for A being AnyAction is needed so that any reducer acting on the state State would be assignable to Reducer<State> (with omitted action type parameter).

You may want to change it back to Action and check out the tests that break.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see now. Thanks.

@timdorr
Copy link
Member

timdorr commented Jan 24, 2018

Since no one has come in and yelled "Holy balls, this is terrible!", I'm going to assume this is at least somewhat of an improvement and merge it in.

I swear, I'll learn TS at some point...

@jayphelps
Copy link
Contributor

jayphelps commented May 15, 2018

Edited because it unintentionally could have been read as hostile. Not intended! 🙂


Hey folks 👋 I'm curious if someone could elaborate on the decision to change the order of the generic params MiddlewareAPI<State, Dispatch> was reversed in v4 to MiddlewareAPI<Dispatch, State>? It means that everyone who wants to type their State must provide a Dispatch type so they can get to the second type argument. It seems like it would have been a breaking change for the average TS person upgrading.

@aikoven
Copy link
Collaborator Author

aikoven commented May 16, 2018

@jayphelps Old MiddlewareAPI of Redux v3 didn't have Dispatch parameter:

export interface MiddlewareAPI<S> {
  dispatch: Dispatch<S>;
  getState(): S;
}

The new one has a default for it:

export interface MiddlewareAPI<D extends Dispatch = Dispatch, S = any> {
  dispatch: D;
  getState(): S;
}

If you don't need a custom dispatch signature, you can just use the built-in Dispatch type: MiddlewareAPI<Dispatch, MyState>, or even have it inferred for you, see the test for middleware that expects exact state type.

It seems like it would have been a breaking change for the average TS person upgrading.

That's why we have bumped a major version 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeScript definition for StoreEnhancer prevents state type extension
4 participants