Skip to content

Match code to tutorial description / tests #310

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

Closed
wants to merge 1 commit into from

Conversation

vthommeret
Copy link

For toggleTodo reducer, the tutorial says:

For toggleTodo, the only value we need is the id of the todo being changed. We could have made that the payload, but I prefer always having payload be an object, so I made it action.payload.id instead.

But the code appears to just check the payload directly:

const todo = state.find(todo => todo.id === action.payload)

The tests also pass in the payload as an object, so they don't pass with the code in the tutorial:

{
  type: toggleTodo.type,
  payload: {
    id: 1
  }
}

For `toggleTodo` reducer, the tutorial says:

> For `toggleTodo`, the only value we need is the `id` of the todo being changed. We could have made that the `payload`, but I prefer always having `payload` be an object, so I made it `action.payload.id` instead.

But the code appears to just check the payload directly:

`const todo = state.find(todo => todo.id === action.payload)`

The tests also pass in the payload as an object, so they don't pass with the code in the tutorial:

```
{
  type: toggleTodo.type,
  payload: {
    id: 1
  }
}
```
@netlify
Copy link

netlify bot commented Jan 9, 2020

Deploy preview for redux-starter-kit-docs ready!

Built with commit 3c68070

https://deploy-preview-310--redux-starter-kit-docs.netlify.com

@markerikson
Copy link
Collaborator

Hmm. The current code in the actual example repo just checks action.payload. It's possible I mis-matched things somewhere along the way:

https://github.com/reduxjs/rtk-convert-todos-example/blob/138cc162b1cc9c64ab67fae0a1171d07940414e6/src/features/todos/todosSlice.js#L19

I'll have to look at this in more detail.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jan 9, 2020

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 3c68070:

Sandbox Source
muddy-bird-pky2n Configuration
affectionate-leaf-o57ks Configuration
rsk-github-issues-example Configuration

@vthommeret
Copy link
Author

Ah yeah, and it does look like the tests pass just the ID as the payload (I was pattern matching based on the example for addTodo) but looks like the payload guidance is out-of-sync with the actual code.

@markerikson
Copy link
Collaborator

Obsolete due to #905 .

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

Successfully merging this pull request may close these issues.

2 participants