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

[Docs] Sample when intruducing Redux Toolkit may cause confusion #4718

Closed
muyangren918 opened this issue Jul 16, 2024 · 4 comments
Closed

[Docs] Sample when intruducing Redux Toolkit may cause confusion #4718

muyangren918 opened this issue Jul 16, 2024 · 4 comments

Comments

@muyangren918
Copy link
Contributor

What docs page needs to be fixed?

What is the problem?

The todoToggled function before using Redux Toolkit accepts id as parameter and returns an object inside which payload property is also an object. The object has id as the only property and the id parameter as its value. So within reducer, payload.id is used when comparing with todo.id.
However, I guess the todoToggled function after using Redux Toolkit(The auto generated one) differs so this time payload itself is used when comparing with todo.id. It seems that the auto generated one's behavior is that the parameter is just assigned to payload, which means if you call generated todoToggled function like todoToggled(123) then you get {type: TODO_TOGGLED, payload: 123} as action, and if todoToggled({id: 123}) then you get {type: TODO_TOGGLED, payload: {id: 123}}, without any magic.

A few days ago, a friend asked me why the latter is using payload itself to compare with todo.id. "It's hard to beleive that the offical site sometimes alse make mistake." She told. I aggred with her at the beginning and finally I realized that the sample after using Redux Toolkit may suppose actual function calling is todoToggled(123). We are both foreign beginners to Redux. I think there may be an confusing sample design to beginners on this page. I suggest the sample before using Redux Toolkit should be modified to make sure the right place is only focused on. Besides, when toggling todo, the id paratemer is only needed.

What should be changed to fix the problem?

Change the todoToggled function to return an object of {type: TODO_TOGGLED, payload: id} and change the according reducer to todo.id !== action.payload when comparing todos.

@timdorr
Copy link
Member

timdorr commented Jul 17, 2024

We don't want to imply that the payload has to be a simple, scalar type. It can be more complex, so that you can pass multiple values over to your reducer. The action creator accepts a simple input as a convenience, but also so you can self-document the code with the parameter names and types. When you start to add TypeScript, this becomes a lot more obvious.

@timdorr timdorr closed this as not planned Won't fix, can't repro, duplicate, stale Jul 17, 2024
@markerikson markerikson reopened this Jul 17, 2024
@markerikson
Copy link
Contributor

Eh, I think the original complaint here is just that there's a mismatch in logic and usage between the "handwritten" snippet and the "RTK" snippet, which makes it feel like they're doing something different. Ultimately they both compare vs the ID, it's whether the ID is the payload or is in a nested field.

Simplest tweak would be to remove the curly braces and the .id from the handwritten example.

@timdorr
Copy link
Member

timdorr commented Jul 17, 2024

Oh, I didn't scroll down the page far enough... 🤦‍♂️

Yeah, that should be synced up. @muyangren918 Can you make a PR for this?

@muyangren918
Copy link
Contributor Author

Thank you for quick response. I made a PR.

@timdorr timdorr closed this as completed Jul 18, 2024
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

3 participants