-
-
Notifications
You must be signed in to change notification settings - Fork 15.2k
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
[Docs] Rework "Usage with TypeScript" page #4025
Comments
Hey Mark, we discussed me possibly contributing here via Twitter. Here's some thoughts:
Random Notes:
|
@ricokahler I really do want to drop the "union of action types" thing. Per Lenz's article, it doesn't have any real benefit, and it's even more useless when you're writing code with RTK. Agreed that we should rename the React-Redux TS page. I originally had thoughts of getting some Flow examples in there, but Flow is dead at this point. Also agreed that we really need to emphasize how to type dispatch for use with thunks correctly. Not sure I'm keen on the idea of augmented types, at least for now. I think Lenz has some opinions on that, though. Paging @phryneas and @msutkowski for their thoughts here. Tell you what, why don't you go ahead and try to put up a PR so we've got some concrete changes for discussion. |
I'm conflicted about it. @phryneas's article makes a lot of sense (and is great!) but the solution presented requires more code and loses the idiomatic redux that we've known for years. I actually do think it's possible to tell typescript that our action could potentially contain more than just the given union. Here's what I came up with: const initialState = { messages: [] }
export const SEND_MESSAGE = 'SEND_MESSAGE'
export const DELETE_MESSAGE = 'DELETE_MESSAGE'
interface ChatState { }
interface SendMessageAction { type: typeof SEND_MESSAGE, messages: string[]; }
interface DeleteMessageAction { type: typeof DELETE_MESSAGE }
type Actions = SendMessageAction | DeleteMessageAction;
type AnyAction = { type: any }
type OtherActions<T extends AnyAction> = { type: T extends T['type'] ? T : { type: unknown } }
export function chatReducer(
state = initialState,
action: Actions | OtherActions<Actions>
): ChatState {
switch (action.type) {
case SEND_MESSAGE:
return {
messages: [...state.messages, action.messages]
}
case DELETE_MESSAGE:
return {
messages: state.messages // etc etc
}
default:
return state
}
} The trick here is Its type parameter accepts the union of all action types and uses a conditional type to assert that if the type doesn't conform, return I think there's some value in matching the switch style for the sake of matching all the historical material (or even just the rest of the docs) that exists out there. E.g. if someone new is coming to redux, most learning material will probably show the switch stuff. I think the practical answer we can allude to would be "just use redux toolkit" but for the sake of the docs, we should show a type example as close to historical redux as possible. I really think discriminated unions are the best mechanism for this.
I like the idea of type augments because it meets people where they're at. What's nice about it is that it brings the typescript usage closer to the non-typed usage (and vice versa e.g. javascript users can get type hints). In general, I try to create type solutions that don't require any runtime support (as you can see from my previous recommendation). I do this for two reasons:
Let me know what you think! I'd like to write the docs using the |
Hmm. Let's get the augmentation thing out of the way first. I think for state it's amazing if you look at it in separation. And enormously dangerous if you look at it in real life. Pretty much the same goes with augmenting the Dispatch type. While I would totally love to see that augmented dispatch type in the redux-thunk types get released and would enthusiastically use it in my own projects, it is a fact that that type augmentation would always be in place - no matter if the middleware is only installed (e.g. as a dependency of RTK) or actually instantiated (it can be disabled after all). Getting back from that to the actual concern of rewriting the "static typing" page: I'm not sure we should actually add any new information on that. My personal approach would honestly be to replace the whole "static typing" page with a section that emphazises that we really really really want people to use RTK. And then add a link to the github archive of that page so that people who need TS in a legacy project still can read up on it. But really just in the raw markdown form, to really empathize that this is part of the history of Redux and not how we suggest it to be done nowadays. This might seem a bit drastic but I'm honestly getting frustrated by the amount of people ignoring all the blinking signs and doing themselves the disservice of learning only vanilla redux. |
That's fair. I could imagine ways augments could mess with more than you'd sign up for. Maybe something like first-party type helpers could help with that but I don't feel too strongly about this, was just an idea.
Sometimes simplicity is the best answer haha. I actually like that but at this point I'm not sure if there's too much additive work here I can contribute. It looks like all that's needed is to move the docs to markdown files and point them to RTK. Deleting the current docs kinda solves everything haha |
I noted the other day that the "Usage with TS" page is actually one of the top 10 pages hits-wise over the last few months: https://twitter.com/acemarke/status/1372591748367720451 so there's definitely value in keeping it, and also in pointing people to how we want them to use RTK as a default approach. Given that, I'm going to go ahead and try updating the TS page myself right now. @ricokahler , I really appreciate your enthusiasm and interest in helping out. Are there any other open issues or tasks you'd be interested in tackling? I'd definitely still love to have you involved here. |
Fixed by #4042 . |
The current "Usage with TypeScript" page has multiple issues, and we should rework it. When that page was written in 2018-ish, I still didn't know TS yet. So, I couldn't offer any meaningful feedback on the contents. Now that I actually have a decent TS background, I can see that there's problems:
Dispatch
type to allow correctly dispatching thunks. (This is by far one of the most common questions I see lately, and I'm not sure how to easily address it through code, so we need to highlight the importance of this for thunk usage.)Reducer
typeand I'm sure there's some other improvements we could make as well.
The recently updated "TS Quick Start" page in the RTK docs would be a good starting point for "here's what to do":
https://redux-toolkit.js.org/tutorials/typescript
and then we can give more details for specific situations after that.
The text was updated successfully, but these errors were encountered: