SolidJS: createEffect triggers infinite loop when updating store values-CodePudding

I'm trying to make a to-do list app with daily streak, so every time you check all of your todos in a day, your streak increments. I'm using Solid with Typescript. I'm making a Streak component that receives from App by props the store signal state and setState defined like this (the Todo is a type with just a string text and a bool complete:

const [state, setState] = createStore<{days: number; todos: Todo[]}> ({
  days: 0,
  todos: todos(),
})

Also in App I made an effect that looks if the checked todos have the same length as the complete todo list, so the days variable can increment:

createEffect(() => {
  let newTodos = todos();
  let getOneMoreDay: number = 0;
  if (newTodos.filter((x) => x.complete === true).length === newTodos.length)
    getOneMoreDay = 1;
  setState({
    days: state.days   getOneMoreDay,
    todos: newTodos,
  });
});

The problem is. In the page, when I check all the todos, the Streak component starts a loop and instead of show me 1 day it shows 8492 days with the following message in console: Uncaught InternalError: too much recursion. So, what am I doing wrong? Should I use another functions instead of createEffect? I tried createMemo, but what happens is that days is not even updated. Ask me for any details that could help you and I'm sorry for the long question. I'm very new to web dev.

CodePudding user response：

Because you are setting state in effect which causes infinite loop. Effects are for handling side effects, you should not use them to update states. If you are going to do it anyway, make sure you run partial state update which does not trigger infinite loop, or use on method to listen to certain properties while updating other ones.

From your code I can see you are storing signals in your store:

({
  days: 0,
  todos: todos(), // ← This is bad!
})

Store itsef is reactive and can store multiple items without needing additional signals. That is the whole point of having a store.

To answer your question, there are multiple ways to do it. If you are going to use an effect, check if all todos are marked done and update only days value:

import { createEffect, For } from "solid-js";
import { render } from 'solid-js/web';
import { createStore } from 'solid-js/store';

interface Todo {
  title: string;
  done: boolean;
}

interface Store {
  days: number,
  todos: Array<Todo>
}

export const App = () => {
  const [store, setStore] = createStore<Store>({ days: 0, todos: [] });

  const addTodo = () => {
    const id = store.todos.length;
    setStore('todos', id, { title: 'Todo number '   id, done: false });
  };

  const markDone = (index: number) => () => {
    setStore('todos', index, todo => ({ ...todo, done: true }));
  };

  createEffect(() => {
    setStore('days', val => store.todos.length > 0 && store.todos.every(item => item.done) ? val   1 : val);
  });

  return (
    <div>
      <div onClick={addTodo}>Add Todo</div>
      <For each={store.todos}>
        {(item, index) => {
          return (
            <li onClick={markDone(index())}><input type="checkbox" checked={item.done} /> {item.title}</li>
          );
        }}
      </For>
      <div>{store.days}</div>
    </div>
  );
}
render(() => <App />, document.body);

You can find live demo at: https://playground.solidjs.com/anonymous/bea60daa-cf72-4880-82e4-ccdac72fbd8b

Alternative and better way would be updating days when you mark the last todo item done:

interface Todo {
  title: string;
  done: boolean;
}

interface Store {
  days: number,
  todos: Array<Todo>
}

export const App = () => {
  const [store, setStore] = createStore<Store>({ days: 0, todos: [] });

  const addTodo = () => {
    const id = store.todos.length;
    setStore('todos', id, { title: 'Todo number '   id, done: false });
  };

  const markDone = (index: number) => () => {
    setStore(prev => {
      const newTodos = prev.todos.map((item, i) => i === index ? { ...item, done: true } : item);
      const newDays = newTodos.every(item => item.done) ? prev.days   1 : prev.days;
      return { days: newDays, todos: newTodos };
    })
  };

  return (
    <div>
      <div onClick={addTodo}>Add Todo</div>
      <For each={store.todos}>
        {(item, index) => {
          return (
            <li onClick={markDone(index())}><input type="checkbox" checked={item.done} /> {item.title}</li>
          );
        }}
      </For>
      <div>{store.days}</div>
    </div>
  );
}

Here is a live demo: https://playground.solidjs.com/anonymous/927300e2-d801-4e48-b461-4e29527f5788

You can make your effect immune to infinite loops by listening on a property that is not updated by the inner callback:

createEffect(on(() => store.todos, () => {
  // Effects will run only when state.todos update
  setStore('days', val => val   1);
}));

Another alternative would be memoizing todos so that the effect is not triggered for other properties:

createEffect(createMemo(() => store.todos, () => {
  // Effects will run only when state.todos update
  setStore('days', val => val   1);
}));