Add useLocalStorage hook

[thomaslombart]

This pull request introduces the useLocalStorage hook, which provides a convenient way to manage values stored in the local storage. The hook internally uses useCachedPromise to cache the value and provide a loading state.

Here's the signature:

function useLocalStorage<T>(key: string, initialValue?: T): {
  value: T | undefined;
  setValue: (value: T) => Promise<void>;
  removeValue: () => Promise<void>;
  isLoading: boolean;
}

And an example:

import { Action, ActionPanel, Color, Icon, List } from "@raycast/api";
import { useLocalStorage } from "@raycast/utils";

const exampleTodos = [
  { id: "1", title: "Buy milk", done: false },
  { id: "2", title: "Walk the dog", done: false },
  { id: "3", title: "Call mom", done: false },
];

export default function Command() {
  const { value: todos, setValue: setTodos, isLoading } = useLocalStorage("todos", exampleTodos);

  async function toggleTodo(id: string) {
    const newTodos = todos?.map((todo) => (todo.id === id ? { ...todo, done: !todo.done } : todo)) ?? [];
    await setTodos(newTodos);
  }

  return (
    <List isLoading={isLoading}>
      {todos?.map((todo) => (
        <List.Item
          icon={todo.done ? { source: Icon.Checkmark, tintColor: Color.Green } : Icon.Circle}
          key={todo.id}
          title={todo.title}
          actions={
            <ActionPanel>
              <Action title={todo.done ? "Uncomplete" : "Complete"} onAction={() => toggleTodo(todo.id)} />
              <Action title="Delete" style={Action.Style.Destructive} onAction={() => toggleTodo(todo.id)} />
            </ActionPanel>
          }
        />
      ))}
    </List>
  );
}

Screencast

CleanShot.2024-03-29.at.11.56.14.mp4

[mathieudutour]

don't you need to handle undefined here?

[thomaslombart]

You mean, that setItem shouldn't be called if value is undefined?

[mathieudutour]

no, but it should probably remove the value, no?

[mathieudutour]

I'm wondering if the hook should be called useStoredState to make the parallel with useCachedState and useState.

Additionally, I don't know if we really need removeValue 🤔

[thomaslombart]

I feel like useStoredState doesn't directly convey compared to useLocalStorage that's widely known amongst developers.

removeValue could be useful in case a user wants to reset something or similar. They could use LocalStorage.removeItem(key) instead but it's more convenient to get it from the hook, especially since it's calling mutate which will make sure the value is properly updated.

[mathieudutour]

I feel like useStoredState doesn't directly convey compared to useLocalStorage that's widely known amongst developers.

Convey what? Is useLocalStorage really known? It's not part of react, nor react-dom so... I feel like the fact that it uses the local storage is an implementation detail

[thomaslombart]

I forgot some words 😄 I meant, convey its meaning. react-use has a useLocalStorage hook as well and while Local Storage is indeed an implementation details, it's widely known in web developments so they know Local Storage = storing a value that will be remembered. I feel like useStoredState is blurrier in that way.

[mathieudutour]

Any reason to use the initialValue here instead of passing it to the useCachedState?

Thinking about it more, I think that when the cache is wiped (which can happen at any time) the initialValue will flicker even if there's something on the localStorage.

I'm wondering if we actually want the useCachedPromise and the initialValue 🤔

[thomaslombart]

Did you mean useCachedPromise instead of useCachedState? The reason is to avoid having undefined returned even if the user passed an initial value. What would you suggest instead?

[mathieudutour]

yes sorry.

The reason is to avoid having undefined returned even if the user passed an initial value.

I don't understand, useCachedPromise should do that already.

But yeah I don't think caching the value and returning the initial value if there is no cache is what we want. I understand that it would allow you to get the previous value (maybe, sometimes) without having to wait but it will lead to flickers

[thomaslombart]

I don't understand, useCachedPromise should do that already.

I don't know if that's intended or not, but even if I passed the initialData option to useCachedPromise, the value can still be undefined for a brief moment. I'd expect it to never be undefined if there's an initial value 🤔

So should we go with usePromise then?

[mathieudutour]

I don't know if that's intended or not, but even if I passed the initialData option to useCachedPromise, the value can still be undefined for a brief moment

Hum that's a bug then. Do you have a repro?

So should we go with usePromise then?

I'd say so yeah

[thomaslombart]

But, won't we get flickers as well with usePromise? If an initial value is passed, the data will always load and we can get flickers as well if the value in the local storage is different than the initial one. Here's an example where the initial value is a list of three to-dos:

CleanShot.2024-04-18.at.12.08.27.mp4

Unless we should get rid of initialValue as well?

[mathieudutour]

I'm wondering if we actually want the useCachedPromise and the initialValue

That's what I said yeah, I don't think we need either

[thomaslombart]

Also thinking about it, shouldn't we clear the local storage in the onError callback if it fails getting the data?

[mathieudutour]

Hum I'm not too sure. It might be a temporary error or something, no? At worse, the value will get overwritten and that'd be it, no?

[mathieudutour]

shall this be typeof item !=='undefined? What happens if you store ""?

[mathieudutour]

I had another thought: I'm wondering if we could support a setTodos(todos => ...) signature for the setter (similar to the setter of useState). This could some async check 🤔
But I guess it can come in a follow up PR

[thomaslombart]

Yeah, I'd keep it simple for now. We can always iterate on it later since it won't be a breaking change.