So I am new to javascript and I tried making a todo list. This works well with adding elements. The issue is when I am removing some item, the first one gets removed too, why is it so? I know I am missing a small thing and this may be really basic but I am not able to find out what that is.

const App1 = () => {
    const [item, updatedItem]=useState('');
    const [Items, setItems]=useState([]);
    function inputEvent(event) {
        updatedItem(event.target.value);
    }
    const addItem = (event) => {
        event.preventDefault();
        setItems((prev) => {
            return[
                ...prev,
                item
        ]
        });
        updatedItem('');    
    }
    
    let key=0;
    return(<>
    <div className='back'>
        <div className='list'>
            <header>ToDo List</header>
                <form onSubmit={addItem}>
                    <input type='text' placeholder='Add an item' value={item} onChange={inputEvent}/>
                    <button type='submit'> </button>
                </form>
                    <div className='items'>
                        <ol> 
                            {Items.map((val) => <li><button id={key  } onClick={(event) => {
                                setItems((Items) => {
                                    return Items.filter((val, index) => {
                                        if(index!==Number(event.target.id)){ 
                                            return index;
                                        }
                                    }
                                );
                            });
                        key=0;
                    }}>x</button>{val}</li>)}
                </ol>
            </div>    
        </div>
    </div>
</>);

CodePudding user response：

You return the index in your filter, expecting this to always be true, yet 0 (the index of the first element) is a falsy value.

Try this instead:

return Items.filter((_val, index) => index !== Number(event.target.id));

Some unrelated code-quality notes:

1. In React, you should always set a key prop on each element when looping through them, rather than id.
2. map has a second argument, index, which it passes into the callback --- you don't have to keep track of this yourself with e.g. key etc.
3. If you use map's index parameter, then you can pass that directly into your filter rather than using Number(event.target.id), which is not very idiomatic in React.
4. If you don't use an argument of a callback, it's a good idea to prefix it with a _ (like I've done with _val here), to make it explicit that you're not using it.

CodePudding user response：

Your filter callback should return a flag. index is a number. When treated as a flag, 0 is false (more on MDN). Instead:

return Items.filter((val, index) => index !== Number(event.target.id));

However, your code is returning an array of li elements without setting key on them (see: keys), which React needs in order to manage that list properly (you should be seeing a warning about it in devtools if you're using the development version of the libs, which is best in development). You can't use the mechanism you're using now for keys when doing that, it will not work reliably (see this article linked by the React documentation). Instead, assign each Todo item a unique ID when you create it that doesn't change, and use that as the key (and as the value to look for when removing the item):

// Outside the component:
let lastId = 0;

// Inside the component:
const addItem = (event) => {
    event.preventDefault();
    setItems((prev) => {
        return [
            ...prev,
            {text: item, id:   lastId}
        ];
    });
    updatedItem("");    
};

// Add a remove function:
const removeItem = ({currentTarget: {id}}) => {
    setItems(items => items.filter(item => item.id !== id));
};

// When rendering:
{Items.map((item) => <li key={item.id}><button onClick={removeItem}>x</button>{item.text}</li>)}

In some cases it may be useful to use useCallback to memoize removeItem to avoid unnecessary rendering, but often that's overkill.