-
-
Notifications
You must be signed in to change notification settings - Fork 99
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
New API - v2.0 #22
Comments
This all looks very useful to me. With regard to
The following one we also use quite often:
|
@flq thanks for feedback, could you also add what would be expected action shape from calling |
See, I never stop learning, I wasn't quite aware of said standard. In that context it'd probably have the same signature as |
yes |
Looking forward to using this new API. Just one thing, it maybe a stupid question, but can you please explain what |
@farzadmf as @flq mentioned above it stands for Flux Standard Action: https://github.com/redux-utilities/flux-standard-action, most redux related libraries are following it more or less |
@piotrwitek Do you think it would be useful to add a way to set the async suffix that is used? For example if I wanted the suffix the be Could make creating generic middleware for promisifying these action creators more sane. Let me know if I'm not making sense. |
@piotrwitek Not sure if I found a bug, or am just doing something wrong. Given the following code: setToken: buildAction(SessionActionType.SET_TOKEN).fsa<{
token: string
resolve?: () => void
reject?: (error: Error) => void
}>(
({ token }) => ({ token }),
({ resolve, reject }) => ({ resolve, reject })
) I am getting the following type errors in VSCode using TS2.8 |
I was able to get my use case to work with the following: setToken: buildAction(SessionActionType.SET_TOKEN).fsa<
{ token: string; resolve: () => void; reject: (error: Error) => void },
{ token: string },
{ resolve: () => void; reject: (error: Error) => void }
>(
({ token }) => ({ token: 'fgfg' }),
({ resolve, reject }) => ({ resolve, reject })
) Now I seem to be having some oddities around using the Any thoughts there? |
@duro fsa is WIP, for the moment it works differently like this: setToken: buildAction(SessionActionType.SET_TOKEN).fsa(
({ token }: { token: string }) => ({ token }),
({ token }) => ({ resolve: ..., reject: ... })
) But I plan to change it to using only the generic type for payload, and the rest will use inference |
@piotrwitek Did you see my last comment #22 (comment). I was able to get it working with three type args. That said, see the second half of the comment. New issue is with use of I'm no TS wiz, but perhaps for that case, we could have a named interface for the return of the |
Yes I saw it and working on it :) Trying to modify mapped with conditional type |
@duro Ok got it working :) const actions = {
empty: buildAction('INCREMENT').empty(),
payload: buildAction('ADD').payload<number>(),
fsa: buildAction('SHOW_NOTIFICATION').fsa((message: string) => message),
async: buildAction('GET_USER').async<number, { name: string }, string>(),
};
type RootAction = ActionsUnion<typeof actions>;
// { type: "INCREMENT"; } | { type: "ADD"; payload: number; } | { type: "SHOW_NOTIFICATION"; payload: string; } | { type: "GET_USER" & "REQUEST"; payload: number; } | { type: "GET_USER" & "SUCCESS"; payload: { name: string; }; } | { type: "GET_USER" & "FAILURE"; payload: string; } I'll do new release in a few minutes |
@piotrwitek Awesome! Can't wait to see it. I was fiddling with it over here, and I wonder how close I was to the right track. |
@duro I deployed a new version to npm |
@piotrwitek Works great! And I was getting close. Your library has taught me more about TS that the docs :) |
@piotrwitek buildAction doesn't seem to be returning the right shape for the action (in empty/payload/fsa) with the latest RC.2. |
@manvesh it works fine I tested. Please check in console with tsc compiler, you have probably selected default ts version in vscode, not from node_modules |
i like all your good work on this. 👍 I have one question: Why is used this syntax? const actions = {
empty: buildAction('INCREMENT').empty(),
payload: buildAction('ADD').payload<number>(),
fsa: buildAction('SHOW_NOTIFICATION').fsa((message: string) => message),
async: buildAction('GET_USER').async<number, { name: string }, string>(),
}; im asking, because it look little wierd to me create simple js object by calling 2 functions only to make ts happy ... PS: not sure if typescript can infer return type based on number of type parameters passes in - something like this: buildAction('FOO') // no type params -> buildAction('FOO').empty();
buildAction<T>('BAR') // single param with single argument -> buildAction('BAR').payload<T>();
buildAction<T>('FSA', pc, mc) // single param with 3 arguments -> buildAction('FSA').fsa<T>(pc,mc);
buildAction<R, S, F>('RSF') // three params -> buildAction('RSF').async<R,S,F>(); if this works i think it would be cleaner syntax (or at least less typing syntax :) ) |
@CzBuCHi I tried that already and I couldn't make it work, seems like it's not possible but I'm not 100% sure |
@piotrwitek ah okay ... i will try to experiment with it when i get some time and get result here ... |
@piotrwitek I was able to use actions that are not async. But as soon as I use an Errors on using async -
I'm not sure if I'm doing something wrong, or if |
@manvesh I think I don't have a type tests for this scenario yet, so yes it's WIP, I'll look to fix it tomorrow :) |
This new is looking really nice! Tried converting some of my project, ran into an issue with actionCreators that take a boolean: Given
I get this error from the callsite where I try to invoke
Is this expected to work, or have I misunderstood the API? |
I added a test here: https://github.com/runeh/typesafe-actions/tree/next . It fails when running |
@runeh thanks a lot for help!
|
@runeh the fix is already deployed to npm |
@piotrwitek Don't know what I did, but it seems to be working now. Thanks. |
@piotrwitek Should |
@Methuselah96 yes correct, is should be dependencies :) thanks EDIT: fix published as v2.0.0-1 |
Dear @piotrwitek , I would really appreciate if you can provide a "tutorial" for this new API (similar to the one you already have in the Github README for the current API). |
Playing around with this API and I overall I really like, although I'm a little stuck: is there a way to add logic Use case: Handling async API requests in a cross-cutting way. Currently, I have action creator functions that delegate to this utility method: interface ApiRequestAction<T extends string> {
readonly type: T;
readonly payload: { readonly url: string, readonly init?: RequestInit },
readonly meta: { readonly statusType: "REQUEST" | "RESPONSE" },
}
const createApiRequest = <T extends string>(
type: T,
url: string,
init?: RequestInit
): ApiRequestAction<T> => ({
meta: { statusType: "REQUEST" },
payload: { url, init },
type
}); For instance: const submitSearch: (values: SearchFormValues) = (
values: SearchFormValues
) =>
createApiRequest("SEARCH_API", "/api/search", {
body: JSON.stringify(values),
method: "POST"
}); Using redux-saga, I have a saga listening for actions with Do you have any suggestions for how to approach this with your api? The best I've come up with so far is writing an extra action creator factory function like this (untested, but you get the idea): interface RequestPayload {
readonly url: string,
readonly init?: RequestInit
}
const actions = {
search: buildAction("SEARCH_API").async<RequestPayload, Response, string>()
};
const createSearch = (values: SearchFormValues) =>
actions.search.request({
init: {
body: JSON.stringify(values),
method: "POST"
},
url: "/api/search"
}); |
I agree that the syntax is a bit ugly; I would much rather have a bunch of separate functions that I could call like The problem is that there are actually two generic type arguments for a function like Someone else reached the same conclusion to their problem (stated here) and explained their results in a little more detail here. In terms of function naming, I think my preference would still be to have several specialised top-level functions, so things could look more like:
But I realise that doesn't save very much, and might still be controversial. |
Hi @cjol, thanks for your summary and feedback! This is exactly the reason why I went with such API, moreover I was also considering switching to a specialized functions approach as you suggested (empty action API won't be necessary because you could do I was thinking to clean the new API to this form, which would handle all the use-cases from initial proposal: const noPayload = buildAction('NO_PAYLOAD').withType<void>(); // noPayload()
const createUser = buildAction('CREATE_USER').withType<PayloadType>();
const createUser = buildAction('CREATE_USER').withTypes<PayloadType, MetaType>();
const notify = buildAction('NOTIFY').withMappers<InputParameterType>( // output types are inferred
({ username, message }) => `${username}: ${message || ''}`, // payload mapper
({ timestamp }) => ({ timestamp }) // meta mapper
);
const fetchUser = buildAsyncAction(
'FETCH_USER_REQUEST',
'FETCH_USER_SUCCESS',
'FETCH_USER_FAILURE'
).withTypes<string, User, Error>();
const fetchUser = buildAsyncAction(
'FETCH_USER_REQUEST',
'FETCH_USER_SUCCESS',
'FETCH_USER_FAILURE'
).withMappers<string, User, Error>(
id => ({ id }), // request mapper
({ firstName, lastName }) => `${firstName} ${lastName}` // success mapper
({ message }) => message // error mapper
); PS: I saw TS team were discussing on a design meeting possibility to add a partially specified type arguments with inference, so when this feature arrives we will iterate and update API to the new and cleaner form (by removing withType chain call)(microsoft/TypeScript#22368 (comment)) |
100% chance I misunderstand a lot of stuff in this new release, but here is a problem I have ran into. Actions: export const actions = {
fetchModelFamily: buildAction("FetchModelFamily").async<string, ModelFamily, ResponseError>(),
//Here will come more later on
}; export type rootAction = ActionsUnion<typeof actions>; Reducer: export default function modelFamilyReducer(
state: ModelFamilyState = initialState,
action: rootAction
): ModelFamilyState {
switch (action.type) {
case getType(actions.fetchModelFamily.success):
return { ...state, family: action.payload};
default:
return state;
} The payload in this example is |
@piotrwitek Have you had a chance to release the fix you mentioned? Not trying to pester you about it, just wanted to check in case you forgot since you said later that day. |
@chasecaleb I got it, but I'm trying to complete an entire new test suite (which is bigger than the code base itself) to be sure everything is working smooth and I will be able to release a stable production ready v2.0. |
@piotrwitek How do I hook this up with
I can fix it by making the type of
How should I be typing this? |
@Methuselah96 i use this example as base. |
@kuzvac Yeah, I'm pretty sure I'm matching that base example (I've tried splitting out the |
@Methuselah96 i think yes, i use bindAction (but not async). Add actions file to Gist. |
Here's the updated component that more closely matches the base example:
Here's the actions file:
|
@piotrwitek Any thoughts as to how I can get this to work? |
Wait a bit more please, I'm preparing a https://codesandbox.io playground that will work as a documentation and cookbook for all redux patterns (later extended with epics, sagas, reselect etc.) I'll try to post the base today with a pre-release of v2.0 stable After that I'll focus on the issues and requests because API has been simplified a bit to overcome some type inference limitations and should fix all the issues found in the last experimental release. |
Just FYI, was experimenting with typesafe-actions 2.0.0-1 along with redux-observable 1.0.0-alpha.2 + rxjs 6.0.0 (lettable operators) and it seems to be working just fine! (I originally posted an error, but it was just user error... getting too late in the day for me). const changeTheme: Epic<RootAction, RootState> = (action$, state$) =>
action$.pipe(
filter(isActionOf(themeActions.changeTheme)),
withLatestFrom(state$),
mergeMap(([action, state]) =>
...
)
); |
@pachuka that's great to hear, thanks |
Hi @piotrwitek , great job with this, I'm a huge TS fan and must work also with React so I'm very gratefull with your guide and utilities. I have some like a bug or maybe I missunderstood something, I define an async action with export const recipeActions = {
fetchRecipes: buildAction(FETCH_RECIPES).async<
void,
APIResponse<Recipe[]>,
string
>()
};
export type RecipeAction = ActionsUnion<typeof recipeActions>; But then in the reducer have an error accesing to the payload, seems that when chain types with an OR expect that all types share the same params and since the export function recipeReducer(
state: RecipesState = RECIPES_DEFAULT_STATE,
action: RecipeAction
): RecipesState {
switch (action.type) {
case getType(recipeActions.fetchRecipes.success):
return action.payload.data; /* <-- payload property doesn't exist in EmptyAction type... */
default:
return state;
}
} |
Hi. I have next error: However i have used installing with next commands: edit: it's used |
@TotallWAR It has been renamed to |
Got it, @jeremejevs thank you! |
Road to v2.0: New API
I'm working on New API using the new language feature (conditional types) coming to TypesScript in the next release (v2.8)
I have already published working version that is compatible with recent typescript@rc release
> to install
yarn add typesafe-actions@next
Some changes are inspired by the suggestion from the past and I would really appreciate some feedback from their authors:
The implementation of a new API is taking place on the next branch, and will be released on the release of TypeScript v2.8.
Finished
ActionsUnion
async
action creator (check below)ReturnType
buildAction
import { buildAction } from 'typesafe-actions'
buildAction('INCREMENT').empty()
buildAction('INCREMENT').payload<P>()
buildAction('INCREMENT').async<R, S, F>()
buildAction('NOTIFY').fsa<P>(payloadCreator, metaCreator)
Breaking changes
🎉NO BREAKING CHANGES 🎉
Known Issues
ActionsUnion
not working withbuildAction().async()
#24Migration to 1.2.0
Here is an migration example of example in the docs:
The text was updated successfully, but these errors were encountered: