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

Some breakage with new 3.0 beta #56

Open
xdave opened this issue Apr 15, 2018 · 12 comments
Open

Some breakage with new 3.0 beta #56

xdave opened this issue Apr 15, 2018 · 12 comments

Comments

@xdave
Copy link

xdave commented Apr 15, 2018

My package, typescript-fsa-redux-thunk and my attempts to refactor it have failed, and this also affects typescript-fsa-reducers. The best way to explain the errors I'm getting is to post a test case and explain where the errors are:

import {
    Dispatch,
    applyMiddleware,
    bindActionCreators,
    createStore
} from 'redux';
import thunk, { ThunkAction } from 'redux-thunk';
import actionCreatorFactory, {
    AsyncActionCreators,
    ActionCreatorFactory
} from 'typescript-fsa';
import { reducerWithInitialState } from 'typescript-fsa-reducers';

/**
 * It's either a promise, or it isn't
 */
type MaybePromise<T> = T | Promise<T>;

/**
 * A redux-thunk with the params as the first argument.  You don't have to
 * return a promise; but, the result of the dispatch will be one.
 */
export type AsyncWorker<State, Params, Result, Extra = any> = (
    params: Params,
    dispatch: Dispatch<State>,
    getState: () => State,
    extra: Extra
) => MaybePromise<Result>;

/**
 * Bind a redux-thunk to typescript-fsa async action creators
 * @param actionCreators - The typescript-fsa async action creators
 * @param asyncWorker - The redux-thunk with the params as the first argument
 * @returns a regular redux-thunk you can pass to dispatch()
 */
export const bindThunkAction = <State, P, S, E, ExtraArg = any>(
    actionCreators: AsyncActionCreators<P, S, E>,
    asyncWorker: AsyncWorker<State, P, S, ExtraArg>
) => (
    params: P
): ThunkAction<Promise<S>, State, ExtraArg> => (
    dispatch,
    getState,
    extra
) => {
    dispatch(actionCreators.started(params));
    return Promise.resolve(asyncWorker(params, dispatch, getState, extra))
        .then(result => {
            dispatch(actionCreators.done({ params, result }));
            return result;
        })
        .catch((error: E) => {
            dispatch(actionCreators.failed({ params, error }));
            throw error;
        });
};

/**
 * Factory function to easily create a typescript-fsa redux thunk
 * @param factory - typescript-fsa action creator factory
 * @returns an object with the async actions and the thunk itself
 */
export const asyncFactory = <State, E = Error, ExtraArg = any>(
    factory: ActionCreatorFactory
) => <P, S>(type: string, fn: AsyncWorker<State, P, S, ExtraArg>) => {
    const actions = factory.async<P, S, E>(type);
    return {
        async: actions,
        action: bindThunkAction(actions, fn)
    };
};

/**
 * Passing the result of this to bindActionCreators and then calling the result
 * is equivalent to calling `store.dispatch(thunkAction(params))`. Useful for
 * when you pass it to `connect()` as the actionCreators map object.
 * @param thunkAction - The thunk action
 * @returns thunkAction as if it was bound
 */
export const thunkToAction = <P, R, S, ExtraArg>(
    thunkAction: (params: P) => ThunkAction<R, S, ExtraArg>
): ((params: P) => R) => thunkAction as any;


/****** TEST CODE ******/

interface SomeState {
    hmm: number;
}

const create = actionCreatorFactory('something');
const createAsync = asyncFactory<SomeState>(create);
const someAction = create<string>('SOME_ACTION');

const test = createAsync<{ arg: string; }, number>(
    'ASYNC_ACTION',
    async params => {
        console.log(params);
        return 100;
    }
);

const initial: SomeState = {
    hmm: 0
};

const reducer = reducerWithInitialState(initial)
    .case(someAction, state => state)
    .case(test.async.started, state => state)
    .case(test.async.failed, state => state)
    .case(test.async.done, (state, { result }) => ({
        ...state,
        hmm: result
    }));

if (module === require.main) {
    const store = createStore(reducer, applyMiddleware(thunk));

    const actions = bindActionCreators({
        test: thunkToAction(test.action)
    }, store.dispatch);

    actions.test({ arg: 'test' })
        .then(result => console.log(result))
        .catch(err => console.log(err));
}

There are only three errors, and interestingly, all occurring inside of my bindThunkAction function where the async actions are called:

// Line 46:
dispatch(actionCreators.started(params));
// Cannot invoke an expression whose type lacks a call signature. Type '({ type: string; match: (action: AnyAction) => action is Action<P>; } & ((payload?: P | undefined...' has no compatible call signatures.

// Line 49:
dispatch(actionCreators.done({ params, result }));
// Argument of type '{ params: P; result: S; }' is not assignable to parameter of type 'Optionalize<{ params: P; result: S; }>'.
//  Type '{ params: P; result: S; }' is not assignable to type '{ [P in (P extends undefined ? never : "params") | (S extends undefined ? never : "result")]: { p...'.

// Line 53:
dispatch(actionCreators.failed({ params, error }));
// Argument of type '{ params: P; error: E; }' is not assignable to parameter of type 'Optionalize<{ params: P; error: E; }>'.
//  Type '{ params: P; error: E; }' is not assignable to type '{ [P in (P extends undefined ? never : "params") | (E extends undefined ? never : "error")]: { pa...'.

I've been fiddling with my own code for hours, and I can't seem to figure out why this is happening. Passing normal parameters to these action creators outside of bindThunkAction works fine.

Another weirdness: even with these errors, the reducer seems to work; but, the types of params and result, etc have a weird optional-like, for example: number | { undefined & number }.

If one ignores the errors and runs the code with ts-node, it executes without errors. Am I losing it?

@xdave
Copy link
Author

xdave commented Apr 15, 2018

Alright, here's a smaller test case:

import actionCreatorFactory, { AsyncActionCreators } from 'typescript-fsa';

const create = actionCreatorFactory();
const asyncActions = create.async<string, boolean>('SOME_ACTION');

const something = <P, S>(
    actions: AsyncActionCreators<P, S, any>,
    params: P,
    result: S
) => {
    actions.started(params);
    actions.failed({ params, error: 'some error' });
    actions.done({ params, result });
};

something(asyncActions, 'test', true);
src/typescript-fsa-issue-56.ts:11:5 - error TS2349: Cannot invoke an expression whose type lacks a call signature. Type '({ type: string; match: (action: AnyAction) => action is Action<P>; } & ((pa
yload?: P | undefined...' has no compatible call signatures.

11     actions.started(params);
       ~~~~~~~~~~~~~~~~~~~~~~~

src/typescript-fsa-issue-56.ts:12:20 - error TS2345: Argument of type '{ params: P; error: string; }' is not assignable to parameter of type 'Optionalize<{ params: P; error: any; }>'.
  Type '{ params: P; error: string; }' is not assignable to type '{ [P in "error" | (P extends undefined ? never : "params")]: { params: P; error: any; }[P]; }'.

12     actions.failed({ params, error: 'some error' });
                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


src/typescript-fsa-issue-56.ts:13:18 - error TS2345: Argument of type '{ params: P; result: S; }' is not assignable to parameter of type 'Optionalize<{ params: P; result: S; }>'.
  Type '{ params: P; result: S; }' is not assignable to type '{ [P in (P extends undefined ? never : "params") | (S extends undefined ? never : "result")]: { p...'.

13     actions.done({ params, result });
                    ~~~~~~~~~~~~~~~~~~

@xdave
Copy link
Author

xdave commented Apr 15, 2018

OK if I change ActionCreator to be an interface:

interface ActionCreator<P> {
    type: string;
    match: (action: AnyAction) => action is Action<P>;
    (payload: P, meta?: Meta): Action<P>;
    (payload?: P, meta?: Meta): Action<P>;
}

Calling .started(P) works without any problem; however, I must assert the function parameters of the .done() and .failed()actions to Success<> or Failure<> in my function.

For example:

const something = <P, S>(actions: AsyncActionCreators<P, S, any>, params?: P, result?: S) => {
    actions.started(params);
    actions.failed({ params } as Failure<P, any>);
    actions.done({ params, result } as Success<P, S>);
};

After this, the types of the values in the reducer are correct, too.

@xdave xdave changed the title Several things broken with new 3.0 beta Some breakage with new 3.0 beta Apr 17, 2018
@suutari-ai
Copy link
Contributor

@xdave Can you check if PR #57 fixes this? At least it helped me to resolve a problem with very similar error message.

@rplotkin
Copy link

Just adding my notes here, that I have a package that is incorporated into another, and when using 3.0.0-beta.1, TS reported that the typings file for typescript-fsa had formatting issues. Which didn't make a lot of sense (it looked fine), but may be part of this problem.

@xdave
Copy link
Author

xdave commented Apr 27, 2018

@suutari-ai Thanks... I haven't had the chance to check it yet, but I will see this weekend.

@xdave
Copy link
Author

xdave commented Apr 27, 2018

@rplotkin need to make sure your editor/project is using TypeScript 2.8 to parse/check. new typescript-fsa uses conditional types, which were not supported in earlier versions of Typescript.

@aikoven
Copy link
Owner

aikoven commented May 3, 2018

Thanks for your report @xdave!
I'm sorry for not answering for so long. I've just come back from a vacation, will get to it once I sort things out at work.

@aikoven
Copy link
Owner

aikoven commented May 16, 2018

@xdave Please try out 3.0.0-beta-2.

@xdave
Copy link
Author

xdave commented May 16, 2018

@aikoven Hmm, so this makes my test case above pass, but beta version of my package is still failing around the optional params, etc. I'll have to see if I can massage my code to handle optional in the same way as yours with extends void conditions.

Until then, I'm passing params around with ! after it (which coerces away the | undefined part). For now.

@aikoven
Copy link
Owner

aikoven commented May 31, 2018

@xdave Could you please post these other problems you get?

@xdave
Copy link
Author

xdave commented May 31, 2018

@aikoven For example:

export const bindThunkAction = <Params, Succ, Err, State, Extra = any>(
	actionCreators: AsyncActionCreators<Params, Succ, Err>,
	asyncWorker: AsyncWorker<Params, Succ, State, Extra>
): ThunkActionCreator<Params, Promise<Succ>, State, Extra> => params => async (
	dispatch,
	getState,
	extra
) => {
	try {
		dispatch(actionCreators.started(params));
		const result = await asyncWorker(params, dispatch, getState, extra);
		dispatch(actionCreators.done({ params, result }));
		return result;
	} catch (error) {
		dispatch(actionCreators.failed({ params, error }));
		throw error;
	}
};

Yields:

src/index.ts(40,35): error TS2345: Argument of type 'Params | undefined' is not assignable to parameter of type 'Params'.
  Type 'undefined' is not assignable to type 'Params'.
src/index.ts(41,36): error TS2345: Argument of type 'Params | undefined' is not assignable to parameter of type 'Params'.
  Type 'undefined' is not assignable to type 'Params'.
src/index.ts(42,32): error TS2345: Argument of type '{ params: Params | undefined; result: any; }' is not assignable to parameter of type 'Success<Params, Succ>'.
  Type '{ params: Params | undefined; result: any; }' is not assignable to type '(Params extends void ? { params?: Params | undefined; } : never) & (Succ extends void ? { result?...'.
    Type '{ params: Params | undefined; result: any; }' is not assignable to type 'Params extends void ? { params?: Params | undefined; } : never'.
src/index.ts(45,34): error TS2345: Argument of type '{ params: Params | undefined; error: any; }' is not assignable to parameter of type 'Failure<Params, Err>'.
  Type '{ params: Params | undefined; error: any; }' is not assignable to type '(Params extends void ? { params?: Params | undefined; } : never) & { error: Err; }'.
    Type '{ params: Params | undefined; error: any; }' is not assignable to type 'Params extends void ? { params?: Params | undefined; } : never'.

Because ThunkActionCreator is a function that has a single optional parameter:

export type ThunkActionCreator<Params, Result, State, Extra> =
	(params?: Params) => ThunkAction<Result, State, Extra, AnyAction>;

So, the type of params in bindThunkAction is Params | undefined.

@xdave
Copy link
Author

xdave commented May 31, 2018

@aikoven I realize that even though having nice optional params in my library is convenient, it's not type-safe because you can still call a thunk that does take params without them. I need to adjust the way I optionalize it in my code.

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

No branches or pull requests

4 participants