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

store.dispatch does not work in 1.2.2 #321

Closed
yhnavein opened this issue Jan 17, 2020 · 15 comments · Fixed by #329
Closed

store.dispatch does not work in 1.2.2 #321

yhnavein opened this issue Jan 17, 2020 · 15 comments · Fixed by #329

Comments

@yhnavein
Copy link

After migrating to 1.2.2 store.dispatch stopped working with the following error message:

Argument of type 'ThunkAction<void, CombinedState<{ auth: AuthState; }>, null, Action<string>>' is not assignable to parameter of type 'AnyAction'.
  Property 'type' is missing in type 'ThunkAction<void, CombinedState<{ auth: AuthState; }>, null, Action<string>>' but required in type 'AnyAction'.

Dispathing through dispatcher from hook works fine though:

  const dispatch = useDispatch();
  ...
  dispatch(logOut());  // OK

I can't use React Hooks in the axios interceptor, so I was using this:

import store from '../../store';

store.dispatch(logOut());
@phryneas
Copy link
Member

Could you please hover over store.dispatch and copy&paste what it gives you as definition there?

Also, are you using custom middlewares?

jonjaques referenced this issue Jan 21, 2020
* correctly infer dispatch from provided middlewares

* simplify getDefaultMiddleware typings

* test readability

* documentation, update api report

* extract tests for >=3.4

* update documentation

* Tweak TS usage wording

Co-authored-by: Mark Erikson <mark@isquaredsoftware.com>
@jonjaques
Copy link

jonjaques commented Jan 21, 2020

One more instance of error for ya:

Type 'ThunkAction<any, CombinedState<{ router: RouterState<any>; form: FormStateMap; user: IUser; cameras: IDuckState; users: IUsersState; global: IGlobalState; ... 7 more ...; connectionManager: IConnectManagerState; }>, { ...; }, IAction>' is missing the following properties from type 'LocationChangeAction<any>': type, payload

typeof store.dispatch:

EnhancedStore<CombinedState<{ router: RouterState<any>; form: FormStateMap; user: IUser; cameras: IDuckState; users: IUsersState; global: IGlobalState; ... 7 more ...; connectionManager: IConnectManagerState; }>, AnyAction | LocationChangeAction<...>, any[]>.dispatch: Dispatch
<AnyAction | LocationChangeAction<any>>(action: AnyAction | LocationChangeAction<any>) => AnyAction | LocationChangeAction<...>

This happens to be complaining about types being picked up from 'connected-react-router' but I don't think that's the issue. Edit: Sorry for lack of details, will try to test out the proposed change this evening.

@phryneas
Copy link
Member

phryneas commented Jan 21, 2020

Hm. As for your comment: It's definitely a &, not a |. See this playground. Function signature overloads work that way.

How does your middlewares option to configureStore look? You might need to add a as const there

@yhnavein
Copy link
Author

Could you please hover over store.dispatch and copy&paste what it gives you as definition there?
Also, are you using custom middlewares?

(property) EnhancedStore<{ readonly [$CombinedState]?: undefined; } & { auth: AuthState; managePsw: ManagePswState; journal: JournalState; } & PersistPartial, AnyAction, (ThunkMiddleware<...> & { ...; })[]>.dispatch: Dispatch
<any>(action: any) => any

I am using only thunk middleware:

import thunk, { ThunkAction } from 'redux-thunk';

const store = configureStore({
  reducer: persistReducer(persistConfig, rootReducer),
  middleware: [thunk],
});

export type AppThunk = ThunkAction<void, RootState, null, Action<string>>;

@phryneas
Copy link
Member

Could you please hover over store.dispatch and copy&paste what it gives you as definition there?
Also, are you using custom middlewares?

(property) EnhancedStore<{ readonly [$CombinedState]?: undefined; } & { auth: AuthState; managePsw: ManagePswState; journal: JournalState; } & PersistPartial, AnyAction, (ThunkMiddleware<...> & { ...; })[]>.dispatch: Dispatch
<any>(action: any) => any

I am using only thunk middleware:

import thunk, { ThunkAction } from 'redux-thunk';

const store = configureStore({
  reducer: persistReducer(persistConfig, rootReducer),
  middleware: [thunk],
});

export type AppThunk = ThunkAction<void, RootState, null, Action<string>>;

@yhnavein I'll look into that.
In the meantime: thunk is already integrated as a middleware, you don't need to specify that there. I guess skipping middleware altogether will solve your problem already.

@yhnavein
Copy link
Author

If I delete line with middleware: [thunk] then my error is like this:

No overload matches this call.
  Overload 1 of 2, '(action: AnyAction): AnyAction', gave the following error.
    Argument of type 'ThunkAction<void, CombinedState<{ auth: AuthState; managePsw: ManagePswState; journal: JournalState; }>, null, Action<string>>' is not assignable to parameter of type 'AnyAction'.
      Property 'type' is missing in type 'ThunkAction<void, CombinedState<{ auth: AuthState; managePsw: Manag
ePswState; journal: JournalState; }>, null, Action<string>>' but required in type 'AnyAction'.
  Overload 2 of 2, '(asyncAction: ThunkAction<void, { readonly [$CombinedState]?: undefined; } & { auth: Auth
State; managePsw: ManagePswState; journal: JournalState; } & PersistPartial, undefined, AnyAction>): void', g
ave the following error.
    Argument of type 'ThunkAction<void, CombinedState<{ auth: AuthState; managePsw: ManagePswState; journal: JournalState; }>, null, Action<string>>' is not assignable to parameter of type 'ThunkAction<void, { readonly
 [$CombinedState]?: undefined; } & { auth: AuthState; managePsw: ManagePswState; journal: JournalState; } & P
ersistPartial, undefined, AnyAction>'.
      Type 'undefined' is not assignable to type 'null'.  TS2769

@phryneas
Copy link
Member

phryneas commented Jan 22, 2020

If I delete line with middleware: [thunk] then my error is like this:

No overload matches this call.
  Overload 1 of 2, '(action: AnyAction): AnyAction', gave the following error.
    Argument of type 'ThunkAction<void, CombinedState<{ auth: AuthState; managePsw: ManagePswState; journal: JournalState; }>, null, Action<string>>' is not assignable to parameter of type 'AnyAction'.
      Property 'type' is missing in type 'ThunkAction<void, CombinedState<{ auth: AuthState; managePsw: Manag
ePswState; journal: JournalState; }>, null, Action<string>>' but required in type 'AnyAction'.
  Overload 2 of 2, '(asyncAction: ThunkAction<void, { readonly [$CombinedState]?: undefined; } & { auth: Auth
State; managePsw: ManagePswState; journal: JournalState; } & PersistPartial, undefined, AnyAction>): void', g
ave the following error.
    Argument of type 'ThunkAction<void, CombinedState<{ auth: AuthState; managePsw: ManagePswState; journal: JournalState; }>, null, Action<string>>' is not assignable to parameter of type 'ThunkAction<void, { readonly
 [$CombinedState]?: undefined; } & { auth: AuthState; managePsw: ManagePswState; journal: JournalState; } & P
ersistPartial, undefined, AnyAction>'.
      Type 'undefined' is not assignable to type 'null'.  TS2769

This seems to somehow be triggered by your CombinedState generic type... could you please share that type? (And maybe also a thunk that is exposing that problem? I believe those two are not using the same State)

@belgattitude
Copy link

belgattitude commented Jan 23, 2020

Same problem updating to latest, so here's a code snippet

The store.ts config:

import { configureStore, Action } from '@reduxjs/toolkit';
import { ThunkAction } from 'redux-thunk';
import { authReducer } from './features/auth/auth.redux';

export const store = configureStore({
    reducer: {
        auth: authReducer,
    },
});

export type AppThunk = ThunkAction<void, RootState, null, Action<string>>;
export type RootState = ReturnType<typeof store.getState>;

The thunk in auth.redux.ts:

export const runLogoutThunk = (deps?: { tokenStore?: ITokenStore }): AppThunk => async dispatch => {
    try {
        const { tokenStore = getTokenStore() } = deps || {};
        const token = tokenStore.getToken();
        tokenStore.removeToken();
        if (token) {
            const authResult = await authApi.logout(token);
        }
    } catch (e) {
    } finally {
        dispatch(getAuthLogout());
    }
};

And the TS error

import { store } from '../../store';

export const createDefaultApiService = () => {
    const serverUrl = 'http://localhost:3000';
    const tokenStore = getTokenStore();
    const refreshTokenService = new RefreshTokenService({
        tokenStore: tokenStore,
        refreshTokenUrl: `${serverUrl}/auth/refresh-token`,
    });
    return new ApiService({
        serverUrl,
        refreshTokenService: refreshTokenService,
        tokenStore: tokenStore,
        onAuthFailure: (response): Response => {
           /// TYPESCRIPT IS COMPLAINING HERE ABOUT
           /// ------------------------------------------------------------------
           // error TS2769: No overload matches this call.
           // Overload 1 of 2, '(action: AnyAction): AnyAction', gave the following error.
           // ...
            store.dispatch(runLogoutThunk());
            return response;
        },
    });
};

The complete error

src/core/api/api-service.ts:112:28 - error TS2769: No overload matches this call.
  Overload 1 of 2, '(action: AnyAction): AnyAction', gave the following error.
    Argument of type 'ThunkAction<void, { auth: AuthState; }, null, Action<string>>' is not assignable to parameter of type 'AnyAction'.
      Property 'type' is missing in type 'ThunkAction<void, { auth: AuthState; }, null, Action<string>>' but required in type 'AnyAction'.
  Overload 2 of 2, '(asyncAction: ThunkAction<void, { auth: AuthState; }, undefined, AnyAction>): void', gave the following error.
    Argument of type 'ThunkAction<void, { auth: AuthState; }, null, Action<string>>' is not assignable to parameter of type 'ThunkAction<void, { auth: AuthState; }, undefined, AnyAction>'.
      Type 'undefined' is not assignable to type 'null'.

112             store.dispatch(runLogoutThunk());
                               ~~~~~~~~~~~~~~~~

  ../../node_modules/redux/index.d.ts:21:3
    21   type: T
         ~~~~
    'type' is declared here.

Don't know if it helps getting an idea... I'll try to investigate in the week-end if not found before. Any help appreciated and keep up the fantastic work !

Edit: reverting to 1.2.1 works

@phryneas
Copy link
Member

phryneas commented Jan 23, 2020

@belgattitude I think I can see the problem there. Does it work if you change it like this?

- export type AppThunk = ThunkAction<void, RootState, null, Action<string>>;
+ export type AppThunk = ThunkAction<void, RootState, undefined, Action<string>>;

I believe I may have found the root cause for this:

Now, we get Dispatch from the middleware via

M extends Middleware<infer DispatchExt, any, any>
  ? DispatchExt extends Function
    ? DispatchExt
  : never
: never

and our Middleware is per default

ThunkMiddleware<S>. After some resolving, all these are equal:

type DefaultMiddleware<S> = ThunkMiddleware<S>
type DefaultMiddleware<S> = ThunkMiddleware<S, AnyAction, undefined, ThunkDispatch<S, undefined, AnyAction>>
type DefaultMiddleware<S> = Middleware<ThunkDispatch<S, undefined, AnyAction>, S, <ThunkDispatch<S, undefined, AnyAction>

So we'd extract a Dispatch type of ThunkDispatch<S, undefined, AnyAction> from this.

Back in 1.2.1, the Dispatch type was ThunkDispatch<S, any, AnyAction> which would not conflict with the new Dispatch type, but was a little more loose.
Unfortunately, the advanced tutorial recommends adding type AppThunk = ThunkAction<void, RootState, null, Action<string>> which would have been compatible with the old, more loose definition, but not with the new definition that is more strict, but more in line with the redux-thunk default behavior.

Let's see if my fix above would fix it for @belgattitude - as a result, I might have to specify the ThunkMiddleware to be a ThunkMiddleware<S, AnyAction, any, ThunkDispatch<S, any, AnyAction>>. Or we change the tutorial and try to communicate the breakage there. Would be cleaner, but more of a hassle for users. What do you think @markerikson ?

@markerikson
Copy link
Collaborator

Both would probably be good, I guess.

@belgattitude
Copy link

@phryneas,

Does it work if you change it like this?

+ export type AppThunk = ThunkAction<void, RootState, undefined, Action<string>>;

Nope

error TS2345: Argument of type 'null' is not assignable to parameter of type 'undefined'.
55  await runLogoutThunk({ tokenStore: ts })(dispatch, getState, null);

Setting it to unknown does the job, but I'm not sure about the implications.

@phryneas
Copy link
Member

Nice, you went from

Type 'undefined' is not assignable to type 'null'.
to
Argument of type 'null' is not assignable to parameter of type 'undefined'.
there 😆

unknown should be a good bet there.

@ yhnavein and @jonjaques does that solve your problems as well?
Then I'd try to do a PR over the weekend.

@yhnavein
Copy link
Author

@phryneas For me it does indeed fix the issue and everything works fine :)

phryneas pushed a commit to phryneas/redux-toolkit that referenced this issue Jan 24, 2020
@phryneas
Copy link
Member

phryneas commented Jan 24, 2020

Okay given that feedback so far, I opened a PR for a fix.

Could you please try the CodeSandbox build of that PR with your original code and report if it fixes the problems?

yarn add https://pkg.csb.dev/reduxjs/redux-toolkit/commit/933aa4a1/@reduxjs/toolkit
or
npm i https://pkg.csb.dev/reduxjs/redux-toolkit/commit/933aa4a1/@reduxjs/toolkit

@belgattitude
Copy link

@phryneas

Updated to unknown type and installed the sandbox... it seems to work ;)

Thanks for your help, very clear and super pro... Amazing :)

markerikson pushed a commit that referenced this issue Feb 9, 2020
* fix #321

* better type for AppThunk in advanced tutorial

* keep backwards-compatiblity with ThunkActions with a `null` ExtraArgument

* small tweak, add type test

* lint
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 a pull request may close this issue.

5 participants