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

useSessionStorage improvements #276

Closed

Conversation

sskirby
Copy link

@sskirby sskirby commented Jan 9, 2023

This PR has 3 changes for the useSessionStorage hook:

  1. It allows an initial value function to be used without provoking TS errors.
  2. It eliminates changes to the hook state when another instance of the hook with a different storage key is updated.
  3. It ensures that the value set to the hook is referentially equal to the value reported by the hook.

I love the library! Please let me know how I can improve the PR for your approval!

@sskirby sskirby force-pushed the useSessionStorage-improvements branch from ada583f to 1266f5b Compare January 10, 2023 19:32
}
}, [initialValue, key])

// State to store our value
// Pass initial state function to useState so logic is only executed once
const [storedValue, setStoredValue] = useState<T>(readValue)
// create a new empty object to keep track of the identity of the hook
const [source] = useState({})
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@juliencrn do you think it worthwhile to extract the creation of the empty object into a function so that it isn't created on each render?

@sskirby sskirby force-pushed the useSessionStorage-improvements branch from 1266f5b to 3b72152 Compare January 11, 2023 16:08
Comment on lines +26 to +27
const getInitialValue = () =>
initialValue instanceof Function ? initialValue() : initialValue
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@juliencrn or maybe memo-ize the initialValue so that we guarantee it only ever gets evaluated once?

@juliencrn
Copy link
Owner

Hi @sskirby, thanks for the PR. I will unfortunately close it because most of the changes already have been addressed now, see #435, #436 and #437

@juliencrn juliencrn closed this Jan 23, 2024
juliencrn added a commit that referenced this pull request Jan 23, 2024
juliencrn added a commit that referenced this pull request Jan 25, 2024
* ✨ Add serialization support for use-*-storage hooks

* ✅ Add unit tests about #437 (code from #276 by @sskirby)

* ✨ Explicity allow () => T as initial value (missing in #436)

* ✨ Add Date, Set & Map support to use*Storage (#309 by @AlecsFarias)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants