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

Fix redux plugin type definition #301

Merged
merged 4 commits into from
Jun 2, 2018
Merged

Fix redux plugin type definition #301

merged 4 commits into from
Jun 2, 2018

Conversation

eatonphil
Copy link
Contributor

Closes #300.

Does this make sense?

@troch
Copy link
Member

troch commented May 29, 2018

What's dispatched are actions with type and an optional payload. The payload (when present) can contain a route, previousRoute and optionally a transitionError.

@eatonphil
Copy link
Contributor Author

@troch not sure what that implies. The Typescript compiler was failing because Dispatch expects to act on an object containing a type field and no type field was declared in the TState type.

@troch
Copy link
Member

troch commented May 29, 2018

The initial type signature is incorrect, { router: RouterState } is the type of the reducer output rather than the type of actions dispatched.

Would you mind correcting it as well as adding type?

@eatonphil
Copy link
Contributor Author

Got it. Look better now?

@troch
Copy link
Member

troch commented May 29, 2018

RouterState is not exactly payload. I think actions should be typed like this:

interface Action {
    type: string,
    payload?: {
        route: State
        previousRoute: State | null
        transitionError?: any
    }
}

@eatonphil
Copy link
Contributor Author

Oh I see. How about now?

@troch
Copy link
Member

troch commented Jun 2, 2018

Thanks 👍

@troch troch changed the title Add type field to TState Fix redux plugin type definition Jun 2, 2018
@troch troch merged commit 2c08640 into router5:master Jun 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants