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

useLocalStorage regression in 7.6.1 #5848

Closed
1 of 2 tasks
dfaust opened this issue Mar 1, 2024 · 7 comments
Closed
1 of 2 tasks

useLocalStorage regression in 7.6.1 #5848

dfaust opened this issue Mar 1, 2024 · 7 comments
Labels
Fixed patch Completed issues that will be published with next patch (1.0.X)

Comments

@dfaust
Copy link

dfaust commented Mar 1, 2024

Dependencies check up

  • I have verified that I use latest version of all @mantine/* packages

What version of @mantine/* packages do you have in package.json?

7.6.1

What package has an issue?

@mantine/hooks

What framework do you use?

Vite

In which browsers you can reproduce the issue?

Firefox

Describe the bug

useLocalStorage stores 'undefined' while loading a value.

  const [settings] = useLocalStorage<Settings>({
    key: 'appSettings',
    getInitialValueInEffect: false,
  });

Reverting @mantine/hooks to 7.6.0 fixes the problem:

  "dependencies": {
    "@mantine/hooks": "=7.6.0"
  }

I assume this is related to #5796.

If possible, include a link to a codesandbox with a minimal reproduction

No response

Possible fix

No response

Self-service

  • I would be willing to implement a fix for this issue
@lgaspari
Copy link
Contributor

Hey @dfaust, how are you doing? Do you think you could provide a defaultValue in this case? That way, you'll:

  • Get the defaultValue as settings (instead of undefined)
  • Store the defaultValue on Local Storage under the appSettings key

Otherwise, what are you trying to achieve? A codesandbox with a minimal reproduction should help too. Thanks!

@dfaust
Copy link
Author

dfaust commented Mar 11, 2024

Hi @lgaspari,

I don't want to have a default value. I want useLocalStorage to return undefined, if the value doesn't exist.

If useLocalStorage is called and the value doesn't exist, it correctly returns undefined. But it also writes a string "undefined" to the local storage.
The next time useLocalStorage is called, it returns this "undefined" string.

image

@lgaspari
Copy link
Contributor

Gotcha, now I understand why this is a regression for you, @dfaust.

It might be related to a concern I raised on the linked issue (see #5796 (comment)) where you ALWAYS get the default value on return, therefore, this side-effect isn't called. In other words, even though my issue was solved, I strongly believe that there are other use cases that aren't considered and might be broken as your case.

I'll give it a try on my end to see if I can help solve your issue while keeping it working for me as well 😄

@wassim
Copy link

wassim commented Mar 12, 2024

I have the same use case as @dfaust I need the useLocalStorage to return null or undefined, not "undefined" as text.

@lgaspari
Copy link
Contributor

@dfaust @wassim I've created a fork repository and addressed the issue in there. I'd appreciate if you could give it a try. In order to do so, please follow these steps:

  1. Clone the fork repository
  2. Install dependencies by using yarn
  3. Run storybook by using yarn storybook
  4. Find use-local-storage stories (e.g. http://localhost:6006/?path=/story/use-local-storage--no-default-value)

I've covered only 8 scenarios which look similar but they work differently as the following (not in the same order as stories):

Type Default value Storage value Returned value
No SSR No No undefined
No SSR No Yes ('123') '123'
No SSR Yes ('123') No '123'
No SSR Yes ('123') Yes ('654') '654'
SSR No No undefined
SSR No Yes ('456') '456'
SSR Yes ('456') No '456'
SSR Yes ('456') Yes ('321') '321'

I'll wait for your feedback before creating a pull request :) — if in any case you cannot try it, please let me know.

rtivital added a commit that referenced this issue Mar 12, 2024
…the local storage when `defaultValue` is not defined (#5848)
@rtivital rtivital added the Fixed patch Completed issues that will be published with next patch (1.0.X) label Mar 12, 2024
@rtivital
Copy link
Member

Fixed in 7.6.2

@dfaust
Copy link
Author

dfaust commented Mar 12, 2024

@lgaspari: I didn't try your fix, but @rtivital's fix in 7.6.2 works.

Thanks everybody!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fixed patch Completed issues that will be published with next patch (1.0.X)
Projects
None yet
Development

No branches or pull requests

4 participants