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

[BUG]: @toss/react default values setting of useStorageState #421

Open
okinawaa opened this issue Jan 24, 2024 · 1 comment
Open

[BUG]: @toss/react default values setting of useStorageState #421

okinawaa opened this issue Jan 24, 2024 · 1 comment

Comments

@okinawaa
Copy link
Member

okinawaa commented Jan 24, 2024

Package Scope

Package name: @toss/react

Describe the bug

If a value is initialized through defaultValue, the value is not set in the session storage.

Expected behavior

Even if the set function is not executed, if the value is initialized through defaultValue, the value must be set in the session storage.

To Reproduce

const [state, set] = useStorageState('dummy-key', { defaultValue: 'foo'}); // it doesn't stored in session storage until we call set funciton

Possible Solution

const getValue = useCallback(<T>() => {
    const data = storage.get(key);

    if (data == null) {
      return defaultValue;
    }

    try {
      const result = JSON.parse(data);

      if (result == null) {
        storage.set(key, JSON.stringify(defaultValue)); // add this line
        return defaultValue;
      }

      return result as T;
    } catch {
      // NOTE: JSON 객체가 아닌 경우
      return defaultValue;
    }
  }, [defaultValue, key, storage]);

Additional context

@sa02045
Copy link
Contributor

sa02045 commented Jun 4, 2024

It works to add code to this line rather than add code to the line you mentioned.

const getValue = useCallback(<T>() => {
    const data = storage.get(key);

    if (data == null) {
      storage.set(key, JSON.stringify(defaultValue)); // add this line

      return defaultValue;
    }

   try {
      const result = JSON.parse(data);

// ...
// ... 

But I think we should avoid setting the value inside the getValue function (because it's a side effect)

I think we need a logic to set defaultValue to storage right after component mount, we need to useEffect because this is synchronizing with external system.

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

Successfully merging a pull request may close this issue.

2 participants