Remove key when setting value to undefined

[franciscohanna92]

I'm expecting that a key I've setted with a value of undefined to be removed from the localStorage.

Give the following:

import useLocalStorage from "use-local-storage";

function App() {
  const [username, setUsername] = useLocalStorage("name", "");

  return (
    <>
      <input
        value={username}
        onChange={(e) => {
          setUsername(e.target.value);
        }}
      />
    <button onClick={() => setUsername(undefined)}>Clear localStorage</button>
    </>
  );
}

Once I click the "Clear localStorage" button, I expect the "name" key to be removed from the local storage. The observed result is that the name key gets the value undefined in the storage.

Here's a Codesandbox that reproduces this behavior.

[atifcppprogrammer]

Hi there @nas5w @franciscohanna92 ! I am a new contributor here would you like to assign this issue to me ? also i think it would be better if we addressed the above issue in the following manner

• Ensure that useLocalStorage hook returns a third function, with which we can clear the value set against the provided key using the example provided above

import useLocalStorage from "use-local-storage";

function App() {
  const [username, setUsername, unsetUsername] = useLocalStorage("name", "");

  return (
    <>
      <input
        value={username}
        onChange={(e) => {
          setUsername(e.target.value);
        }}
      />
      {/* removing the value set against the key "name" from localStorage */}
      <button onClick={() => unsetUsername()}>
        Clear localStorage
      </button>
    </>
  );
}

[nas5w]

@atifcppprogrammer that sounds like a good solution! Would love to see a PR for this (with tests + docs updated). Thank you!

[atifcppprogrammer]

@nas5w thanks a million 👍 I will work on this over the weekend 👨‍💻 , will get you that pull request next week 📆, sound good 🧐 ?

[atifcppprogrammer]

@nas5w so just so we are clear on this, if we are targeting the following implementation

import useLocalStorage from "use-local-storage";
function App() {
  const [username, setUsername, unsetUsername] = useLocalStorage("name", "");

  return (
    <>
      <input
        value={username}
        onChange={(e) => {
          setUsername(e.target.value);
        }}
      />
      {/* removing the value set against the key "username" from localStorage */}
      <button onClick={() => unsetUsername()}>
        Clear localStorage
      </button>
    </>
  );
}

will setUsername(undefined) preserve the current behavior i.e it will set value "undefined" against the key "name" in localStorage or should it remove the value for "name" from localStorage as @franciscohanna92 has requested in the original comment, i suppose we could respect the new implementation and the request made by @francisohanna92 where setUsername(undefined) is equivalent to unsetUsername(), however this would make the introduction of the third function somewhat redundant.

Thoughts ?

[nas5w]

@atifcppprogrammer let's just add the third function to the array as you first proposed. I don't want to change the current behavior of the state-setter because then there'd be no way to set the key to undefined if that was the desired outcome.

[franciscohanna92]

@nas5w

there'd be no way to set the key to undefined if that was the desired outcome

The outcome for setUsername(undefined) is the key to be set with the string "undefined" and not undefined as it should be expected. Anyway, it could always be accomplished by calling setUsername("undefined"), which would have a predictable outome.

At the same time, removing the key when setting it to undefined is a breaking change: I imagine there are some folks out there doing something like if (username === "undefined") given the current implementation.

[nas5w]

Thanks for chiming in @franciscohanna92!

@atifcppprogrammer since undefined is actually being stored as a string in local storage, I'm now starting to think we should go with what @franciscohanna92 initially suggested: if the user sets the item to undefined, the key is removed from local storage. Sorry to ask you to rework the PR you already have, but I think it's best to do it this way (plus it should be easier).

[atifcppprogrammer]

Thanks for chiming in @franciscohanna92!

@atifcppprogrammer since undefined is actually being stored as a string in local storage, I'm now starting to think we should go with what @franciscohanna92 initially suggested: if the user sets the item to undefined, the key is removed from local storage. Sorry to ask you to rework the PR you already have, but I think it's best to do it this way (plus it should be easier).

Hey @nas5w no problem, so given this new specification there won't be any need for the hook to provide the third remove function as well I imagine, also the current PR is already a bit of a mess is it okay if i close this and create a new PR or push the new changes on top or keep the existing PR and squash all previous commits into a single one.

[nas5w]

@atifcppprogrammer exactly. I think this will be a better solution

[atifcppprogrammer]

Hi @nas5w please find the PR for above discussed specification.

[atifcppprogrammer]

So @nas5w @franciscohanna92 does the current PR need any further changes ?

[nas5w]

@atifcppprogrammer will finish my review this weekend. Thank you!

[atifcppprogrammer]

Hi @nas5w noticed a small typo made the correction in the README.