Skip to content
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

updating value with createLocalStorage inside createEffect causes "too much recursion" error #273

Closed
jhhom opened this issue Dec 18, 2022 · 1 comment

Comments

@jhhom
Copy link

jhhom commented Dec 18, 2022

I use createLocalStorage from @solid-primitives/storage and encounter too much recursion error in browser when I did the following:

import { createLocalStorage } from "@solid-primitives/storage";

const [localStore, setLocalStore] = createLocalStorage({ prefix: "test" });

createEffect(() => {
  if (localStore["token"] === undefined || localStore["token"] === "") {
    setLocalStore("token", "");
  }
});

Although I admit that there isn't much reason to create an effect like that, this is quite a simplified example, and I sometimes need to reset the state of the local storage under a certain condition, and that condition along with localStore["token" === ""] can be combined with other conditions with && or ||.

And I think it can sometimes be easy to overlook an effect like that?

I didn't expect that this could cause infinite loop or "too much recursion" as createSignal and createStore from solid-js don't seem to have this problem.

import { createSignal } from "solid-js";

const [token, setToken] = createSignal('');

createEffect(() => {
  if (token === "") {
    setToken("");
  }
});
import { createStore } from "solid-js/store";

const [state, setState] = createStore({
  value: "",
});

createEffect(() => {
  if (state["value"] === "") {
    setState("value", "");
  }
});

This could probably be fixed in user's code by creating a guard like:

createEffect(() => {
  if (someCustomCondition...) {
    if (!localStore["token"] === "") {
      setLocalStore("token", "");
    }
  }
});

But could this be something that is good to fix? Just to make it behaves the same as createStore or createSignal from solid-js, which doesn't causes infinite loop whe updating it inside an effect like that

@thetarnav
Copy link
Member

Signals shouldn't be synchronized in effects, and reading from a signal you are writing to is especially bad as it can lead to infinite loops, as you saw.
The general solution to ensuring that signals have certain values is to:

  1. create a custom setter callback that corrects the value before writing to the signal:
    const [localStore, setLocalStore] = createLocalStorage({ prefix: "test" });
    
    function setLocalStoreToken(value?: string) {
    	if (value === undefined) value = ""
    	setLocalStore("token", value)
    }
  2. correct the value in a derivation
    const [localStore, setLocalStore] = createLocalStorage({ prefix: "test" });
    
    const localStoreToken = () => localStore.token ?? ""

@thetarnav thetarnav closed this as not planned Won't fix, can't repro, duplicate, stale Aug 29, 2023
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

No branches or pull requests

2 participants