typescript-fsa icon indicating copy to clipboard operation
typescript-fsa copied to clipboard

Some breakage with new 3.0 beta

Open xdave opened this issue 6 years ago • 12 comments

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 avatar Apr 15 '18 08:04 xdave

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 avatar Apr 15 '18 19:04 xdave

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 avatar Apr 15 '18 20:04 xdave

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

suutari-ai avatar Apr 17 '18 11:04 suutari-ai

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.

rplotkin avatar Apr 27 '18 13:04 rplotkin

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

xdave avatar Apr 27 '18 23:04 xdave

@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.

xdave avatar Apr 27 '18 23:04 xdave

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 avatar May 03 '18 04:05 aikoven

@xdave Please try out 3.0.0-beta-2.

aikoven avatar May 16 '18 08:05 aikoven

@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.

xdave avatar May 16 '18 23:05 xdave

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

aikoven avatar May 31 '18 04:05 aikoven

@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 avatar May 31 '18 18:05 xdave

@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.

xdave avatar May 31 '18 18:05 xdave