react redux poprawnośc kodu

Question

Nie jestem pewny czy ten kawałek kodu jest zgodny z dobrymi praktykami, proszę o radę, nie wiem czy powinno się modyfikować w ten sposób stan, ale nic innego nie przychodzi mi do głowy.

chodzi o case 'CHANGE_CHECK': nie potrafię inaczej zmodyfikować stanu, czy to jest poprawnie? kod działa, ale nie do końca podoba mi się sposób

Po kliknieciu zmeinia się stan

const initItem = {
    todos: [
        {
            title: 'Get out of bed',
            titleMarked: false,
            id: 1,
            detail: [
                { point: 'wake my wife upGG', pointMarked: false },
                { point: 'check the weatherGG', pointMarked: true },
                { point: 'check the weatherGGG', pointMarked: false }
            ]
        },
        {
            title: 'Walk the dog',
            titleMarked: false,
            id: 2,
            detail: [
                { point: 'wake my wife upWW', pointMarked: true },
                { point: 'check the weatherWW', pointMarked: false },
            ]
        }
    ],
    selectedItem: {
        id: 1
    }

}



const itemReducer = (state = initItem, action) => {

    switch (action.type) {
        case 'DELETE':
            let newItems = state.todos.filter(item => {
                return item.id != action.id
            })
            return {
                ...state,
                todos: newItems,
                selectedItem: newItems[0]
            }
        case 'DISPLAY_DETAIL':
            return {
                ...state,
                selectedItem: { id: action.id }

            }
        case 'ADD_NEW_TASK':
            return {
                ...state,
                todos: [...state.todos, action.newTask],
                selectedItem: { id: action.newTask.id },
            }
        case 'CHANGE_CHECK':
            const todos = state.todos.map(each => {
                const points = each.detail.map(each => {
                    if (action.id === each.point) {
                        each.pointMarked === false ? (each.pointMarked = true) : (each.pointMarked = false)
                    }
                })

            })
            return {
                ...state,

            }


    }
    return state
}

export default itemReducer

import React, { useEffect } from 'react'
import { connect } from 'react-redux'
import Points from './Points'
import alarm from './../assets/alarm.mp3'
import Play from './Play'

function Task(props) {




    const clickDelete = (id) => {
        props.deleteItem(id)
    }
    const handlePoint = (id) => {

        props.pointCheck(id)
    }
    const itemDetail = props.items.todos.filter(each => {
        return each.id === props.items.selectedItem.id
    })

    const [details] = itemDetail
    if (itemDetail.length < 1) {
        return (
            <div className='title'>
                <div className='title-wrapper'>
                    <p>You have no todos left</p>
                </div>
            </div>
        )
    }
    const alldetails = details.detail.map(each => {
        const pointcheck = each.pointMarked === true ?
            (<input onClick={(id) => { handlePoint(each.point) }} className='point-check' id={each.point} type="checkbox" checked />) :
            (<input onClick={(id) => { handlePoint(each.point) }} className='point-check' id={each.point} type="checkbox" />)
        return (
            <div>
                {pointcheck}
                <label className='point-label' htmlFor={each.point}>{each.point}</label>
            </div>
        )
    })
    return (
        <>
            <div className='task-container'>
                <div className='title'>
                    <div className='title-wrapper'>
                        <input className='title-check' type="checkbox" id="todo" name="todo" value="todo" />
                        <label className='title-label' htmlFor="todo" data-content={details.title}>{details.title}</label>
                    </div>
                    <i onClick={() => { clickDelete(details.id) }} className="fas fa-trash-alt"></i>
                </div>
                {alldetails}
            </div>

        </>
    )
}
const mapStateToProps = (state) => {
    return {
        items: state.item
    }
}
const mapDispatchToProps = (dispatch) => {
    return {
        deleteItem: (id) => { dispatch({ type: 'DELETE', id: id }) },
        pointCheck: (id) => { dispatch({ type: 'CHANGE_CHECK', id: id }) }
    }
}
export default connect(mapStateToProps, mapDispatchToProps)(Task)

proszę o pomoc, radę

Comments

A mógłbyś opisać co ten kod ma robić?

---

Chodzi o to, że po kliknieciu i uruchomieniu funkcji handlePoint jest uruchomiany dispatch 'CHANGE_CHECK' który zmienia stan w odpowiednim podpunkcie i  zmienia się stan tego podpunktu, wszystko działa ale gdzieś się dogrzebałem, ze nie powinno się zmieniać stanu bezpośrenio a ja to robię tak

if (action.id === each.point) {
                        each.pointMarked === false ? (each.pointMarked = true) : (each.pointMarked = false)
                    }

czy coś źle doczytałem i to jest dobrze czy nie za bardzo?  można tak zmienić stan?

each.pointMarked = true

jak masz jeszcze inne uwagi chętnie wysłucham

Answer 1

Tak jak napisałeś, nie reducery nie powinny modyfikować stanu, jedynie zwracać nowy stan na podstawie poprzedniego.

Możesz to poprawić w taki sposób(nie przetestowałem więc możliwe że coś trzeba poprawić, ale ogólnie ideę prezentuje):

const todos = state.todos.map((todo) => {
  return {...todo, points: todo.detail.map((detail) => {
      if (action.id !== detail.point) return detail;
      return {...detail, pointedMarked: !detail.pointMarked}
  })}
})
return {
  ...state,
  todos
};

 

Comments

dzięki bardzo, z dwoma drobnymi poprawkami kod śmiga, na przyszłość będę wiedział jak