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 generic defaulting w/ middleware types #155

Merged

Conversation

petejohanson
Copy link
Contributor

  • Need to put the TEntities first in the generic parameters,
    so the action generic type can be defaulted properly.

* Need to put the `TEntities` first in the generic parameters,
  so the action generic type can be defaulted properly.
@@ -254,7 +254,7 @@ declare module 'redux-query' {
};

export const queryMiddleware: QueryMiddlewareFactory;
export interface ReduxQueryDispatch<A extends AnyAction = ReduxQueryAction<TEntities>, TEntities = Entities> {
export interface ReduxQueryDispatch<TEntities = Entities, A extends AnyAction = ReduxQueryAction<TEntities>> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the change in order here?

Choose a reason for hiding this comment

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

Otherwise you get this errror:

node_modules/redux-query/index.d.ts:257:78 - error TS2744: Type parameter defaults can only reference previously declared type parameters.

257 export interface ReduxQueryDispatch<A extends AnyAction = ReduxQueryAction, TEntities = Entities> {

See also OpenAPITools/openapi-generator#3824

@rctbusk rctbusk merged commit 800082e into amplitude:master Nov 1, 2019
@petejohanson
Copy link
Contributor Author

@rctbusk Thanks for merging this! Any chance we can get a 3.2.1 release that publishes this?

@rctbusk
Copy link
Contributor

rctbusk commented Nov 4, 2019

Yep! Working on testing the patch now

@rctbusk
Copy link
Contributor

rctbusk commented Nov 4, 2019

I have published an alpha release and am testing it now. https://www.npmjs.com/package/redux-query

@rctbusk
Copy link
Contributor

rctbusk commented Nov 5, 2019

A full version (3.2.1) should be published now

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.

3 participants