toss / slash

A collection of TypeScript/JavaScript packages to build high-quality web services.
https://slash.page
MIT License
2.73k stars 302 forks source link

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

Open okinawaa opened 9 months ago

okinawaa commented 9 months ago

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 commented 4 months ago

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.