0

I am unable to see what's wrong with this code, where I'm simply trying to update the object and get it refreshed in DOM, which doesn't happen. The console log does say that the object was updated, but why is this isn't updating my DOM?

import { render } from "solid-js/web";
import { createSignal, For } from "solid-js";

function ToDo() {
    const getToDos = () => {
        return [{
            id: 1, name: 'one'
        }, {
            id: 2, name: 'two'
        }]
    }

    const onTodoClick = (id: number) => {
        const td = [...todos()];
        const elem = td.find(t => t.id === id);
        elem!.name = elem!.name + ' - Done';
        console.log(td);
        console.log(todos());
        setTodos(() => td);
    }

    const [todos, setTodos] = createSignal(getToDos());

    return (
        <div>
            <For each={todos()} >
                {(todo) => <p onClick={() => onTodoClick(todo.id)}>{todo.name}</p>}
            </For>
        </div>
    );
}

render(() => <ToDo />, document.getElementById("app")!);

snippetkid
  • 282
  • 4
  • 16
  • Just use stores instead. Signals don't give you reactivity if you change internal properties of the array objects. A store on the other hand is setting Signals to every property and is fully reactive on any property no matter how deep down the object/array tree – Ilja KO Aug 28 '23 at 04:02

3 Answers3

0

The issue is the way in which you are updating the element. When you do

const elem = td.find(t => t.id === id);
elem!.name = elem!.name + ' - Done';

You are creating a new variable that references the item you want to update in the array. You then mutate the variable by updating the name property. That's why it shows up the console log. But you should be updating the array immutably. So something like

const td = todos().map(todo=>todo.id===id ? {...todo, name:todo.name+'done'} : todo)

You can even do this in the callback its more concise

 setTodos(todos=>todos.map(todo=>todo.id===id ? {...todo, name:todo.name+'done'} : todo));

If you're using typescript you can also use readonly types for objects and arrays so it will let you know when you mutate something.

Hope it helps

Max
  • 859
  • 5
  • 10
0

tl;dr, it's because const td [...todos()]; is just a shallow copy of todos(), which does work when creating something directly dependent on that array, but <For> depends on the internals of the array, so you need to make a copy of the object you're modifying instead of directly changing the .name attribute.

One way, similar to how you currently have your, that you can solve this is like so:

const onTodoClick = (id: number) => {
    const td = [...todos()];
    const elemIndex = td.findIndex(t => t.id === id);
    td[elemIndex] = { ...td[elemIndex], name: td[elemIndex].name + ' - Done' }
    console.log(td);
    console.log(todos());
    setTodos(() => td);
}

You can tell that it's not directly dependent on todos() itself because if you change the createSignal call to be const [todos, setTodos] = createSignal(getToDos(), { equals: false }); it will always count it as the todos changing whenever you call setTodos (docs).

If you check the source code of <For>, you can see that it, internally, uses createMemo(), which uses referential equality.

A more modern way you can do this is using Array.prototype.with.

const onTodoClick = (id: number) => {
    const elemIndex = todos().findIndex(t => t.id === id);
    const td = todos().with(elemIndex, {
      ...todos()[elemIndex],
      name: todos()[elemIndex].name + ' - Done'
    })
    console.log(td);
    console.log(todos());
    setTodos(() => td);
}
Samathingamajig
  • 11,839
  • 3
  • 12
  • 34
0

Signals uses === operator to check if a new value is set, only then calls the effects. Objects are compared by reference. If you mutate an object, change will not be perceived as an update by the Solid's runtime.

You satisfy this condition on the array:

const td = [...todos()];

But For component also rely on this feature since it takes the each value, maps over it and creates an array of signals. It uses these signals to update individual DOM element associated to an item.

For uses objects references as the key to cache DOM nodes, so that whey you pass the same value, it will return previously created DOM element otherwise create a new one. You can check this answer for a detailed explanation:

https://stackoverflow.com/a/72366316/7134134

Since you are mutating the array item, For returns the cached DOM element and UI remains untouched:

const elem = td.find(t => t.id === id);
elem!.name = elem!.name + ' - Done';

You should create a new todo object instead:

const onTodoClick = (id: number) => {
  setTodos(prev => prev.map(item => {
    if (item.id !== id) return item;
    return { ...item, done: true };
  }));
}

Here I used the callback form of the setter function for clarity. Also used done property to mark an item done. Using a separate property is better because now you can use different html tag, different styles for completed items using a simple conditional when rendering items.

snnsnn
  • 10,486
  • 4
  • 39
  • 44